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

chromeos/components/tether/ble_advertiser.h

Index: chromeos/components/tether/ble_advertiser.h
diff --git a/chromeos/components/tether/ble_advertiser.h b/chromeos/components/tether/ble_advertiser.h
index 590b86e15bbe7b294d41ce7cebfb222bb9db78ca..561515c183197baea55e91b836adc53d61eed488 100644
--- a/chromeos/components/tether/ble_advertiser.h
+++ b/chromeos/components/tether/ble_advertiser.h
@@ -6,7 +6,9 @@
 #define CHROMEOS_COMPONENTS_TETHER_BLE_ADVERTISER_H_
 
 #include <map>
+#include <unordered_set>
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/cryptauth/foreground_eid_generator.h"
@@ -44,6 +46,17 @@ class BleAdvertiser {
  private:
   friend class BleAdvertiserTest;
 
+  // One IndividualAdvertisement is created for each device to which
+  // BleAdvertiser should be advertising. When an IndividualAdvertisement is
+  // created, it starts trying to advertise to its associated device ID;
+  // likewise, when it is destroyed, it stops advertising if necessary.
+  //
+  // However, because unregistering an advertisement is an asynchronous
+  // operation, it is possible that if an IndividualAdvertisement for one device
+  // is created just after another IndividualAdvertisement for the same device
+  // was destroyed, the previous advertisement was not fully unregistered.
+  // If that is the case, the newly-created IndividualAdvertisement must wait
+  // until OnPreviousAdvertisementUnregistered() is called.
   class IndividualAdvertisement
       : public device::BluetoothAdapter::Observer,
         public device::BluetoothAdvertisement::Observer {
@@ -51,9 +64,17 @@ class BleAdvertiser {
     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_advertisement_unregistered_callback,
+        const base::Callback<void(device::BluetoothAdvertisement::ErrorCode)>&
+            on_unregister_advertisement_error_callback,
+        std::unordered_set<std::string>* active_advertisement_device_ids_set);
     ~IndividualAdvertisement() override;
 
+    // Callback for when a previously-registered advertisement corresponding to
+    // |device_id_| has been unregistered.
+    void OnPreviousAdvertisementUnregistered();
+
     // device::BluetoothAdapter::Observer
     void AdapterPoweredChanged(device::BluetoothAdapter* adapter,
                                bool powered) override;
@@ -85,6 +106,11 @@ class BleAdvertiser {
     bool is_initializing_advertising_;
     scoped_refptr<device::BluetoothAdvertisement> advertisement_;
 
+    base::Closure on_advertisement_unregistered_callback_;
+    base::Callback<void(device::BluetoothAdvertisement::ErrorCode)>
+        on_unregister_advertisement_error_callback_;
+    std::unordered_set<std::string>* active_advertisement_device_ids_set_;
+
     base::WeakPtrFactory<IndividualAdvertisement> weak_ptr_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(IndividualAdvertisement);
@@ -96,6 +122,12 @@ class BleAdvertiser {
       const cryptauth::RemoteBeaconSeedFetcher* remote_beacon_seed_fetcher,
       const cryptauth::LocalDeviceDataProvider* local_device_data_provider);
 
+  void OnAdvertisementUnregistered(const std::string& associated_device_id);

[Ryan Hansberry, 2017/07/12 16:27:10]

one more nit: make the language of the successful and failed unregistration
methods match. one suggestion: rename 'OnAdvertisementUnregistered' to
'OnUnregisterAdvertisementSuccess'

[Kyle Horimoto, 2017/07/12 17:40:30]

Done.

+  void OnUnregisterAdvertisementError(
+      const std::string& associated_device_id,
+      device::BluetoothAdvertisement::ErrorCode error_code);
+  void RemoveAdvertisingDeviceIdAndRetry(const std::string& device_id);
+
   scoped_refptr<device::BluetoothAdapter> adapter_;
 
   std::unique_ptr<cryptauth::ForegroundEidGenerator> eid_generator_;
@@ -104,7 +136,10 @@ class BleAdvertiser {
   const cryptauth::LocalDeviceDataProvider* local_device_data_provider_;
 
   std::map<std::string, std::unique_ptr<IndividualAdvertisement>>
-      device_id_to_advertisement_map_;
+      device_id_to_individual_advertisement_map_;
+  std::unordered_set<std::string> active_advertisement_device_ids_set_;
+
+  base::WeakPtrFactory<BleAdvertiser> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BleAdvertiser);
 };