From 581d232cd5c8249b2d78d61abddde9faf70f975e Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Fri, 16 Jan 2026 15:19:15 -0800 Subject: [PATCH] Deprecate EnableReadGattForExtendedAdvertisement flag. PiperOrigin-RevId: 857322636 --- .../flags/nearby_connections_feature_flags.h | 3 - .../ble/discovered_peripheral_tracker.cc | 44 ++----- .../ble/discovered_peripheral_tracker.h | 1 - .../ble/discovered_peripheral_tracker_test.cc | 110 +++--------------- 4 files changed, 26 insertions(+), 132 deletions(-) diff --git a/connections/implementation/flags/nearby_connections_feature_flags.h b/connections/implementation/flags/nearby_connections_feature_flags.h index 5f481635..dfaded1f 100644 --- a/connections/implementation/flags/nearby_connections_feature_flags.h +++ b/connections/implementation/flags/nearby_connections_feature_flags.h @@ -85,9 +85,6 @@ constexpr auto kEnablePayloadManagerToSkipChunkUpdate = // Enable/Disable payload-received-ack feature. constexpr auto kEnablePayloadReceivedAck = flags::Flag(kConfigPackage, "45425840", false); -// Enable/Disable GATT query for extended advertisement. -constexpr auto kEnableReadGattForExtendedAdvertisement = - flags::Flag(kConfigPackage, "45718229", false); // Enable/Disable safe-to-disconnect feature. constexpr auto kEnableSafeToDisconnect = flags::Flag(kConfigPackage, "45425789", false); diff --git a/connections/implementation/mediums/ble/discovered_peripheral_tracker.cc b/connections/implementation/mediums/ble/discovered_peripheral_tracker.cc index 9d21f260..be6fe13f 100644 --- a/connections/implementation/mediums/ble/discovered_peripheral_tracker.cc +++ b/connections/implementation/mediums/ble/discovered_peripheral_tracker.cc @@ -65,18 +65,10 @@ constexpr absl::Duration kAdvertisementHeaderExpiry = absl::Seconds(15); DiscoveredPeripheralTracker::DiscoveredPeripheralTracker( bool is_extended_advertisement_available) - : is_read_gatt_for_extended_advertisement_enabled_( - NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)), - is_extended_advertisement_available_( + : is_extended_advertisement_available_( is_extended_advertisement_available) { - LOG(INFO) << __func__ << ": read GATT for extended advertisement: " - << is_read_gatt_for_extended_advertisement_enabled_; executor_ = std::make_unique(kGattThreadCount); - if (is_read_gatt_for_extended_advertisement_enabled_) { - executor_->Execute([this]() { GattFetchingLoop(); }); - } + executor_->Execute([this]() { GattFetchingLoop(); }); } DiscoveredPeripheralTracker::~DiscoveredPeripheralTracker() { @@ -93,7 +85,7 @@ void DiscoveredPeripheralTracker::Shutdown() { executor_to_shutdown = std::move(executor_); } - if (is_read_gatt_for_extended_advertisement_enabled_) { + { MutexLock lock(&task_mutex_); cond_.Notify(); } @@ -328,21 +320,16 @@ bool DiscoveredPeripheralTracker::IsSkippableGattAdvertisement( BleAdvertisementHeader advertisement_header( ExtractAdvertisementHeaderBytes(advertisement_data)); - if (is_read_gatt_for_extended_advertisement_enabled_) { - if (!advertisement_header.IsValid()) { - return false; - } - - if (advertisement_header.IsSupportExtendedAdvertisement() && - (SystemClock::ElapsedRealtime() - medium_start_scanning_time_) < - kExtendedAdvertisementHeaderDelay) { - return true; - } + if (!advertisement_header.IsValid()) { return false; - } else { - return advertisement_header.IsValid() && - advertisement_header.IsSupportExtendedAdvertisement(); } + + if (advertisement_header.IsSupportExtendedAdvertisement() && + (SystemClock::ElapsedRealtime() - medium_start_scanning_time_) < + kExtendedAdvertisementHeaderDelay) { + return true; + } + return false; } void DiscoveredPeripheralTracker::ClearGattAdvertisement( @@ -771,7 +758,7 @@ void DiscoveredPeripheralTracker::HandleAdvertisementHeader( return; } - if (is_read_gatt_for_extended_advertisement_enabled_) { + { MutexLock lock(&task_mutex_); if (advertisement_header.IsSupportExtendedAdvertisement() && is_extended_advertisement_available_) { @@ -806,13 +793,6 @@ void DiscoveredPeripheralTracker::HandleAdvertisementHeader( }); } cond_.Notify(); - } else { - executor_->Execute( - [this, peripheral, advertisement_header, - advertisement_fetcher = std::move(advertisement_fetcher)]() mutable { - FetchRawAdvertisementsInThread(peripheral, advertisement_header, - std::move(advertisement_fetcher)); - }); } // Regardless of whether or not we read a new GATT advertisement, the maps diff --git a/connections/implementation/mediums/ble/discovered_peripheral_tracker.h b/connections/implementation/mediums/ble/discovered_peripheral_tracker.h index f80f987b..4b13d568 100644 --- a/connections/implementation/mediums/ble/discovered_peripheral_tracker.h +++ b/connections/implementation/mediums/ble/discovered_peripheral_tracker.h @@ -306,7 +306,6 @@ class DiscoveredPeripheralTracker { ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); Mutex mutex_; - bool is_read_gatt_for_extended_advertisement_enabled_ = true; bool is_extended_advertisement_available_; absl::Time medium_start_scanning_time_ ABSL_GUARDED_BY(mutex_) = diff --git a/connections/implementation/mediums/ble/discovered_peripheral_tracker_test.cc b/connections/implementation/mediums/ble/discovered_peripheral_tracker_test.cc index 5c3d6888..923fe4d9 100644 --- a/connections/implementation/mediums/ble/discovered_peripheral_tracker_test.cc +++ b/connections/implementation/mediums/ble/discovered_peripheral_tracker_test.cc @@ -60,6 +60,7 @@ namespace connections { namespace mediums { namespace { +using ::testing::UnorderedElementsAre; constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kFastAdvertisementServiceUuid = "FE2C"; @@ -188,24 +189,21 @@ class MockDiscoveredPeripheralCallback : public DiscoveredPeripheralCallback { class DiscoveredPeripheralTrackerTest : public testing::TestWithParam< - std::tuple> { + std::tuple> { public: void SetUp() override { NearbyFlags::GetInstance().OverrideBoolFlagValue( config_package_nearby::nearby_connections_feature::kEnableInstantOnLost, false); - bool enable_read_gatt_for_extended_advertisement = std::get<0>(GetParam()); - NearbyFlags::GetInstance().OverrideBoolFlagValue( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement, - enable_read_gatt_for_extended_advertisement); + bool is_extended_advertisement_available = std::get<0>(GetParam()); EnvironmentConfig config{ .webrtc_enabled = false, - .use_simulated_clock = enable_read_gatt_for_extended_advertisement}; + .use_simulated_clock = true, + }; MediumEnvironment::Instance().Start(config); discovered_peripheral_tracker_ = std::make_unique( - enable_read_gatt_for_extended_advertisement); + is_extended_advertisement_available); adapter_peripheral_ = std::make_unique(); adapter_central_ = std::make_unique(); ble_peripheral_ = std::make_unique(*adapter_peripheral_); @@ -1674,70 +1672,8 @@ TEST_P(DiscoveredPeripheralTrackerTest, EXPECT_EQ(GetFetchAdvertisementCallbackCount(), 2); } -TEST_P(DiscoveredPeripheralTrackerTest, - GattAdvertisementGotEarlierThanExtendedAdvertisement) { - if (!NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)) { - return; - } - - std::vector service_ids = {std::string(kServiceIdA)}; - ByteArray advertisement_hash = GenerateRandomAdvertisementHash(); - ByteArray advertisement_header_bytes = - CreateExtendedBleAdvertisementHeader(advertisement_hash, service_ids); - ByteArray advertisement_bytes = CreateBleAdvertisement( - std::string(kServiceIdA), ByteArray(std::string(kData)), - ByteArray(std::string(kDeviceToken))); - CountDownLatch found_latch(1); - CountDownLatch fetch_latch(2); - - discovered_peripheral_tracker_->StartTracking( - std::string(kServiceIdA), false, Pcp::kP2pPointToPoint, - { - .peripheral_discovered_cb = - [&found_latch](BlePeripheral peripheral, - const std::string& service_id, - const ByteArray& advertisement_bytes, - bool fast_advertisement) { - EXPECT_EQ(advertisement_bytes, ByteArray(std::string(kData))); - EXPECT_FALSE(fast_advertisement); - found_latch.CountDown(); - }, - }, - bleutils::kCopresenceServiceUuid); - - // 1. First receive a GATT advertisement data, it should be skipped. - api::ble::BleAdvertisementData advertisement_data{}; - if (!advertisement_header_bytes.Empty()) { - advertisement_data.service_data.insert( - {bleutils::kCopresenceServiceUuid, advertisement_header_bytes}); - } - - FindAdvertisement(advertisement_data, {advertisement_bytes}, fetch_latch); - - // 2. Receive extended advertisement data first. - api::ble::BleAdvertisementData extended_advertisement_data{}; - extended_advertisement_data.is_extended_advertisement = true; - extended_advertisement_data.service_data.insert( - {bleutils::kCopresenceServiceUuid, advertisement_bytes}); - - FindExtendedAdvertisement(extended_advertisement_data, fetch_latch); - - // We should receive a client callback of a peripheral discovery. - fetch_latch.Await(kWaitDuration); - EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); - EXPECT_EQ(GetFetchAdvertisementCallbackCount(), 0); -} - TEST_P(DiscoveredPeripheralTrackerTest, OnlyGattAdvertisementReceivedOnDeviceWithExtended) { - if (!NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)) { - return; - } - std::optional fake_clock = MediumEnvironment::Instance().GetSimulatedClock(); @@ -1787,12 +1723,6 @@ TEST_P(DiscoveredPeripheralTrackerTest, } TEST_P(DiscoveredPeripheralTrackerTest, SkipExpiredGattAdvertisement) { - if (!NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)) { - return; - } - std::optional fake_clock = MediumEnvironment::Instance().GetSimulatedClock(); @@ -1843,12 +1773,6 @@ TEST_P(DiscoveredPeripheralTrackerTest, SkipExpiredGattAdvertisement) { TEST_P(DiscoveredPeripheralTrackerTest, DiscoveredOnceWhenGattAndExtendedAdvertisementReceived) { - if (!NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)) { - return; - } - std::optional fake_clock = MediumEnvironment::Instance().GetSimulatedClock(); @@ -1903,12 +1827,6 @@ TEST_P(DiscoveredPeripheralTrackerTest, TEST_P(DiscoveredPeripheralTrackerTest, FindGattAdvertisementInHigherPriorityThanExtendedGattAdvertisement) { - if (!NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableReadGattForExtendedAdvertisement)) { - return; - } - std::optional fake_clock = MediumEnvironment::Instance().GetSimulatedClock(); std::vector service_ids = {std::string(kServiceIdA)}; @@ -1932,17 +1850,17 @@ TEST_P(DiscoveredPeripheralTrackerTest, ByteArray(std::string(kDeviceToken))); CountDownLatch found_latch(3); CountDownLatch fetch_latch(3); - std::vector discovered_peripheral_order; + std::vector discovered_peripheral; discovered_peripheral_tracker_->StartTracking( std::string(kServiceIdA), false, Pcp::kP2pPointToPoint, { .peripheral_discovered_cb = - [&found_latch, &discovered_peripheral_order]( + [&found_latch, &discovered_peripheral]( BlePeripheral peripheral, const std::string& service_id, const ByteArray& advertisement_bytes, bool fast_advertisement) { - discovered_peripheral_order.push_back( + discovered_peripheral.push_back( std::string(advertisement_bytes)); found_latch.CountDown(); }, @@ -1971,16 +1889,16 @@ TEST_P(DiscoveredPeripheralTrackerTest, EXPECT_TRUE(fetch_latch.Await(kWaitDuration).ok()); EXPECT_TRUE(found_latch.Await(kWaitDuration).result()); - ASSERT_EQ(discovered_peripheral_order.size(), 3); - EXPECT_EQ(discovered_peripheral_order[0], "advertisement_a"); - EXPECT_EQ(discovered_peripheral_order[1], "advertisement_c"); - EXPECT_EQ(discovered_peripheral_order[2], "advertisement_b"); + ASSERT_EQ(discovered_peripheral.size(), 3); + EXPECT_THAT(discovered_peripheral, + UnorderedElementsAre("advertisement_a", "advertisement_c", + "advertisement_b")); } INSTANTIATE_TEST_SUITE_P( DiscoveredPeripheralTrackerFlagsTest, DiscoveredPeripheralTrackerTest, ::testing::Combine( - /*kEnableReadGattForExtendedAdvertisement=*/testing::Bool())); + /*is_extended_advertisement_available=*/testing::Bool())); } // namespace