From da678f5b55420d5afb64cf5c819901a7ff6ae3e9 Mon Sep 17 00:00:00 2001 From: Anay Wadhera Date: Thu, 22 Jun 2023 12:28:39 -0700 Subject: [PATCH] Adjust device provider registration API to match NP expectations PiperOrigin-RevId: 542634789 --- connections/core.h | 13 +++++++++-- connections/core_test.cc | 3 ++- connections/implementation/client_proxy.cc | 12 +++++----- connections/implementation/client_proxy.h | 22 ++++++++++++++----- .../implementation/client_proxy_test.cc | 7 +++--- 5 files changed, 41 insertions(+), 16 deletions(-) diff --git a/connections/core.h b/connections/core.h index 4cf64995..017970be 100644 --- a/connections/core.h +++ b/connections/core.h @@ -516,8 +516,17 @@ class Core { // Registers a DeviceProvider to provide functionality for Nearby Connections // to interact with the DeviceProvider for retrieving the local device. - void RegisterDeviceProvider(std::unique_ptr provider) { - client_.RegisterDeviceProvider(std::move(provider)); + void RegisterDeviceProvider(NearbyDeviceProvider* provider) { + client_.RegisterDeviceProvider(provider); + } + + // Like RegisterDeviceProvider(NearbyDeviceProvider*) above, but for + // a Connections device provider, so Connections can manage the lifetime of + // this device provider. If an external provider is registered, this provider + // will be ignored. + void RegisterConnectionsDeviceProvider( + std::unique_ptr provider) { + client_.RegisterConnectionsDeviceProvider(std::move(provider)); } private: diff --git a/connections/core_test.cc b/connections/core_test.cc index c63deaaf..7c5517a2 100644 --- a/connections/core_test.cc +++ b/connections/core_test.cc @@ -153,7 +153,8 @@ TEST(CoreV3Test, TestStartAdvertisingV3NonConnectionsDeviceProvider) { CountDownLatch initiated_latch(1); AdvertisingOptions advertising_options; advertising_options.strategy = Strategy::kP2pCluster; - core.RegisterDeviceProvider(std::make_unique()); + FakeNearbyDeviceProvider device_provider; + core.RegisterDeviceProvider(&device_provider); core.StartAdvertisingV3( "service", advertising_options, { diff --git a/connections/implementation/client_proxy.cc b/connections/implementation/client_proxy.cc index 9134fd3f..626d7273 100644 --- a/connections/implementation/client_proxy.cc +++ b/connections/implementation/client_proxy.cc @@ -77,23 +77,25 @@ std::string ClientProxy::GetLocalEndpointId() { if (!local_endpoint_id_.empty()) { return local_endpoint_id_; } - if (device_provider_ == nullptr) { + if (external_device_provider_ == nullptr) { local_endpoint_id_ = GenerateLocalEndpointId(); } else { - local_endpoint_id_ = device_provider_->GetLocalDevice()->GetEndpointId(); + local_endpoint_id_ = + external_device_provider_->GetLocalDevice()->GetEndpointId(); } return local_endpoint_id_; } const NearbyDevice* ClientProxy::GetLocalDevice() { - if (device_provider_ == nullptr) { + if (external_device_provider_ == nullptr && + connections_device_provider_ == nullptr) { // TODO(b/285602283): Plug in actual endpoint info once available. auto provider = v3::ConnectionsDeviceProvider(GetLocalEndpointId(), "V3 endpoint", {}); - RegisterDeviceProvider( + RegisterConnectionsDeviceProvider( std::make_unique(provider)); } - return device_provider_->GetLocalDevice(); + return GetLocalDeviceProvider()->GetLocalDevice(); } std::string ClientProxy::GetConnectionToken(const std::string& endpoint_id) { diff --git a/connections/implementation/client_proxy.h b/connections/implementation/client_proxy.h index b22174ed..aa456321 100644 --- a/connections/implementation/client_proxy.h +++ b/connections/implementation/client_proxy.h @@ -31,6 +31,7 @@ #include "connections/status.h" #include "connections/strategy.h" #include "connections/v3/connection_listening_options.h" +#include "connections/v3/connections_device_provider.h" #include "connections/v3/listeners.h" #include "internal/analytics/event_logger.h" #include "internal/interop/device.h" @@ -74,9 +75,11 @@ class ClientProxy final { std::string GetConnectionToken(const std::string& endpoint_id); const NearbyDevice* GetLocalDevice(); - // Test-only. NearbyDeviceProvider* GetLocalDeviceProvider() { - return device_provider_.get(); + if (external_device_provider_ != nullptr) { + return external_device_provider_; + } + return connections_device_provider_.get(); } // Clears all the runtime state of this client. @@ -233,8 +236,13 @@ class ClientProxy final { absl::string_view endpoint_id, const location::nearby::connections::OsInfo& remote_os_info); - void RegisterDeviceProvider(std::unique_ptr provider) { - device_provider_ = std::move(provider); + void RegisterDeviceProvider(NearbyDeviceProvider* provider) { + external_device_provider_ = provider; + } + + void RegisterConnectionsDeviceProvider( + std::unique_ptr provider) { + connections_device_provider_ = std::move(provider); } private: @@ -392,7 +400,11 @@ class ClientProxy final { std::unique_ptr error_code_recorder_; // Local device OS information. location::nearby::connections::OsInfo local_os_info_; - std::unique_ptr device_provider_; + // For device providers not owned by Nearby connections (e.g. Nearby + // Presence's DeviceProvider.) + NearbyDeviceProvider* external_device_provider_ = nullptr; + // For Nearby Connections' own device provider. + std::unique_ptr connections_device_provider_; }; } // namespace connections diff --git a/connections/implementation/client_proxy_test.cc b/connections/implementation/client_proxy_test.cc index cb2217de..d564ad04 100644 --- a/connections/implementation/client_proxy_test.cc +++ b/connections/implementation/client_proxy_test.cc @@ -459,10 +459,10 @@ TEST_F(ClientProxyTest, GeneratedEndpointIdIsUnique) { } TEST_F(ClientProxyTest, GeneratedEndpointIdIsUniqueWithDeviceProvider) { - client1_.RegisterDeviceProvider( + client1_.RegisterConnectionsDeviceProvider( std::make_unique( v3::ConnectionsDeviceProvider("", {}))); - client2_.RegisterDeviceProvider( + client2_.RegisterConnectionsDeviceProvider( std::make_unique( v3::ConnectionsDeviceProvider("", {}))); EXPECT_NE(client1_.GetLocalEndpointId(), client2_.GetLocalEndpointId()); @@ -1012,7 +1012,8 @@ TEST_F(ClientProxyTest, GetLocalDeviceWorksWithoutDeviceProvider) { } TEST_F(ClientProxyTest, GetLocalDeviceWorksWithDeviceProvider) { - client1_.RegisterDeviceProvider(std::make_unique()); + MockDeviceProvider provider; + client1_.RegisterDeviceProvider(&provider); ASSERT_NE(client1_.GetLocalDeviceProvider(), nullptr); EXPECT_CALL( *(down_cast(client1_.GetLocalDeviceProvider())),