From 7d3b6d7d829985ec7e9396e36bd544eadbd66913 Mon Sep 17 00:00:00 2001 From: hai007 Date: Mon, 6 Jun 2022 11:48:00 -0700 Subject: [PATCH] Disable BWU from WLAN to Hotspot for Discoverer side Windows doesn't support BWU from WIFI_LAN to WIFI_HOTSPOT. Because when the discoverer side try to switch from current AP to the Hotspot, it loss physical connection. When here's no physical connection, BWU protocol frame exchange won't be finished successfully, then BWU will fail eventually. In the current code implementation, if Windows Nearby act as Advertiser, it will not do BWU from WLAN to Hotspot. This CL adds similar logic when Windows Nearby act as discoverer. PiperOrigin-RevId: 453248717 --- connections/implementation/bwu_manager.cc | 35 ++++++++++++------- .../implementation/bwu_manager_test.cc | 23 ++++++++++-- .../endpoint_channel_manager.cc | 20 +++++++++++ .../implementation/endpoint_channel_manager.h | 4 +++ 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/connections/implementation/bwu_manager.cc b/connections/implementation/bwu_manager.cc index c1803b75..f15bbba4 100644 --- a/connections/implementation/bwu_manager.cc +++ b/connections/implementation/bwu_manager.cc @@ -178,20 +178,16 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, << " with medium " << proto::connections::Medium_Name(proposed_medium); - auto channel = channel_manager_->GetChannelForEndpoint(endpoint_id); - Medium channel_medium = - channel ? channel->GetMedium() : Medium::UNKNOWN_MEDIUM; - - if ((channel_medium == Medium::WIFI_LAN) && + if (channel_manager_->isWifiLanConnected() && (proposed_medium == Medium::WIFI_HOTSPOT)) { NEARBY_LOGS(INFO) - << "Current medium is WIFI_LAN and proposed upgrade medium is " - ":WIFI_HOTSPOT. Don't do the BWU because connecting to " - "WIFI_HOTSPOT will destroy WIFI_LAN which will lead BWU fail"; + << "Some endpoint is using WIFI_LAN and proposed upgrade medium is " + "WIFI_HOTSPOT. Don't do the BWU because connecting to " + "WIFI_HOTSPOT will destroy WIFI_LAN which will lead BWU fail and " + "other endpoint connection fail"; return; } - SetBwuMediumForEndpoint(endpoint_id, proposed_medium); BwuHandler* handler = GetHandlerForMedium(proposed_medium); if (!handler) { @@ -213,6 +209,9 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, CancelRetryUpgradeAlarm(endpoint_id); + auto channel = channel_manager_->GetChannelForEndpoint(endpoint_id); + Medium channel_medium = + channel ? channel->GetMedium() : Medium::UNKNOWN_MEDIUM; client->GetAnalyticsRecorder().OnBandwidthUpgradeStarted( endpoint_id, channel_medium, proposed_medium, proto::connections::INCOMING, client->GetConnectionToken(endpoint_id)); @@ -614,11 +613,23 @@ void BwuManager::RunUpgradeProtocol( void BwuManager::ProcessBwuPathAvailableEvent( ClientProxy* client, const string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { - Medium medium = + Medium upgrade_medium = parser::UpgradePathInfoMediumToMedium(upgrade_path_info.medium()); NEARBY_LOGS(INFO) << "ProcessBwuPathAvailableEvent for endpoint " << endpoint_id << " medium " - << proto::connections::Medium_Name(medium); + << proto::connections::Medium_Name(upgrade_medium); + + if (channel_manager_->isWifiLanConnected() && + (upgrade_medium == Medium::WIFI_HOTSPOT)) { + NEARBY_LOGS(INFO) + << "Some endpoint is using WIFI_LAN and proposed upgrade medium is " + "WIFI_HOTSPOT. Don't do the BWU because connecting to " + "WIFI_HOTSPOT will destroy WIFI_LAN which will lead BWU fail and " + "other endpoint connection fail"; + RunUpgradeFailedProtocol(client, endpoint_id, upgrade_path_info); + return; + } + if (in_progress_upgrades_.contains(endpoint_id)) { NEARBY_LOGS(ERROR) << "BwuManager received a duplicate bandwidth upgrade for endpoint " @@ -646,8 +657,6 @@ void BwuManager::ProcessBwuPathAvailableEvent( return; } Medium current_medium = GetBwuMediumForEndpoint(endpoint_id); - Medium upgrade_medium = - parser::UpgradePathInfoMediumToMedium(upgrade_path_info.medium()); if (current_medium == Medium::UNKNOWN_MEDIUM) { SetBwuMediumForEndpoint(endpoint_id, upgrade_medium); } diff --git a/connections/implementation/bwu_manager_test.cc b/connections/implementation/bwu_manager_test.cc index 97cf9dbe..779b42ae 100644 --- a/connections/implementation/bwu_manager_test.cc +++ b/connections/implementation/bwu_manager_test.cc @@ -224,6 +224,17 @@ TEST_P(BwuManagerTestParam, EXPECT_EQ(1u, fake_web_rtc_bwu_handler_->handle_initialize_calls().size()); } +TEST_P(BwuManagerTestParam, + InitiateBwu_Error_DontUpgradeFromWIFI_LANToWIFI_HOTSPOT) { + CreateInitialEndpoint(kServiceIdA, kEndpointId1, Medium::WIFI_LAN); + + // Ignore request to upgrade to WebRTC if we're already connected. + bwu_manager_->InitiateBwuForEndpoint(&client_, std::string(kEndpointId1), + Medium::WIFI_HOTSPOT); + EXPECT_TRUE( + fake_wifi_hotspot_bwu_handler_->handle_initialize_calls().empty()); +} + TEST_P(BwuManagerTestParam, InitiateBwu_Error_NoMediumHandler) { // Try to upgrade to a medium without a handler (WIFI_HOTSPOT is not support // in these tests). Should just early return with no action. @@ -493,12 +504,12 @@ TEST_F( CreateInitialEndpoint(kServiceIdB, kEndpointId4, Medium::BLUETOOTH); FullyUpgradeEndpoint(kEndpointId1, /*initial_medium=*/Medium::BLUETOOTH, /*upgrade_medium=*/Medium::WEB_RTC); + FullyUpgradeEndpoint(kEndpointId4, /*initial_medium=*/Medium::BLUETOOTH, + /*upgrade_medium=*/Medium::WIFI_HOTSPOT); FullyUpgradeEndpoint(kEndpointId2, /*initial_medium=*/Medium::BLUETOOTH, /*upgrade_medium=*/Medium::WIFI_LAN); FullyUpgradeEndpoint(kEndpointId3, /*initial_medium=*/Medium::BLUETOOTH, /*upgrade_medium=*/Medium::WIFI_LAN); - FullyUpgradeEndpoint(kEndpointId4, /*initial_medium=*/Medium::BLUETOOTH, - /*upgrade_medium=*/Medium::WIFI_HOTSPOT); std::string upgrade_service_id_A = WrapInitiatorUpgradeServiceId(kServiceIdA); std::string upgrade_service_id_B = WrapInitiatorUpgradeServiceId(kServiceIdB); @@ -658,6 +669,14 @@ TEST_F(BwuManagerTest, InitiateBwu_Revert_OnUpgradeFailure_FlagDisabled) { EXPECT_TRUE(fake_web_rtc_bwu_handler_->handle_revert_calls().empty()); } +TEST_F(BwuManagerTest, OnReceiveBwuEvent) { + // TODO(b/235109434): Add more unit tests coverage for BWU module +} + +TEST_F(BwuManagerTest, OnProcessBwuEvent) { + // TODO(b/235109434): Add more unit tests coverage for BWU module +} + INSTANTIATE_TEST_SUITE_P(BwuManagerTestParam, BwuManagerTestParam, testing::Bool()); diff --git a/connections/implementation/endpoint_channel_manager.cc b/connections/implementation/endpoint_channel_manager.cc index 7d95358f..4e98ec4f 100644 --- a/connections/implementation/endpoint_channel_manager.cc +++ b/connections/implementation/endpoint_channel_manager.cc @@ -109,6 +109,11 @@ int EndpointChannelManager::GetConnectedEndpointsCount() const { return channel_state_.GetConnectedEndpointsCount(); } +bool EndpointChannelManager::isWifiLanConnected() const { + MutexLock lock(&mutex_); + return channel_state_.isWifiLanConnected(); +} + ///////////////////////////////// ChannelState ///////////////////////////////// // endpoint - channel endpoint to encrypt @@ -164,6 +169,21 @@ bool EndpointChannelManager::ChannelState::RemoveEndpoint( return true; } +bool EndpointChannelManager::ChannelState::isWifiLanConnected() const { + for (auto& endpoint : endpoints_) { + auto channel = endpoint.second.channel; + if (channel) { + if (channel->GetMedium() == Medium::WIFI_LAN) { + NEARBY_LOGS(INFO) << "Found WIFI_LAN Medium for endpoint:" + << endpoint.first; + return true; + } + } + } + + return false; +} + bool EndpointChannelManager::UnregisterChannelForEndpoint( const std::string& endpoint_id) { MutexLock lock(&mutex_); diff --git a/connections/implementation/endpoint_channel_manager.h b/connections/implementation/endpoint_channel_manager.h index fa431c18..f3367ed0 100644 --- a/connections/implementation/endpoint_channel_manager.h +++ b/connections/implementation/endpoint_channel_manager.h @@ -96,6 +96,9 @@ class EndpointChannelManager final { int GetConnectedEndpointsCount() const ABSL_LOCKS_EXCLUDED(mutex_); + // Check if any endpoint uses WLAN Medium + bool isWifiLanConnected() const ABSL_LOCKS_EXCLUDED(mutex_); + private: // Tracks channel state for all endpoints. This includes what EndpointChannel // the endpoint is currently using and whether or not the EndpointChannel has @@ -149,6 +152,7 @@ class EndpointChannelManager final { bool EncryptChannel(EndpointData* endpoint); int GetConnectedEndpointsCount() const { return endpoints_.size(); } + bool isWifiLanConnected() const; private: // Endpoint ID -> EndpointData. Contains everything we know about the