diff --git a/fastpair/fast_pair_service.cc b/fastpair/fast_pair_service.cc index 57942e41..c67cb9e4 100644 --- a/fastpair/fast_pair_service.cc +++ b/fastpair/fast_pair_service.cc @@ -39,11 +39,14 @@ FastPairService::FastPairService() : FastPairService(std::make_unique()) {} FastPairService::FastPairService(std::unique_ptr repository) - : fast_pair_repository_(std::move(repository)) { + : fast_pair_repository_(std::move(repository)), + on_device_destroyed_callback_( + [this](const FastPairDevice& device) { OnDeviceDestroyed(device); }) { NearbyFlags::GetInstance().OverrideBoolFlagValue( platform::config_package_nearby::nearby_platform_feature:: kEnableBleV2Gatt, true); + devices_.AddObserver(&on_device_destroyed_callback_); seeker_ = std::make_unique( FastPairSeekerImpl::ServiceCallbacks{ .on_initial_discovery = @@ -81,8 +84,10 @@ absl::Status FastPairService::RegisterPluginProvider( absl::string_view name, std::unique_ptr provider) { Future result; executor_.Execute("register-plugin", [&]() { - bool success = - providers_.insert({std::string(name), std::move(provider)}).second; + bool success = plugin_states_ + .insert({std::string(name), + PluginState{.provider = std::move(provider)}}) + .second; absl::Status status = success ? absl::OkStatus() : absl::AlreadyExistsError(absl::StrFormat( @@ -97,7 +102,7 @@ absl::Status FastPairService::RegisterPluginProvider( absl::Status FastPairService::UnregisterPluginProvider(absl::string_view name) { Future result; executor_.Execute("unregister-plugin", [&]() { - bool success = success = providers_.erase(name); + bool success = success = plugin_states_.erase(name); absl::Status status = success ? absl::OkStatus() : absl::NotFoundError(absl::StrFormat( @@ -114,8 +119,8 @@ void FastPairService::OnInitialDiscoveryEvent(const FastPairDevice& device, executor_.Execute("on-initial-discovery", [this, device = &device, event = std::move(event)]() { NEARBY_LOGS(INFO) << "OnInitialDiscoveryEvent " << *device; - for (auto& entry : providers_) { - auto plugin = entry.second->GetPlugin(seeker_.get(), device); + for (auto& entry : plugin_states_) { + auto plugin = entry.second.GetPlugin(seeker_.get(), device); plugin->OnInitialDiscoveryEvent(event); } }); @@ -131,5 +136,22 @@ void FastPairService::OnBatteryEvent(const FastPairDevice& device, void FastPairService::OnRingEvent(const FastPairDevice& device, RingEvent event) {} +void FastPairService::OnDeviceDestroyed(const FastPairDevice& device) { + NEARBY_LOGS(INFO) << "OnDeviceDestroyed " << device; + for (auto& entry : plugin_states_) { + entry.second.plugins.erase(&device); + } +} + +FastPairPlugin* FastPairService::PluginState::GetPlugin( + FastPairSeeker* seeker, const FastPairDevice* device) { + auto it = plugins.find(device); + if (it != plugins.end()) { + return it->second.get(); + } + auto result = plugins.insert({device, provider->GetPlugin(seeker, device)}); + DCHECK(result.second); + return result.first->second.get(); +} } // namespace fastpair } // namespace nearby diff --git a/fastpair/fast_pair_service.h b/fastpair/fast_pair_service.h index 720b48b2..4b771a73 100644 --- a/fastpair/fast_pair_service.h +++ b/fastpair/fast_pair_service.h @@ -57,6 +57,14 @@ class FastPairService { FastPairSeeker* GetSeeker() const { return seeker_.get(); } private: + struct PluginState { + // Gets the plugin for `device`. Creates the plugin if it does not exist. + FastPairPlugin* GetPlugin(FastPairSeeker* seeker, + const FastPairDevice* device); + std::unique_ptr provider; + absl::flat_hash_map> + plugins; + }; void OnInitialDiscoveryEvent(const FastPairDevice& device, InitialDiscoveryEvent event); void OnSubsequentDiscoveryEvent(const FastPairDevice& device, @@ -65,12 +73,14 @@ class FastPairService { void OnScreenEvent(const FastPairDevice& device, ScreenEvent event); void OnBatteryEvent(const FastPairDevice& device, BatteryEvent event); void OnRingEvent(const FastPairDevice& device, RingEvent event); + void OnDeviceDestroyed(const FastPairDevice& device); SingleThreadExecutor executor_; std::unique_ptr seeker_; - absl::flat_hash_map> - providers_; + // Plugin name is the key. + absl::flat_hash_map plugin_states_; FastPairDeviceRepository devices_{&executor_}; std::unique_ptr fast_pair_repository_; + FastPairDeviceRepository::RemoveDeviceCallback on_device_destroyed_callback_; }; } // namespace fastpair diff --git a/fastpair/internal/BUILD b/fastpair/internal/BUILD index 17785dc7..a6b1e965 100644 --- a/fastpair/internal/BUILD +++ b/fastpair/internal/BUILD @@ -18,6 +18,7 @@ cc_library( "//fastpair/repository:device_repository", "//fastpair/scanning:scanner", "//internal/platform:types", + "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/status", "@com_google_absl//absl/strings:str_format", ], diff --git a/fastpair/repository/BUILD b/fastpair/repository/BUILD index f08a02ab..8e78b378 100644 --- a/fastpair/repository/BUILD +++ b/fastpair/repository/BUILD @@ -40,8 +40,10 @@ cc_library( visibility = ["//fastpair:__subpackages__"], deps = [ "//fastpair/common", + "//internal/base", "//internal/platform:logging", "//internal/platform:types", + "@com_google_absl//absl/functional:any_invocable", ], ) diff --git a/fastpair/repository/fast_pair_device_repository.cc b/fastpair/repository/fast_pair_device_repository.cc index dee735e2..9a88cc56 100644 --- a/fastpair/repository/fast_pair_device_repository.cc +++ b/fastpair/repository/fast_pair_device_repository.cc @@ -47,7 +47,10 @@ void FastPairDeviceRepository::RemoveDevice(const FastPairDevice* device) { // Tasks running in the background may still be referencing `device`. Defering // the destruction to the background thread should prevent use-after-free // errors. - executor_->Execute([fast_pair_device = std::move(fast_pair_device)]() { + executor_->Execute([this, fast_pair_device = std::move(fast_pair_device)]() { + for (auto* callback : observers_.GetObservers()) { + (*callback)(*fast_pair_device); + } NEARBY_LOGS(VERBOSE) << "Destroyed FP device: " << fast_pair_device; }); } diff --git a/fastpair/repository/fast_pair_device_repository.h b/fastpair/repository/fast_pair_device_repository.h index 25c31d3c..ad70bb32 100644 --- a/fastpair/repository/fast_pair_device_repository.h +++ b/fastpair/repository/fast_pair_device_repository.h @@ -19,7 +19,9 @@ #include #include +#include "absl/functional/any_invocable.h" #include "fastpair/common/fast_pair_device.h" +#include "internal/base/observer_list.h" #include "internal/platform/mutex.h" #include "internal/platform/single_thread_executor.h" @@ -29,6 +31,12 @@ namespace fastpair { // Owner of `FastPairDevice` instances. class FastPairDeviceRepository { public: + // Called on the background thread right before `device` is destroyed. + // The callbacks are not called when FastPairDeviceRepository is + // destructing. + using RemoveDeviceCallback = + absl::AnyInvocable; + explicit FastPairDeviceRepository(SingleThreadExecutor* executor) : executor_(executor) {} @@ -46,12 +54,20 @@ class FastPairDeviceRepository { // or BLE. std::optional FindDevice(absl::string_view mac_address); + void AddObserver(RemoveDeviceCallback* observer) { + observers_.AddObserver(observer); + } + void RemoveObserver(RemoveDeviceCallback* observer) { + observers_.RemoveObserver(observer); + } + private: // Removes `device` from `devices_`. std::unique_ptr ExtractDevice(const FastPairDevice* device); Mutex mutex_; SingleThreadExecutor* executor_; std::vector> devices_ ABSL_GUARDED_BY(mutex_); + ObserverList observers_; }; } // namespace fastpair diff --git a/fastpair/repository/fast_pair_device_repository_test.cc b/fastpair/repository/fast_pair_device_repository_test.cc index a9d7e839..c72437e1 100644 --- a/fastpair/repository/fast_pair_device_repository_test.cc +++ b/fastpair/repository/fast_pair_device_repository_test.cc @@ -100,6 +100,19 @@ TEST(FastPairDeviceRepositoryTest, RemovingNonRegisteredDeviceIsSafe) { EXPECT_FALSE(repo.FindDevice(kBleAddress).has_value()); } +TEST(FastPairDeviceRepositoryTest, RemovingDeviceCallsCallback) { + SingleThreadExecutor executor; + FastPairDeviceRepository repo(&executor); + FastPairDevice* device = repo.AddDevice(std::make_unique( + kModelId, kBleAddress, Protocol::kFastPairInitialPairing)); + FastPairDeviceRepository::RemoveDeviceCallback callback = + [&](const FastPairDevice& device) {}; + repo.AddObserver(&callback); + repo.RemoveDevice(device); + + EXPECT_FALSE(repo.FindDevice(kBleAddress).has_value()); +} + } // namespace } // namespace fastpair