From 44fb9b4fa24e1140202c53a7c56bea90a0426adb Mon Sep 17 00:00:00 2001 From: Anay Wadhera Date: Thu, 15 Jun 2023 16:42:42 -0700 Subject: [PATCH] implement stoplisteningforincomingconnections PiperOrigin-RevId: 540724954 --- .../analytics/analytics_recorder.cc | 8 +++ .../analytics/analytics_recorder.h | 1 + .../analytics/analytics_recorder_test.cc | 1 + .../implementation/base_pcp_handler.cc | 13 +++++ connections/implementation/base_pcp_handler.h | 5 ++ .../implementation/base_pcp_handler_test.cc | 56 +++++++++++++++++++ connections/implementation/client_proxy.cc | 6 ++ connections/implementation/client_proxy.h | 1 + .../offline_service_controller.cc | 5 ++ .../offline_service_controller.h | 4 +- .../implementation/p2p_cluster_pcp_handler.cc | 41 ++++++++++++++ .../implementation/p2p_cluster_pcp_handler.h | 3 + .../p2p_cluster_pcp_handler_test.cc | 52 +++++++++++++++++ connections/implementation/pcp_handler.h | 2 + connections/implementation/pcp_manager.cc | 6 ++ connections/implementation/pcp_manager.h | 2 + .../service_controller_router.cc | 8 ++- 17 files changed, 210 insertions(+), 4 deletions(-) diff --git a/connections/implementation/analytics/analytics_recorder.cc b/connections/implementation/analytics/analytics_recorder.cc index 6c518518..f11f4e30 100644 --- a/connections/implementation/analytics/analytics_recorder.cc +++ b/connections/implementation/analytics/analytics_recorder.cc @@ -224,6 +224,14 @@ void AnalyticsRecorder::OnStartedIncomingConnectionListening( } } +void AnalyticsRecorder::OnStoppedIncomingConnectionListening() { + MutexLock lock(&mutex_); + if (!CanRecordAnalyticsLocked("OnStoppedIncomingConnectionListening")) { + return; + } + RecordAdvertisingPhaseDurationLocked(); +} + void AnalyticsRecorder::OnEndpointFound(Medium medium) { MutexLock lock(&mutex_); if (!CanRecordAnalyticsLocked("OnEndpointFound")) { diff --git a/connections/implementation/analytics/analytics_recorder.h b/connections/implementation/analytics/analytics_recorder.h index 8d848052..ff425f34 100644 --- a/connections/implementation/analytics/analytics_recorder.h +++ b/connections/implementation/analytics/analytics_recorder.h @@ -60,6 +60,7 @@ class AnalyticsRecorder { // Connection listening void OnStartedIncomingConnectionListening(connections::Strategy strategy) ABSL_LOCKS_EXCLUDED(mutex_); + void OnStoppedIncomingConnectionListening() ABSL_LOCKS_EXCLUDED(mutex_); // Discovery phase void OnStartDiscovery( diff --git a/connections/implementation/analytics/analytics_recorder_test.cc b/connections/implementation/analytics/analytics_recorder_test.cc index aeb7bc78..ac56a922 100644 --- a/connections/implementation/analytics/analytics_recorder_test.cc +++ b/connections/implementation/analytics/analytics_recorder_test.cc @@ -25,6 +25,7 @@ #include "gmock/gmock.h" #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "absl/time/clock.h" #include "absl/time/time.h" #include "internal/analytics/event_logger.h" #include "internal/platform/count_down_latch.h" diff --git a/connections/implementation/base_pcp_handler.cc b/connections/implementation/base_pcp_handler.cc index 94e2e051..d0c8bb05 100644 --- a/connections/implementation/base_pcp_handler.cc +++ b/connections/implementation/base_pcp_handler.cc @@ -33,6 +33,7 @@ #include "connections/implementation/offline_frames.h" #include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/medium_selector.h" +#include "connections/status.h" #include "connections/v3/connections_device.h" #include "connections/v3/listeners.h" #include "internal/flags/nearby_flags.h" @@ -42,6 +43,7 @@ #include "internal/platform/cancelable_alarm.h" #include "internal/platform/connection_info.h" #include "internal/platform/count_down_latch.h" +#include "internal/platform/future.h" #include "internal/platform/logging.h" #include "internal/platform/wifi_lan_connection_info.h" #include "proto/connections_enums.pb.h" @@ -145,6 +147,17 @@ std::vector BasePcpHandler::GetConnectionInfoFromResult( return connection_infos; } +void BasePcpHandler::StopListeningForIncomingConnections(ClientProxy* client) { + CountDownLatch latch(1); + RunOnPcpHandlerThread("stop-listening-for-incoming-conn", + [this, client, &latch]() RUN_ON_PCP_HANDLER_THREAD() { + StopListeningForIncomingConnectionsImpl(client); + client->StoppedListeningForIncomingConnections(); + latch.CountDown(); + }); + WaitForLatch("StopListeningForIncomingConnections", &latch); +} + Status BasePcpHandler::StartAdvertising( ClientProxy* client, const std::string& service_id, const AdvertisingOptions& advertising_options, diff --git a/connections/implementation/base_pcp_handler.h b/connections/implementation/base_pcp_handler.h index d559d96d..acc75248 100644 --- a/connections/implementation/base_pcp_handler.h +++ b/connections/implementation/base_pcp_handler.h @@ -85,6 +85,8 @@ class BasePcpHandler : public PcpHandler, v3::ConnectionListeningOptions options, v3::ConnectionListener connection_listener) override; + void StopListeningForIncomingConnections(ClientProxy* client) override; + // Starts advertising. Once successfully started, changes ClientProxy's state. // Notifies ConnectionListener (info.listener) in case of any event. // See @@ -287,6 +289,9 @@ class BasePcpHandler : public PcpHandler, absl::string_view local_endpoint_id, v3::ConnectionListeningOptions options) RUN_ON_PCP_HANDLER_THREAD() = 0; + virtual void StopListeningForIncomingConnectionsImpl(ClientProxy* client) + RUN_ON_PCP_HANDLER_THREAD() = 0; + virtual Status InjectEndpointImpl(ClientProxy* client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata) diff --git a/connections/implementation/base_pcp_handler_test.cc b/connections/implementation/base_pcp_handler_test.cc index caad9334..32909af9 100644 --- a/connections/implementation/base_pcp_handler_test.cc +++ b/connections/implementation/base_pcp_handler_test.cc @@ -31,6 +31,8 @@ #include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/listeners.h" #include "connections/params.h" +#include "connections/status.h" +#include "connections/strategy.h" #include "connections/v3/connection_listening_options.h" #include "internal/platform/byte_array.h" #include "internal/platform/exception.h" @@ -159,6 +161,8 @@ class MockPcpHandler : public BasePcpHandler { absl::string_view local_endpoint_id, v3::ConnectionListeningOptions options), (override)); + MOCK_METHOD(void, StopListeningForIncomingConnectionsImpl, + (ClientProxy * client_proxy), (override)); MOCK_METHOD(Status, InjectEndpointImpl, (ClientProxy * client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata), @@ -1258,6 +1262,58 @@ TEST_F(BasePcpHandlerTest, TestStartListeningForIncomingConnectionsBadStatus) { EXPECT_FALSE(client.IsListeningForIncomingConnections()); } +TEST_F(BasePcpHandlerTest, TestCanStopListeningForIncomingConnections) { + env_.Start(); + std::string service_id{"service"}; + std::string endpoint_id{"ABCD"}; + ClientProxy client; + Mediums m; + EndpointChannelManager ecm; + EndpointManager em(&ecm); + BwuManager bwu(m, em, ecm, {}, {}); + MockPcpHandler pcp_handler(&m, &em, &ecm, &bwu); + EXPECT_CALL(pcp_handler, StartListeningForIncomingConnectionsImpl) + .Times(1) + .WillOnce(Return( + MockPcpHandler::StartOperationResult{.status = {Status::kSuccess}})); + EXPECT_CALL(pcp_handler, StopListeningForIncomingConnectionsImpl).Times(1); + v3::ConnectionListeningOptions options = {.strategy = Strategy::kP2pCluster, + .enable_ble_listening = true, + .enable_bluetooth_listening = true, + .enable_wlan_listening = true}; + pcp_handler.StartListeningForIncomingConnections(&client, service_id, options, + {}); + pcp_handler.StopListeningForIncomingConnections(&client); + EXPECT_FALSE(client.IsListeningForIncomingConnections()); +} + +TEST_F(BasePcpHandlerTest, + TestWifiLanStopListeningForIncomingConnectionsSuccessWhenStopped) { + env_.Start(); + std::string service_id{"service"}; + std::string endpoint_id{"ABCD"}; + ClientProxy client; + Mediums m; + EndpointChannelManager ecm; + EndpointManager em(&ecm); + BwuManager bwu(m, em, ecm, {}, {}); + MockPcpHandler pcp_handler(&m, &em, &ecm, &bwu); + EXPECT_CALL(pcp_handler, StartListeningForIncomingConnectionsImpl) + .Times(1) + .WillOnce(Return( + MockPcpHandler::StartOperationResult{.status = {Status::kSuccess}})); + EXPECT_CALL(pcp_handler, StopListeningForIncomingConnectionsImpl).Times(1); + v3::ConnectionListeningOptions options = {.strategy = Strategy::kP2pCluster, + .enable_ble_listening = true, + .enable_bluetooth_listening = true, + .enable_wlan_listening = true}; + pcp_handler.StartListeningForIncomingConnections(&client, service_id, options, + {}); + m.GetWifiLan().StopAcceptingConnections(service_id); + pcp_handler.StopListeningForIncomingConnections(&client); + EXPECT_FALSE(client.IsListeningForIncomingConnections()); +} + } // namespace } // namespace connections } // namespace nearby diff --git a/connections/implementation/client_proxy.cc b/connections/implementation/client_proxy.cc index 3d0e8b22..9134fd3f 100644 --- a/connections/implementation/client_proxy.cc +++ b/connections/implementation/client_proxy.cc @@ -197,6 +197,12 @@ void ClientProxy::StartedListeningForIncomingConnections( analytics_recorder_->OnStartedIncomingConnectionListening(strategy); } +void ClientProxy::StoppedListeningForIncomingConnections() { + MutexLock lock(&mutex_); + listening_info_.Clear(); + analytics_recorder_->OnStoppedIncomingConnectionListening(); +} + bool ClientProxy::IsListeningForIncomingConnections() const { MutexLock lock(&mutex_); return !listening_info_.IsEmpty(); diff --git a/connections/implementation/client_proxy.h b/connections/implementation/client_proxy.h index faa16ce8..b22174ed 100644 --- a/connections/implementation/client_proxy.h +++ b/connections/implementation/client_proxy.h @@ -98,6 +98,7 @@ class ClientProxy final { absl::string_view service_id, Strategy strategy, v3::ConnectionListener listener, const v3::ConnectionListeningOptions& options); + void StoppedListeningForIncomingConnections(); bool IsListeningForIncomingConnections() const; std::string GetListeningForIncomingConnectionsServiceId() const; diff --git a/connections/implementation/offline_service_controller.cc b/connections/implementation/offline_service_controller.cc index e0ff838a..4883a82d 100644 --- a/connections/implementation/offline_service_controller.cc +++ b/connections/implementation/offline_service_controller.cc @@ -78,6 +78,11 @@ OfflineServiceController::StartListeningForIncomingConnections( client, service_id, std::move(listener), options); } +void OfflineServiceController::StopListeningForIncomingConnections( + ClientProxy* client) { + pcp_manager_.StopListeningForIncomingConnections(client); +} + void OfflineServiceController::InjectEndpoint( ClientProxy* client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata) { diff --git a/connections/implementation/offline_service_controller.h b/connections/implementation/offline_service_controller.h index 3c0488ae..dd7e4ae7 100644 --- a/connections/implementation/offline_service_controller.h +++ b/connections/implementation/offline_service_controller.h @@ -61,9 +61,7 @@ class OfflineServiceController : public ServiceController { v3::ConnectionListener listener, const v3::ConnectionListeningOptions& options) override; - void StopListeningForIncomingConnections(ClientProxy* client) override { - // TODO(b/283823898): Implement. - } + void StopListeningForIncomingConnections(ClientProxy* client) override; Status RequestConnection( ClientProxy* client, const std::string& endpoint_id, diff --git a/connections/implementation/p2p_cluster_pcp_handler.cc b/connections/implementation/p2p_cluster_pcp_handler.cc index ecb456d5..22730132 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.cc +++ b/connections/implementation/p2p_cluster_pcp_handler.cc @@ -33,6 +33,7 @@ #include "connections/implementation/wifi_lan_endpoint_channel.h" #include "connections/medium_selector.h" #include "connections/power_level.h" +#include "connections/status.h" #include "internal/flags/nearby_flags.h" #include "internal/platform/logging.h" #include "internal/platform/nsd_service_info.h" @@ -1221,6 +1222,46 @@ P2pClusterPcpHandler::StartListeningForIncomingConnectionsImpl( .status = {Status::kSuccess}, .mediums = std::move(started_mediums)}; } +void P2pClusterPcpHandler::StopListeningForIncomingConnectionsImpl( + ClientProxy* client) { + if (wifi_lan_medium_.IsAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + if (!wifi_lan_medium_.StopAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + NEARBY_LOGS(WARNING) + << "Unable to stop wifi lan from accepting connections."; + } + } + if (bluetooth_medium_.IsAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + if (!bluetooth_medium_.StopAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + NEARBY_LOGS(WARNING) + << "Unable to stop bluetooth medium from accepting connections."; + } + } + if (NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_connections_feature::kEnableBleV2)) { + if (ble_v2_medium_.IsAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + if (!ble_v2_medium_.StopAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + NEARBY_LOGS(WARNING) + << "Unable to stop ble_v2 medium from accepting connections."; + } + } + } else { + if (ble_medium_.IsAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + if (!ble_medium_.StopAcceptingConnections( + client->GetListeningForIncomingConnectionsServiceId())) { + NEARBY_LOGS(WARNING) + << "Unable to stop ble medium from accepting connections."; + } + } + } +} + void P2pClusterPcpHandler::BluetoothConnectionAcceptedHandler( ClientProxy* client, absl::string_view local_endpoint_info, const std::string& service_id, BluetoothSocket socket) { diff --git a/connections/implementation/p2p_cluster_pcp_handler.h b/connections/implementation/p2p_cluster_pcp_handler.h index b3426bda..a8f96249 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.h +++ b/connections/implementation/p2p_cluster_pcp_handler.h @@ -98,6 +98,9 @@ class P2pClusterPcpHandler : public BasePcpHandler { absl::string_view local_endpoint_id, v3::ConnectionListeningOptions options) override; + // @PCPHandlerThread + void StopListeningForIncomingConnectionsImpl(ClientProxy* client) override; + private: // Holds the state required to re-create a BleEndpoint we see on a // BlePeripheral, so BlePeripheralLostHandler can call diff --git a/connections/implementation/p2p_cluster_pcp_handler_test.cc b/connections/implementation/p2p_cluster_pcp_handler_test.cc index b9bdff02..428b9ba7 100644 --- a/connections/implementation/p2p_cluster_pcp_handler_test.cc +++ b/connections/implementation/p2p_cluster_pcp_handler_test.cc @@ -366,6 +366,58 @@ TEST_P(P2pClusterPcpHandlerTest, CanStartListeningForIncomingConnections) { env_.Stop(); } +TEST_P(P2pClusterPcpHandlerTest, CanStopListeningForIncomingConnections) { + env_.Start(); + std::string endpoint_name_a{"endpoint_name"}; + Mediums mediums_a; + BluetoothRadio& radio_a = mediums_a.GetBluetoothRadio(); + radio_a.GetBluetoothAdapter().SetName("BT Device A"); + EndpointChannelManager ecm_a; + EndpointManager em_a(&ecm_a); + BwuManager bwu_a(mediums_a, em_a, ecm_a, {}, + {.allow_upgrade_to = {.bluetooth = true}}); + InjectedBluetoothDeviceStore ibds_a; + P2pClusterPcpHandler handler_a(&mediums_a, &em_a, &ecm_a, &bwu_a, ibds_a); + v3::ConnectionListeningOptions v3_options{ + .strategy = Strategy::kP2pCluster, + .enable_ble_listening = true, + .enable_bluetooth_listening = true, + .enable_wlan_listening = true, + }; + // make sure mediums are not accepting before calling handler. + ASSERT_FALSE( + mediums_a.GetBluetoothClassic().IsAcceptingConnections(service_id_)); + ASSERT_FALSE(mediums_a.GetWifiLan().IsAcceptingConnections(service_id_)); + if (std::get<1>(GetParam())) { + ASSERT_FALSE(mediums_a.GetBleV2().IsAcceptingConnections(service_id_)); + } else { + ASSERT_FALSE(mediums_a.GetBle().IsAcceptingConnections(service_id_)); + } + // call handler. + auto result = handler_a.StartListeningForIncomingConnections( + &client_a_, service_id_, v3_options, {}); + // now check to make sure we are in fact accepting connections. + ASSERT_TRUE( + mediums_a.GetBluetoothClassic().IsAcceptingConnections(service_id_)); + ASSERT_TRUE(mediums_a.GetWifiLan().IsAcceptingConnections(service_id_)); + if (std::get<1>(GetParam())) { + ASSERT_TRUE(mediums_a.GetBleV2().IsAcceptingConnections(service_id_)); + } else { + ASSERT_TRUE(mediums_a.GetBle().IsAcceptingConnections(service_id_)); + } + // stop. + handler_a.StopListeningForIncomingConnections(&client_a_); + EXPECT_FALSE( + mediums_a.GetBluetoothClassic().IsAcceptingConnections(service_id_)); + EXPECT_FALSE(mediums_a.GetWifiLan().IsAcceptingConnections(service_id_)); + if (std::get<1>(GetParam())) { + EXPECT_FALSE(mediums_a.GetBleV2().IsAcceptingConnections(service_id_)); + } else { + EXPECT_FALSE(mediums_a.GetBle().IsAcceptingConnections(service_id_)); + } + env_.Stop(); +} + INSTANTIATE_TEST_SUITE_P(ParametrisedPcpHandlerTest, P2pClusterPcpHandlerTest, ::testing::Combine(::testing::ValuesIn(kTestCases), ::testing::Bool())); diff --git a/connections/implementation/pcp_handler.h b/connections/implementation/pcp_handler.h index 346a2234..c346c7e0 100644 --- a/connections/implementation/pcp_handler.h +++ b/connections/implementation/pcp_handler.h @@ -94,6 +94,8 @@ class PcpHandler { v3::ConnectionListeningOptions options, v3::ConnectionListener connection_listener) = 0; + virtual void StopListeningForIncomingConnections(ClientProxy* client) = 0; + // If Discovery is active with is_out_of_band_connection == true, invoke the // callback with the provided endpoint info. virtual void InjectEndpoint(ClientProxy* client, diff --git a/connections/implementation/pcp_manager.cc b/connections/implementation/pcp_manager.cc index aa48b6d9..06ab81a4 100644 --- a/connections/implementation/pcp_manager.cc +++ b/connections/implementation/pcp_manager.cc @@ -101,6 +101,12 @@ PcpManager::StartListeningForIncomingConnections( client, service_id, options, std::move(listener))}; } +void PcpManager::StopListeningForIncomingConnections(ClientProxy* client) { + if (current_) { + current_->StopListeningForIncomingConnections(client); + } +} + void PcpManager::InjectEndpoint(ClientProxy* client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata) { diff --git a/connections/implementation/pcp_manager.h b/connections/implementation/pcp_manager.h index 15daa652..4d8a579f 100644 --- a/connections/implementation/pcp_manager.h +++ b/connections/implementation/pcp_manager.h @@ -63,6 +63,8 @@ class PcpManager { v3::ConnectionListener listener, const v3::ConnectionListeningOptions& options); + void StopListeningForIncomingConnections(ClientProxy* client); + void InjectEndpoint(ClientProxy* client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata); diff --git a/connections/implementation/service_controller_router.cc b/connections/implementation/service_controller_router.cc index ee2fc56b..87bde53b 100644 --- a/connections/implementation/service_controller_router.cc +++ b/connections/implementation/service_controller_router.cc @@ -383,7 +383,13 @@ void ServiceControllerRouter::StartListeningForIncomingConnectionsV3( void ServiceControllerRouter::StopListeningForIncomingConnectionsV3( ClientProxy* client) { - GetServiceController()->StopListeningForIncomingConnections(client); + RouteToServiceController( + "scr-stop-listening-for-incoming-connections", [this, client]() { + if (!client->IsListeningForIncomingConnections()) { + return; + } + GetServiceController()->StopListeningForIncomingConnections(client); + }); } void ServiceControllerRouter::RequestConnectionV3(