diff --git a/connections/implementation/offline_service_controller.h b/connections/implementation/offline_service_controller.h index 9331aa351d..27f27183d2 100644 --- a/connections/implementation/offline_service_controller.h +++ b/connections/implementation/offline_service_controller.h @@ -39,6 +39,8 @@ namespace connections { class OfflineServiceController : public ServiceController { public: OfflineServiceController() = default; + explicit OfflineServiceController(const BwuManager::Config& bwu_config) + : bwu_config_(bwu_config) {}; ~OfflineServiceController() override; Status StartAdvertising(ClientProxy* client, const std::string& service_id, @@ -102,6 +104,8 @@ class OfflineServiceController : public ServiceController { void ShutdownBwuManagerExecutors() override; + BwuManager::Config GetBwuConfig() const { return bwu_config_; } + private: // Note that the order of declaration of these is crucial, because we depend // on the destructors running (strictly) in the reverse order; a deviation @@ -111,8 +115,9 @@ class OfflineServiceController : public ServiceController { EndpointChannelManager channel_manager_; EndpointManager endpoint_manager_{&channel_manager_}; PayloadManager payload_manager_{endpoint_manager_}; + BwuManager::Config bwu_config_; BwuManager bwu_manager_{ - mediums_, endpoint_manager_, channel_manager_, {}, {}}; + mediums_, endpoint_manager_, channel_manager_, {}, bwu_config_}; InjectedBluetoothDeviceStore injected_bluetooth_device_store_; PcpManager pcp_manager_{mediums_, channel_manager_, endpoint_manager_, bwu_manager_, injected_bluetooth_device_store_}; diff --git a/connections/implementation/service_controller_router.cc b/connections/implementation/service_controller_router.cc index d57ccbedaa..e51e075b87 100644 --- a/connections/implementation/service_controller_router.cc +++ b/connections/implementation/service_controller_router.cc @@ -15,15 +15,20 @@ #include "connections/implementation/service_controller_router.h" #include +#include #include #include #include +#include "absl/strings/string_view.h" +#include "connections/advertising_options.h" #include "connections/discovery_options.h" +#include "connections/implementation/bwu_manager.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "connections/implementation/offline_service_controller.h" #include "connections/listeners.h" +#include "connections/medium_selector.h" #include "connections/params.h" #include "connections/payload.h" #include "connections/v3/bandwidth_info.h" @@ -89,6 +94,17 @@ ServiceControllerRouter::ServiceControllerRouter() { NEARBY_LOGS(INFO) << "ServiceControllerRouter going up."; } +ServiceControllerRouter::ServiceControllerRouter( + bool enable_wifi_hotspot_for_hp_realtek_devices, + std::function if_hp_realtek_device) + : enable_wifi_hotspot_for_hp_realtek_devices_( + enable_wifi_hotspot_for_hp_realtek_devices), + if_hp_realtek_device_(std::move(if_hp_realtek_device)) { + LOG(INFO) << "ServiceControllerRouter going up, " + "enable_wifi_hotspot_for_hp_realtek_devices=" + << enable_wifi_hotspot_for_hp_realtek_devices_; +} + // Constructor called by the CrOS platform implementation to override the // kEnableBleV2 flag. ServiceControllerRouter::ServiceControllerRouter(bool enable_ble_v2) @@ -713,7 +729,29 @@ void ServiceControllerRouter::SetServiceControllerForTesting( ServiceController* ServiceControllerRouter::GetServiceController() { if (!service_controller_) { - service_controller_ = std::make_unique(); + LOG(INFO) << __func__ << ": enable_wifi_hotspot_for_hp_realtek_devices_ = " + << enable_wifi_hotspot_for_hp_realtek_devices_ + << " if_hp_realtek_device_ = " << if_hp_realtek_device_(); + // Temporarily fix to get around wifi hotspot issues for HP Aero with + // Realtek. + // See b/380191431 for more details. + if (!enable_wifi_hotspot_for_hp_realtek_devices_ && + if_hp_realtek_device_()) { + LOG(INFO) << __func__ + << ": it is HP Realtek device, set BwuManager::Config."; + BwuManager::Config bwu_config; + bwu_config.allow_upgrade_to = {/*bluetooth=*/true, + /*ble=*/true, + /*web_rtc_no_cellular=*/true, + /*web_rtc=*/true, + /*wifi_lan=*/true, + /*wifi_hotspot=*/false, + /*wifi_direct=*/true}; + service_controller_ = + std::make_unique(bwu_config); + } else { + service_controller_ = std::make_unique(); + } } return service_controller_.get(); } diff --git a/connections/implementation/service_controller_router.h b/connections/implementation/service_controller_router.h index 31f7a9c35a..d4c290d197 100644 --- a/connections/implementation/service_controller_router.h +++ b/connections/implementation/service_controller_router.h @@ -15,11 +15,11 @@ #ifndef CORE_INTERNAL_SERVICE_CONTROLLER_ROUTER_H_ #define CORE_INTERNAL_SERVICE_CONTROLLER_ROUTER_H_ +#include #include #include #include -#include "absl/container/flat_hash_set.h" #include "absl/strings/string_view.h" #include "absl/types/span.h" #include "connections/implementation/client_proxy.h" @@ -59,6 +59,8 @@ class ServiceControllerRouter { public: ServiceControllerRouter(); explicit ServiceControllerRouter(bool enable_ble_v2); + ServiceControllerRouter(bool enable_wifi_hotspot_for_hp_realtek_devices, + std::function if_hp_realtek_device); virtual ~ServiceControllerRouter(); // Not copyable or movable ServiceControllerRouter(const ServiceControllerRouter&) = delete; @@ -173,11 +175,12 @@ class ServiceControllerRouter { void SetServiceControllerForTesting( std::unique_ptr service_controller); - - private: // Lazily create ServiceController. ServiceController* GetServiceController(); + private: + bool enable_wifi_hotspot_for_hp_realtek_devices_ = false; + std::function if_hp_realtek_device_; void RouteToServiceController(const std::string& name, Runnable runnable); void FinishClientSession(ClientProxy* client); diff --git a/connections/implementation/service_controller_router_test.cc b/connections/implementation/service_controller_router_test.cc index ca953b8301..a9f0824e37 100644 --- a/connections/implementation/service_controller_router_test.cc +++ b/connections/implementation/service_controller_router_test.cc @@ -28,7 +28,9 @@ #include "connections/implementation/client_proxy.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "connections/implementation/mock_service_controller.h" +#include "connections/implementation/offline_service_controller.h" #include "connections/listeners.h" +#include "connections/medium_selector.h" #include "connections/params.h" #include "connections/v3/bandwidth_info.h" #include "connections/v3/connection_listening_options.h" @@ -36,8 +38,8 @@ #include "connections/v3/connections_device.h" #include "connections/v3/listening_result.h" #include "connections/v3/params.h" -#include "internal/interop/authentication_status.h" #include "internal/flags/nearby_flags.h" +#include "internal/interop/authentication_status.h" #include "internal/platform/byte_array.h" #include "internal/platform/condition_variable.h" #include "internal/platform/count_down_latch.h" @@ -1331,6 +1333,60 @@ TEST_F(ServiceControllerRouterTest, /*expecting_call=*/false); } +TEST(ServiceControllerRouterCheckHpRealtekDeviceTest, + disableWifiHotspotForHpRealtekDevices_isHPRealtekDevice_checkBwuConfig) { + ServiceControllerRouter router( + false /* enable_wifi_hotspot_for_hp_realtek_devices */, + []() { return true; }); + auto service_controller = router.GetServiceController(); + EXPECT_NE(service_controller, nullptr); + auto bwu_config = static_cast(service_controller) + ->GetBwuConfig(); + + EXPECT_THAT(bwu_config.allow_upgrade_to, + testing::FieldsAre(/*bluetooth=*/true, /*ble=*/true, + /*web_rtc_no_cellular=*/true, + /*web_rtc=*/true, /*wifi_lan=*/true, + /*wifi_hotspot=*/false, /*wifi_direct=*/true)); +} + +TEST( + ServiceControllerRouterCheckHpRealtekDeviceTest, + disableWifiHotspotForHpRealtekDevices_notHPRealtekDevice_defaultBwuConfig) { + ServiceControllerRouter router( + false /* enable_wifi_hotspot_for_hp_realtek_devices */, + []() { return false; }); + auto service_controller = router.GetServiceController(); + EXPECT_NE(service_controller, nullptr); + auto bwu_config = static_cast(service_controller) + ->GetBwuConfig(); + + EXPECT_THAT( + bwu_config.allow_upgrade_to, + testing::FieldsAre(/*bluetooth=*/false, /*ble=*/false, + /*web_rtc_no_cellular=*/false, + /*web_rtc=*/false, /*wifi_lan=*/false, + /*wifi_hotspot=*/false, /*wifi_direct=*/false)); +} + +TEST(ServiceControllerRouterCheckHpRealtekDeviceTest, + EnableWifiHotspotForHpRealtekDevices_defaultBwuConfig) { + ServiceControllerRouter router( + true /* enable_wifi_hotspot_for_hp_realtek_devices */, + []() { return true; }); + auto service_controller = router.GetServiceController(); + EXPECT_NE(service_controller, nullptr); + auto bwu_config = static_cast(service_controller) + ->GetBwuConfig(); + + EXPECT_THAT( + bwu_config.allow_upgrade_to, + testing::FieldsAre(/*bluetooth=*/false, /*ble=*/false, + /*web_rtc_no_cellular=*/false, + /*web_rtc=*/false, /*wifi_lan=*/false, + /*wifi_hotspot=*/false, /*wifi_direct=*/false)); +} + } // namespace } // namespace connections } // namespace nearby diff --git a/sharing/flags/generated/nearby_sharing_feature_flags.h b/sharing/flags/generated/nearby_sharing_feature_flags.h index 2c62cd7a90..aca93a9403 100755 --- a/sharing/flags/generated/nearby_sharing_feature_flags.h +++ b/sharing/flags/generated/nearby_sharing_feature_flags.h @@ -82,6 +82,9 @@ constexpr auto kDeleteUnexpectedReceivedFileFix = // The default time in milliseconds a cached entry can be in LOST state. constexpr auto kDiscoveryCacheLostExpiryMs = flags::Flag(kConfigPackage, "45658774", 500); +// When true, enable wifi hotspot medium for selected devices. +constexpr auto kEnableWifiHotspotForHpRealtekDevices = + flags::Flag(kConfigPackage, "45673628", false); // When true, honor 3P client_id & client_secret in the gRPC request constexpr auto kHonor3PClientIdAndSecret = flags::Flag(kConfigPackage, "45665616", true); @@ -124,6 +127,7 @@ inline absl::btree_map&> GetBoolFlags() { {45667328, kCallNearbyIdentityApi}, {45664277, kDedupInUnregisterShareTarget}, {45657036, kDeleteUnexpectedReceivedFileFix}, + {45673628, kEnableWifiHotspotForHpRealtekDevices}, {45665616, kHonor3PClientIdAndSecret}, {45417647, kEnableQrCodeUi}, {45410558, kShowAdminModeWarning}, diff --git a/sharing/internal/public/BUILD b/sharing/internal/public/BUILD index 1dc3ee2876..aa1779dca9 100644 --- a/sharing/internal/public/BUILD +++ b/sharing/internal/public/BUILD @@ -22,10 +22,8 @@ cc_library( ], visibility = ["//visibility:public"], deps = [ - "//internal/network:url", "//internal/platform:types", "//sharing/internal/api:platform", - "@com_google_absl//absl/status", "@com_google_absl//absl/strings", ], ) @@ -44,12 +42,16 @@ cc_library( deps = [ ":logging", ":types", + "//internal/flags:nearby_flags", "//internal/network:url", "//internal/platform:types", + "//sharing/flags/generated:generated_flags", "//sharing/internal/api:platform", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", + "@com_google_absl//absl/synchronization", ], ) @@ -66,6 +68,7 @@ cc_test( ":types", "//internal/platform/implementation/g3", # fixdeps: keep "//sharing/internal/api:mock_sharing_platform", + "//sharing/internal/api:platform", "@com_github_protobuf_matchers//protobuf-matchers", "@com_google_googletest//:gtest_main", ], diff --git a/sharing/internal/public/connectivity_manager.h b/sharing/internal/public/connectivity_manager.h index 8e697e26aa..8907ba4e40 100644 --- a/sharing/internal/public/connectivity_manager.h +++ b/sharing/internal/public/connectivity_manager.h @@ -48,6 +48,8 @@ class ConnectivityManager { std::function) = 0; virtual void UnregisterConnectionListener( absl::string_view listener_name) = 0; + // Is the device a HP device with Realtek wireless module. + virtual bool IsHPRealtekDevice() = 0; }; } // namespace nearby diff --git a/sharing/internal/public/connectivity_manager_impl.cc b/sharing/internal/public/connectivity_manager_impl.cc index 8628afe514..15c8d63c1d 100644 --- a/sharing/internal/public/connectivity_manager_impl.cc +++ b/sharing/internal/public/connectivity_manager_impl.cc @@ -14,13 +14,19 @@ #include "sharing/internal/public/connectivity_manager_impl.h" +#include #include #include +#include #include #include #include "absl/container/flat_hash_map.h" +#include "absl/strings/match.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" +#include "internal/flags/nearby_flags.h" +#include "sharing/flags/generated/nearby_sharing_feature_flags.h" #include "sharing/internal/api/network_monitor.h" #include "sharing/internal/api/sharing_platform.h" #include "sharing/internal/public/connectivity_manager.h" @@ -55,7 +61,8 @@ std::string GetConnectionTypeString(ConnectionType connection_type) { } // namespace -ConnectivityManagerImpl::ConnectivityManagerImpl(SharingPlatform& platform) { +ConnectivityManagerImpl::ConnectivityManagerImpl(SharingPlatform& platform) + : platform_(platform) { network_monitor_ = platform.CreateNetworkMonitor( [this](api::NetworkMonitor::ConnectionType connection_type, bool is_lan_connected) { @@ -73,6 +80,36 @@ bool ConnectivityManagerImpl::IsLanConnected() { return network_monitor_->IsLanConnected(); } +bool ConnectivityManagerImpl::IsHPRealtekDevice() { + // This function should not be called if kEnableWifiHotspotForHpRealtekDevices + // is true. + if (NearbyFlags::GetInstance().GetBoolFlag( + sharing::config_package_nearby::nearby_sharing_feature:: + kEnableWifiHotspotForHpRealtekDevices)) { + return false; + } + + absl::MutexLock lock(&mutex_); + + // Lazy initialization since CreateSystemInfo can take a long time. + if (!is_hp_realtek_device_.has_value()) { + auto system_info = platform_.CreateSystemInfo(); + const auto& driver_infos = system_info->GetNetworkDriverInfos(); + auto it = std::find_if(driver_infos.begin(), driver_infos.end(), + [](auto drive_info) { + return absl::StrContainsIgnoreCase( + drive_info.manufacturer, "Realtek"); + }); + if (it != driver_infos.end() && + absl::EqualsIgnoreCase(system_info->GetComputerManufacturer(), "HP")) { + is_hp_realtek_device_ = std::make_optional(true); + } else { + is_hp_realtek_device_ = std::make_optional(false); + } + } + return is_hp_realtek_device_.value(); +} + ConnectionType ConnectivityManagerImpl::GetConnectionType() { return static_cast(network_monitor_->GetCurrentConnection()); } diff --git a/sharing/internal/public/connectivity_manager_impl.h b/sharing/internal/public/connectivity_manager_impl.h index 07b70ec65a..b6e719521d 100644 --- a/sharing/internal/public/connectivity_manager_impl.h +++ b/sharing/internal/public/connectivity_manager_impl.h @@ -17,12 +17,16 @@ #include #include +#include #include +#include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" #include "sharing/internal/api/network_monitor.h" #include "sharing/internal/api/sharing_platform.h" +#include "sharing/internal/api/system_info.h" #include "sharing/internal/public/connectivity_manager.h" namespace nearby { @@ -34,7 +38,7 @@ class ConnectivityManagerImpl : public ConnectivityManager { nearby::sharing::api::SharingPlatform& platform); bool IsLanConnected() override; - + bool IsHPRealtekDevice() override; ConnectionType GetConnectionType() override; void RegisterConnectionListener( @@ -48,6 +52,10 @@ class ConnectivityManagerImpl : public ConnectivityManager { absl::flat_hash_map> listeners_; std::unique_ptr network_monitor_; + nearby::sharing::api::SharingPlatform& platform_; + mutable absl::Mutex mutex_; + + std::optional is_hp_realtek_device_ ABSL_GUARDED_BY(mutex_); }; } // namespace nearby diff --git a/sharing/internal/public/connectivity_manager_impl_test.cc b/sharing/internal/public/connectivity_manager_impl_test.cc index 3d9f60a3db..7479706ee4 100644 --- a/sharing/internal/public/connectivity_manager_impl_test.cc +++ b/sharing/internal/public/connectivity_manager_impl_test.cc @@ -15,6 +15,7 @@ #include "sharing/internal/public/connectivity_manager_impl.h" #include +#include #include #include @@ -23,12 +24,15 @@ #include "gtest/gtest.h" #include "sharing/internal/api/mock_network_monitor.h" #include "sharing/internal/api/mock_sharing_platform.h" +#include "sharing/internal/api/mock_system_info.h" +#include "sharing/internal/api/system_info.h" #include "sharing/internal/public/connectivity_manager.h" namespace nearby { namespace { using ::nearby::sharing::api::MockNetworkMonitor; using ::nearby::sharing::api::MockSharingPlatform; +using ::nearby::sharing::api::MockSystemInfo; using ::testing::_; using ::testing::ByMove; using ::testing::Return; @@ -59,6 +63,54 @@ TEST(ConnectivityManagerImpl, GetConnectionType) { ConnectivityManager::ConnectionType::kWifi); } +TEST(ConnectivityManagerImpl, IsHPRealtekDeviceReturnsTrue) { + MockSharingPlatform sharing_platform; + auto system_info = std::make_unique(); + auto system_info_ptr = system_info.get(); + EXPECT_CALL(sharing_platform, CreateSystemInfo) + .WillOnce(Return(std::move(system_info))); + std::list driver_infos = { + {.manufacturer = "realtek Semiconductors"}, {.manufacturer = "Intel"}}; + EXPECT_CALL(*system_info_ptr, GetNetworkDriverInfos) + .WillOnce(Return(driver_infos)); + EXPECT_CALL(*system_info_ptr, GetComputerManufacturer).WillOnce(Return("hp")); + + ConnectivityManagerImpl connectivity_manager_impl(sharing_platform); + EXPECT_TRUE(connectivity_manager_impl.IsHPRealtekDevice()); +} + +TEST(ConnectivityManagerImpl, IsHPRealtekDevice_NotHPDevice) { + MockSharingPlatform sharing_platform; + auto system_info = std::make_unique(); + auto system_info_ptr = system_info.get(); + EXPECT_CALL(sharing_platform, CreateSystemInfo) + .WillOnce(Return(std::move(system_info))); + std::list driver_infos = { + {.manufacturer = "realtek Semiconductors"}, {.manufacturer = "Intel"}}; + EXPECT_CALL(*system_info_ptr, GetNetworkDriverInfos) + .WillOnce(Return(driver_infos)); + EXPECT_CALL(*system_info_ptr, GetComputerManufacturer) + .WillOnce(Return("Lenovo")); + + ConnectivityManagerImpl connectivity_manager_impl(sharing_platform); + EXPECT_FALSE(connectivity_manager_impl.IsHPRealtekDevice()); +} + +TEST(ConnectivityManagerImpl, IsHPRealtekDevice_NotRealtekDevice) { + MockSharingPlatform sharing_platform; + auto system_info = std::make_unique(); + auto system_info_ptr = system_info.get(); + EXPECT_CALL(sharing_platform, CreateSystemInfo) + .WillOnce(Return(std::move(system_info))); + std::list driver_infos = { + {.manufacturer = "Intel"}}; + EXPECT_CALL(*system_info_ptr, GetNetworkDriverInfos) + .WillOnce(Return(driver_infos)); + + ConnectivityManagerImpl connectivity_manager_impl(sharing_platform); + EXPECT_FALSE(connectivity_manager_impl.IsHPRealtekDevice()); +} + TEST(ConnectivityManagerImpl, RegisterConnectionListener) { std::function listener_1 = [](ConnectivityManager::ConnectionType connection_type, diff --git a/sharing/internal/test/fake_connectivity_manager.h b/sharing/internal/test/fake_connectivity_manager.h index 09adade188..6ee3a27a8e 100644 --- a/sharing/internal/test/fake_connectivity_manager.h +++ b/sharing/internal/test/fake_connectivity_manager.h @@ -28,7 +28,10 @@ namespace nearby { class FakeConnectivityManager : public ConnectivityManager { public: bool IsLanConnected() override { return is_lan_connected_; } - + bool IsHPRealtekDevice() override { return is_hp_realtek_device_; } + void SetIsHPRealtekDevice(bool is_hp_realtek_device) { + is_hp_realtek_device_ = is_hp_realtek_device; + } ConnectionType GetConnectionType() override { return connection_type_; } void RegisterConnectionListener( @@ -59,6 +62,7 @@ class FakeConnectivityManager : public ConnectivityManager { private: bool is_lan_connected_ = true; + bool is_hp_realtek_device_ = false; ConnectionType connection_type_ = ConnectionType::kWifi; absl::flat_hash_map> listeners_; diff --git a/sharing/nearby_connections_manager_factory.cc b/sharing/nearby_connections_manager_factory.cc index e34dd0bee5..5b35b535b0 100644 --- a/sharing/nearby_connections_manager_factory.cc +++ b/sharing/nearby_connections_manager_factory.cc @@ -34,7 +34,8 @@ NearbyConnectionsManagerFactory::CreateConnectionsManager( return std::make_unique( connections_callback_task_runner, context, *context->GetConnectivityManager(), device_info, - std::make_unique(event_logger)); + std::make_unique( + *context->GetConnectivityManager(), event_logger)); } } // namespace nearby::sharing diff --git a/sharing/nearby_connections_manager_impl.cc b/sharing/nearby_connections_manager_impl.cc index 83d9e03ad6..feb22e1c8c 100644 --- a/sharing/nearby_connections_manager_impl.cc +++ b/sharing/nearby_connections_manager_impl.cc @@ -115,6 +115,20 @@ bool ShouldEnableWifiLan(ConnectivityManager& connectivity_manager) { return is_connection_wifi_or_ethernet; } +// Temporarily fix to get around wifi hotspot issues for HP Aero with Realtek. +// See b/380191431 for more details. + +bool ShouldEnableWifiHotspot(ConnectivityManager& connectivity_manager) { + if (!NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_sharing_feature:: + kEnableWifiHotspotForHpRealtekDevices) && + connectivity_manager.IsHPRealtekDevice()) { + LOG(WARNING) << __func__ << ": Disable wifi hotspot for HP with Realtek"; + return false; + } + return true; +} + bool ShouldEnableBleForTransfers() { return NearbyFlags::GetInstance().GetBoolFlag( config_package_nearby::nearby_sharing_feature::kEnableBleForTransfer); @@ -145,7 +159,6 @@ std::string PayloadStatusToString(PayloadStatus status) { return "Canceled"; } } - } // namespace NearbyConnectionsManagerImpl::NearbyConnectionsManagerImpl( @@ -186,9 +199,8 @@ void NearbyConnectionsManagerImpl::StartAdvertising( // upgrades from this advertisement. ShouldEnableWebRtc(connectivity_manager_, data_usage, PowerLevel::kHighPower), - /*wifi_lan=*/ ShouldEnableWifiLan(connectivity_manager_), - /*wifi_hotspot=*/true); + ShouldEnableWifiHotspot(connectivity_manager_)); VLOG(1) << __func__ << ": " << "is_high_power=" << (is_high_power ? "yes" : "no") << ", data_usage=" << static_cast(data_usage) @@ -276,7 +288,6 @@ void NearbyConnectionsManagerImpl::StartDiscovery( std::move(callback)(ConnectionsStatus::kError); return; } - MediumSelection allowed_mediums = MediumSelection( /*bluetooth=*/true, /*ble=*/true, @@ -285,7 +296,7 @@ void NearbyConnectionsManagerImpl::StartDiscovery( PowerLevel::kHighPower), /*wifi_lan=*/ ShouldEnableWifiLan(connectivity_manager_), - /*wifi_hotspot=*/true); + ShouldEnableWifiHotspot(connectivity_manager_)); VLOG(1) << __func__ << ": " << "data_usage=" << static_cast(data_usage) << ", allowed_mediums=" << MediumSelectionToString(allowed_mediums); @@ -348,8 +359,11 @@ void NearbyConnectionsManagerImpl::Connect( PowerLevel::kHighPower), /*wifi_lan=*/ ShouldEnableWifiLan(connectivity_manager_), + // Note: this only affects sending since this function is only for + // outgoing connections. /*wifi_hotspot=*/ - IsTransportTypeFlagsSet(transport_type, TransportType::kHighQuality)); + IsTransportTypeFlagsSet(transport_type, TransportType::kHighQuality) && + ShouldEnableWifiHotspot(connectivity_manager_)); VLOG(1) << __func__ << ": " << "data_usage=" << static_cast(data_usage) << ", allowed_mediums=" << MediumSelectionToString(allowed_mediums); diff --git a/sharing/nearby_connections_manager_impl_test.cc b/sharing/nearby_connections_manager_impl_test.cc index 9a45c35b0c..27707f7d1d 100644 --- a/sharing/nearby_connections_manager_impl_test.cc +++ b/sharing/nearby_connections_manager_impl_test.cc @@ -123,8 +123,7 @@ class MockPayloadStatusListener : public NearbyConnectionsManager::PayloadStatusListener { public: MOCK_METHOD(void, OnStatusUpdate, - (std::unique_ptr update), - (override)); + (std::unique_ptr update), (override)); }; class NearbyConnectionsManagerImplTest : public testing::Test { @@ -526,15 +525,9 @@ TEST_F(NearbyConnectionsManagerImplTest, DiscoveryFlow) { kSynchronizationTimeOut)); } -TEST_F(NearbyConnectionsManagerImplTest, DisableWifiHotspot) { +TEST_F(NearbyConnectionsManagerImplTest, + DisableWifiHotspotForHighQualityNonDisruptiveTransport) { SetConnectionType(ConnectivityManager::ConnectionType::kWifi); - - MediumSelection expected_mediums(/*bluetooth=*/true, - /*ble=*/false, - /*web_rtc=*/should_use_web_rtc_, - /*wifi_lan=*/should_use_wifilan_, - /*wifi_hotspot*/ true); - // StartDiscovery will succeed. NearbyConnectionsService::DiscoveryListener discovery_listener_remote; testing::NiceMock discovery_listener; @@ -570,6 +563,45 @@ TEST_F(NearbyConnectionsManagerImplTest, DisableWifiHotspot) { notification.WaitForNotificationWithTimeout(kSynchronizationTimeOut)); } +TEST_F(NearbyConnectionsManagerImplTest, DisableWifiHotspotForHPRealtekDevice) { + SetConnectionType(ConnectivityManager::ConnectionType::kWifi); + fake_connectivity_manager_.SetIsHPRealtekDevice(true); + + // StartDiscovery will succeed. + NearbyConnectionsService::DiscoveryListener discovery_listener_remote; + testing::NiceMock discovery_listener; + StartDiscovery(discovery_listener_remote, DataUsage::WIFI_ONLY_DATA_USAGE, + discovery_listener); + + absl::Notification notification; + const std::vector local_endpoint_info(std::begin(kEndpointInfo), + std::end(kEndpointInfo)); + EXPECT_CALL(*nearby_connections_, RequestConnection) + .WillOnce([&](absl::string_view service_id, + const std::vector& endpoint_info, + absl::string_view endpoint_id, ConnectionOptions options, + NearbyConnectionsService::ConnectionListener listener, + std::function callback) { + EXPECT_EQ(service_id, kServiceId); + EXPECT_EQ(endpoint_info, local_endpoint_info); + EXPECT_EQ(endpoint_id, kRemoteEndpointId); + EXPECT_FALSE(options.allowed_mediums.wifi_hotspot); + EXPECT_FALSE(options.non_disruptive_hotspot_mode); + std::move(callback)(Status::kSuccess); + notification.Notify(); + }); + + NearbyConnectionsManager::NearbyConnectionCallback connections_callback; + + nearby_connections_manager_->Connect( + local_endpoint_info, kRemoteEndpointId, + /*bluetooth_mac_address=*/std::nullopt, DataUsage::WIFI_ONLY_DATA_USAGE, + TransportType::kHighQuality, connections_callback); + + EXPECT_TRUE( + notification.WaitForNotificationWithTimeout(kSynchronizationTimeOut)); +} + /******************************************************************************/ // Begin: NearbyConnectionsManagerImplTestConnectionMediums /******************************************************************************/ @@ -1434,9 +1466,9 @@ TEST_F(NearbyConnectionsManagerImplTest, IncomingBytesPayload) { StartAdvertising(connection_listener_remote, incoming_connection_listener); NearbyConnectionsService::PayloadListener payload_listener_remote; - ASSERT_TRUE(OnIncomingConnection( - connection_listener_remote, incoming_connection_listener, - payload_listener_remote) != nullptr); + ASSERT_TRUE(OnIncomingConnection(connection_listener_remote, + incoming_connection_listener, + payload_listener_remote) != nullptr); auto payload_listener = std::make_shared>(); @@ -1450,8 +1482,9 @@ TEST_F(NearbyConnectionsManagerImplTest, IncomingBytesPayload) { Payload(kPayloadId, expected_payload)); absl::Notification payload_notification; - EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)) - .WillOnce([&]() { payload_notification.Notify(); }); + EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)).WillOnce([&]() { + payload_notification.Notify(); + }); payload_listener_remote.payload_progress_cb( kRemoteEndpointId, @@ -1495,8 +1528,9 @@ TEST_F(NearbyConnectionsManagerImplTest, IncomingFilePayload) { Payload(kPayloadId, InputFile(file))); absl::Notification payload_notification; - EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)) - .WillOnce([&]() { payload_notification.Notify(); }); + EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)).WillOnce([&]() { + payload_notification.Notify(); + }); payload_listener_remote.payload_progress_cb( kRemoteEndpointId, @@ -1545,8 +1579,9 @@ TEST_F(NearbyConnectionsManagerImplTest, ClearIncomingPayloads) { Payload(kPayloadId, InputFile(file))); absl::Notification payload_notification; - EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)) - .WillOnce([&]() { payload_notification.Notify(); }); + EXPECT_CALL(*payload_listener, OnStatusUpdate(::testing::_)).WillOnce([&]() { + payload_notification.Notify(); + }); payload_listener_remote.payload_progress_cb( kRemoteEndpointId, @@ -1812,9 +1847,9 @@ TEST_F(NearbyConnectionsManagerImplTest, StartAdvertising(connection_listener_remote, incoming_connection_listener); NearbyConnectionsService::PayloadListener payload_listener_remote; - ASSERT_TRUE(OnIncomingConnection( - connection_listener_remote, incoming_connection_listener, - payload_listener_remote) != nullptr); + ASSERT_TRUE(OnIncomingConnection(connection_listener_remote, + incoming_connection_listener, + payload_listener_remote) != nullptr); std::filesystem::path file(std::filesystem::temp_directory_path() / "file.jpg"); payload_listener_remote.payload_cb(kRemoteEndpointId, @@ -1839,17 +1874,16 @@ TEST_F(NearbyConnectionsManagerImplTest, EXPECT_TRUE(unknown_file_paths.empty()); } -TEST_F(NearbyConnectionsManagerImplTest, - OnPayloadReceivedForUnknownFile) { +TEST_F(NearbyConnectionsManagerImplTest, OnPayloadReceivedForUnknownFile) { NearbyConnectionsService::ConnectionListener connection_listener_remote; testing::NiceMock incoming_connection_listener; StartAdvertising(connection_listener_remote, incoming_connection_listener); NearbyConnectionsService::PayloadListener payload_listener_remote; - ASSERT_TRUE(OnIncomingConnection( - connection_listener_remote, incoming_connection_listener, - payload_listener_remote) != nullptr); + ASSERT_TRUE(OnIncomingConnection(connection_listener_remote, + incoming_connection_listener, + payload_listener_remote) != nullptr); std::filesystem::path file(std::filesystem::temp_directory_path() / "file.jpg"); payload_listener_remote.payload_cb(kRemoteEndpointId, @@ -1862,14 +1896,14 @@ TEST_F(NearbyConnectionsManagerImplTest, true); nearby_connections_manager_->ClearIncomingPayloads(); Payload payload(kPayloadId, InputFile(file)); - nearby_connections_manager_->OnPayloadReceivedForTesting( - kRemoteEndpointId, payload); + nearby_connections_manager_->OnPayloadReceivedForTesting(kRemoteEndpointId, + payload); std::filesystem::path file2(std::filesystem::temp_directory_path() / - "file2.jpg"); + "file2.jpg"); Payload payload2(kPayloadId, InputFile(file2)); - nearby_connections_manager_->OnPayloadReceivedForTesting( - kRemoteEndpointId, payload2); + nearby_connections_manager_->OnPayloadReceivedForTesting(kRemoteEndpointId, + payload2); auto unknown_file_paths = nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete(); @@ -1880,10 +1914,10 @@ TEST_F(NearbyConnectionsManagerImplTest, nearby_connections_manager_->RegisterPayloadStatusListener( kPayloadId, payload_listener->GetWeakPtr()); std::filesystem::path file3(std::filesystem::temp_directory_path() / - "file3.jpg"); + "file3.jpg"); Payload payload3(kPayloadId, InputFile(file3)); - nearby_connections_manager_->OnPayloadReceivedForTesting( - kRemoteEndpointId, payload3); + nearby_connections_manager_->OnPayloadReceivedForTesting(kRemoteEndpointId, + payload3); unknown_file_paths = nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting(); @@ -1903,9 +1937,9 @@ TEST_F(NearbyConnectionsManagerImplTest, StartAdvertising(connection_listener_remote, incoming_connection_listener); NearbyConnectionsService::PayloadListener payload_listener_remote; - ASSERT_TRUE(OnIncomingConnection( - connection_listener_remote, incoming_connection_listener, - payload_listener_remote) != nullptr); + ASSERT_TRUE(OnIncomingConnection(connection_listener_remote, + incoming_connection_listener, + payload_listener_remote) != nullptr); auto payload_listener = std::make_shared>(); nearby_connections_manager_->RegisterPayloadStatusListener( @@ -1915,8 +1949,8 @@ TEST_F(NearbyConnectionsManagerImplTest, "file.jpg"); Payload payload(kPayloadId, InputFile(file)); - nearby_connections_manager_->OnPayloadReceivedForTesting( - kRemoteEndpointId, payload); + nearby_connections_manager_->OnPayloadReceivedForTesting(kRemoteEndpointId, + payload); auto unknown_file_paths = nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting(); diff --git a/sharing/nearby_connections_service_impl.cc b/sharing/nearby_connections_service_impl.cc index a11da2a60f..f1c40237cf 100644 --- a/sharing/nearby_connections_service_impl.cc +++ b/sharing/nearby_connections_service_impl.cc @@ -35,6 +35,8 @@ #include "connections/strategy.h" #include "internal/analytics/event_logger.h" #include "internal/platform/logging.h" +#include "sharing/flags/generated/nearby_sharing_feature_flags.h" +#include "sharing/internal/public/connectivity_manager.h" #include "sharing/nearby_connections_service.h" #include "sharing/nearby_connections_types.h" @@ -49,8 +51,14 @@ Core* GetService(NearbyConnectionsService::HANDLE handle) { } // namespace NearbyConnectionsServiceImpl::NearbyConnectionsServiceImpl( - nearby::analytics::EventLogger* event_logger) { - static ServiceControllerRouter* router = new ServiceControllerRouter(); + nearby::ConnectivityManager& connectivity_manager, + nearby::analytics::EventLogger* event_logger) + : connectivity_manager_(connectivity_manager) { + static ServiceControllerRouter* router = new ServiceControllerRouter( + NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_sharing_feature:: + kEnableWifiHotspotForHpRealtekDevices), + [this]() { return connectivity_manager_.IsHPRealtekDevice(); }); static Core* core = new Core(event_logger, router); service_handle_ = core; } diff --git a/sharing/nearby_connections_service_impl.h b/sharing/nearby_connections_service_impl.h index 11ce97d3f0..98f48e0367 100644 --- a/sharing/nearby_connections_service_impl.h +++ b/sharing/nearby_connections_service_impl.h @@ -26,6 +26,7 @@ #include "absl/strings/string_view.h" #include "absl/types/span.h" #include "internal/analytics/event_logger.h" +#include "sharing/internal/public/connectivity_manager.h" #include "sharing/nearby_connections_service.h" #include "sharing/nearby_connections_types.h" @@ -35,6 +36,7 @@ namespace sharing { class NearbyConnectionsServiceImpl : public NearbyConnectionsService { public: explicit NearbyConnectionsServiceImpl( + nearby::ConnectivityManager& connectivity_manager, nearby::analytics::EventLogger* event_logger = nullptr); NearbyConnectionsServiceImpl() = delete; ~NearbyConnectionsServiceImpl() override; @@ -90,7 +92,7 @@ class NearbyConnectionsServiceImpl : public NearbyConnectionsService { private: HANDLE service_handle_ = nullptr; - + nearby::ConnectivityManager& connectivity_manager_; ConnectionListener advertising_listener_; DiscoveryListener discovery_listener_; ConnectionListener connection_listener_;