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