[CrOS Tether] Do not register a new Bluetooth advertisement until any previous identical advertisem…

chromeos/components/tether/ble_advertiser.cc

Index: chromeos/components/tether/ble_advertiser.cc
diff --git a/chromeos/components/tether/ble_advertiser.cc b/chromeos/components/tether/ble_advertiser.cc
index f48087e0f72170617e8a6ff5bedad514faca016c..beb1555bd43c60d43cbf256eb503bd83d838be19 100644
--- a/chromeos/components/tether/ble_advertiser.cc
+++ b/chromeos/components/tether/ble_advertiser.cc
@@ -25,12 +25,21 @@ uint8_t kInvertedConnectionFlag = 0x01;
 BleAdvertiser::IndividualAdvertisement::IndividualAdvertisement(
     const std::string& device_id,
     scoped_refptr<device::BluetoothAdapter> adapter,
-    std::unique_ptr<cryptauth::DataWithTimestamp> advertisement_data)
+    std::unique_ptr<cryptauth::DataWithTimestamp> advertisement_data,
+    const base::Closure& on_unregister_advertisement_success_callback,
+    const base::Callback<void(device::BluetoothAdvertisement::ErrorCode)>&
+        on_unregister_advertisement_error_callback,
+    std::unordered_set<std::string>* active_advertisement_device_ids_set)
     : device_id_(device_id),
       adapter_(adapter),
       advertisement_data_(std::move(advertisement_data)),
       is_initializing_advertising_(false),
       advertisement_(nullptr),
+      on_unregister_advertisement_success_callback_(
+          on_unregister_advertisement_success_callback),
+      on_unregister_advertisement_error_callback_(
+          on_unregister_advertisement_error_callback),
+      active_advertisement_device_ids_set_(active_advertisement_device_ids_set),
       weak_ptr_factory_(this) {
   adapter_->AddObserver(this);
   AdvertiseIfPossible();
@@ -38,15 +47,20 @@ BleAdvertiser::IndividualAdvertisement::IndividualAdvertisement(
 
 BleAdvertiser::IndividualAdvertisement::~IndividualAdvertisement() {
   if (advertisement_) {
-    advertisement_->Unregister(
-        base::Bind(&base::DoNothing),
-        base::Bind(&IndividualAdvertisement::OnAdvertisementUnregisterFailure,
-                   weak_ptr_factory_.GetWeakPtr()));
+    advertisement_->Unregister(on_unregister_advertisement_success_callback_,
+                               on_unregister_advertisement_error_callback_);
   }
 
   adapter_->RemoveObserver(this);
 }
 
+void BleAdvertiser::IndividualAdvertisement::
+    OnPreviousAdvertisementUnregistered() {
+  DCHECK(active_advertisement_device_ids_set_->find(device_id_) ==
+         active_advertisement_device_ids_set_->end());
+  AdvertiseIfPossible();
+}
+
 void BleAdvertiser::IndividualAdvertisement::AdapterPoweredChanged(
     device::BluetoothAdapter* adapter,
     bool powered) {
@@ -60,13 +74,21 @@ void BleAdvertiser::IndividualAdvertisement::AdvertisementReleased(
 
   // If the advertisement was released, delete it and try again. Note that this
   // situation is not expected to occur under normal circumstances.
+  advertisement_->RemoveObserver(this);
   advertisement_ = nullptr;
+  active_advertisement_device_ids_set_->erase(device_id_);
+
   AdvertiseIfPossible();
 }
 
 void BleAdvertiser::IndividualAdvertisement::AdvertiseIfPossible() {
   if (!adapter_->IsPowered() || is_initializing_advertising_ ||
-      advertisement_) {
+      advertisement_ ||
+      active_advertisement_device_ids_set_->find(device_id_) !=
+          active_advertisement_device_ids_set_->end()) {
+    // It is not possible to advertise if the adapter is not powered. Likewise,
+    // we should not try to advertise if there is an advertisement already in
+    // progress.
     return;
   }
 
@@ -90,7 +112,11 @@ void BleAdvertiser::IndividualAdvertisement::AdvertiseIfPossible() {
 void BleAdvertiser::IndividualAdvertisement::OnAdvertisementRegisteredCallback(
     scoped_refptr<device::BluetoothAdvertisement> advertisement) {
   is_initializing_advertising_ = false;
+
   advertisement_ = advertisement;
+  advertisement_->AddObserver(this);
+  active_advertisement_device_ids_set_->insert(device_id_);
+
   PA_LOG(INFO) << "Advertisement registered. "
                << "Device ID: \""
                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id_)
@@ -107,15 +133,6 @@ void BleAdvertiser::IndividualAdvertisement::OnAdvertisementErrorCallback(
                 << ", Error code: " << error_code;
 }
 
-void BleAdvertiser::IndividualAdvertisement::OnAdvertisementUnregisterFailure(
-    device::BluetoothAdvertisement::ErrorCode error_code) {
-  PA_LOG(ERROR) << "Error unregistering advertisement. "
-                << "Device ID: \""
-                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id_)
-                << "\", Service data: " << advertisement_data_->DataInHex()
-                << " Error code: " << error_code;
-}
-
 std::unique_ptr<device::BluetoothAdvertisement::UUIDList>
 BleAdvertiser::IndividualAdvertisement::CreateServiceUuids() const {
   std::unique_ptr<device::BluetoothAdvertisement::UUIDList> list =
@@ -162,11 +179,13 @@ BleAdvertiser::BleAdvertiser(
     : adapter_(adapter),
       eid_generator_(std::move(eid_generator)),
       remote_beacon_seed_fetcher_(remote_beacon_seed_fetcher),
-      local_device_data_provider_(local_device_data_provider) {}
+      local_device_data_provider_(local_device_data_provider),
+      weak_ptr_factory_(this) {}
 
 bool BleAdvertiser::StartAdvertisingToDevice(
     const cryptauth::RemoteDevice& remote_device) {
-  if (device_id_to_advertisement_map_.size() >= kMaxConcurrentAdvertisements) {
+  if (device_id_to_individual_advertisement_map_.size() >=
+      kMaxConcurrentAdvertisements) {
     PA_LOG(ERROR) << "Attempted to register a device when the maximum number "
                   << "of devices have already been registered.";
     return false;
@@ -208,15 +227,52 @@ bool BleAdvertiser::StartAdvertisingToDevice(
     return false;
   }
 
-  device_id_to_advertisement_map_[remote_device.GetDeviceId()] =
+  std::string device_id = remote_device.GetDeviceId();
+  device_id_to_individual_advertisement_map_[device_id] =
       base::MakeUnique<IndividualAdvertisement>(
-          remote_device.GetDeviceId(), adapter_, std::move(advertisement));
+          remote_device.GetDeviceId(), adapter_, std::move(advertisement),
+          base::Bind(&BleAdvertiser::OnUnregisterAdvertisementSuccess,
+                     weak_ptr_factory_.GetWeakPtr(), device_id),
+          base::Bind(&BleAdvertiser::OnUnregisterAdvertisementError,
+                     weak_ptr_factory_.GetWeakPtr(), device_id),
+          &active_advertisement_device_ids_set_);
   return true;
 }
 
 bool BleAdvertiser::StopAdvertisingToDevice(
     const cryptauth::RemoteDevice& remote_device) {
-  return device_id_to_advertisement_map_.erase(remote_device.GetDeviceId()) > 0;
+  return device_id_to_individual_advertisement_map_.erase(
+             remote_device.GetDeviceId()) > 0;
+}
+
+void BleAdvertiser::OnUnregisterAdvertisementSuccess(
+    const std::string& associated_device_id) {
+  RemoveAdvertisingDeviceIdAndRetry(associated_device_id);
+}
+
+void BleAdvertiser::OnUnregisterAdvertisementError(
+    const std::string& associated_device_id,
+    device::BluetoothAdvertisement::ErrorCode error_code) {
+  PA_LOG(ERROR) << "Error unregistering advertisement. "
+                << "Device ID: \""
+                << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(
+                       associated_device_id)
+                << "\", Error code: " << error_code;
+
+  // Even though there was an error unregistering the advertisement, remove it
+  // from the set anyway so that it is possible to try registering the
+  // advertisement again. Note that this situation is not expected to occur
+  // since unregistering an active advertisement should always succeed.
+  RemoveAdvertisingDeviceIdAndRetry(associated_device_id);
+}
+
+void BleAdvertiser::RemoveAdvertisingDeviceIdAndRetry(
+    const std::string& device_id) {
+  active_advertisement_device_ids_set_.erase(device_id);
+
+  auto it = device_id_to_individual_advertisement_map_.find(device_id);
+  if (it != device_id_to_individual_advertisement_map_.end())
+    it->second->OnPreviousAdvertisementUnregistered();
 }
 
 }  // namespace tether