Notify chrome.bluetooth.onDeviceRemoved listeners when Classic devices are removed

BUG=387301
TEST=
  (1) Add listeners for chrome.bluetooth.onDeviceAdded and
      chrome.bluetooth.onDeviceRemoved.
  (2) Start discovery via chrome.bluetooth.startDiscovery().
  (3) Make a nearby device discoverable.
  (4) Wait until the device added notification is called for that device.
  (3) Make the device no longer be discoverable.
  (6) Wait three minutes.
  (7) Make sure the device removed notification is called for that device.
R=keybuk@chromium.org

Review URL: https://codereview.chromium.org/332343004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280958 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 73 insertions, 93 deletions

diff --git a/device/bluetooth/bluetooth_adapter_mac.h b/device/bluetooth/bluetooth_adapter_mac.h
index 7040932..7ed5096 100644
--- a/device/bluetooth/bluetooth_adapter_mac.h
+++ b/device/bluetooth/bluetooth_adapter_mac.h
@@ -70,10 +70,8 @@ class BluetoothAdapterMac : public BluetoothAdapter,
       const CreateServiceErrorCallback& error_callback) OVERRIDE;
 
   // BluetoothDiscoveryManagerMac::Observer overrides
-  virtual void DeviceFound(BluetoothDiscoveryManagerMac* manager,
-                           IOBluetoothDevice* device) OVERRIDE;
-  virtual void DiscoveryStopped(BluetoothDiscoveryManagerMac* manager,
-                                bool unexpected) OVERRIDE;
+  virtual void DeviceFound(IOBluetoothDevice* device) OVERRIDE;
+  virtual void DiscoveryStopped(bool unexpected) OVERRIDE;
 
   // Registers that a new |device| has connected to the local host.
   void DeviceConnected(IOBluetoothDevice* device);
@@ -101,6 +99,10 @@ class BluetoothAdapterMac : public BluetoothAdapter,
   void InitForTest(scoped_refptr<base::SequencedTaskRunner> ui_task_runner);
   void PollAdapter();
 
+  // Registers that a new |device| has replied to an Inquiry, is paired, or has
+  // connected to the local host.
+  void DeviceAdded(IOBluetoothDevice* device);
+
   // Updates |devices_| to include the currently paired devices, as well as any
   // connected, but unpaired, devices. Notifies observers if any previously
   // paired or connected devices are no longer present.
@@ -115,15 +117,6 @@ class BluetoothAdapterMac : public BluetoothAdapter,
   // Discovery manager for Bluetooth Classic.
   scoped_ptr<BluetoothDiscoveryManagerMac> classic_discovery_manager_;
 
-  // A list of discovered device addresses.
-  // This list is used to check if the same device is discovered twice during
-  // the discovery between consecutive inquiries.
-  base::hash_set<std::string> discovered_devices_;
-
-  // Timestamp for the recently accessed device.
-  // Used to determine if |devices_| needs an update.
-  base::scoped_nsobject<NSDate> recently_accessed_device_timestamp_;
-
   scoped_refptr<base::SequencedTaskRunner> ui_task_runner_;
 
   // List of observers interested in event notifications from us.
diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
index d44a0f3..5273ade 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -26,8 +26,13 @@
 
 namespace {
 
+// The frequency with which to poll the adapter for updates.
 const int kPollIntervalMs = 500;
 
+// The length of time that must elapse since the last Inquiry response before a
+// discovered Classic device is considered to be no longer available.
+const NSTimeInterval kDiscoveryTimeoutSec = 3 * 60;  // 3 minutes
+
 }  // namespace
 
 namespace device {
@@ -136,25 +141,11 @@ void BluetoothAdapterMac::CreateL2capService(
       this, uuid, psm, base::Bind(callback, socket), error_callback);
 }
 
-void BluetoothAdapterMac::DeviceFound(BluetoothDiscoveryManagerMac* manager,
-                                      IOBluetoothDevice* device) {
-  // TODO(isherman): The list of discovered devices is never reset. This should
-  // probably key off of |devices_| instead. Currently, if a device is paired,
-  // then unpaired, then paired again, the app would never hear about the second
-  // pairing.
-  std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
-  if (discovered_devices_.find(device_address) == discovered_devices_.end()) {
-    BluetoothDeviceMac device_mac(device);
-    FOR_EACH_OBSERVER(
-        BluetoothAdapter::Observer, observers_, DeviceAdded(this, &device_mac));
-    discovered_devices_.insert(device_address);
-  }
+void BluetoothAdapterMac::DeviceFound(IOBluetoothDevice* device) {
+  DeviceAdded(device);
 }
 
-void BluetoothAdapterMac::DiscoveryStopped(
-    BluetoothDiscoveryManagerMac* manager,
-    bool unexpected) {
-  DCHECK_EQ(manager, classic_discovery_manager_.get());
+void BluetoothAdapterMac::DiscoveryStopped(bool unexpected) {
   if (unexpected) {
     DVLOG(1) << "Discovery stopped unexpectedly";
     num_discovery_sessions_ = 0;
@@ -166,22 +157,11 @@ void BluetoothAdapterMac::DiscoveryStopped(
 }
 
 void BluetoothAdapterMac::DeviceConnected(IOBluetoothDevice* device) {
-  // TODO(isherman): Call -registerForDisconnectNotification:selector:, and
-  // investigate whether this method can be replaced with a call to
-  // +registerForConnectNotifications:selector:.
-  std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
+  // TODO(isherman): Investigate whether this method can be replaced with a call
+  // to +registerForConnectNotifications:selector:.
   DVLOG(1) << "Adapter registered a new connection from device with address: "
-           << device_address;
-
-  // Only notify once per device.
-  if (devices_.count(device_address))
-    return;
-
-  scoped_ptr<BluetoothDeviceMac> device_mac(new BluetoothDeviceMac(device));
-  FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
-                    observers_,
-                    DeviceAdded(this, device_mac.get()));
-  devices_[device_address] = device_mac.release();
+           << BluetoothDeviceMac::GetDeviceAddress(device);
+  DeviceAdded(device);
 }
 
 void BluetoothAdapterMac::AddDiscoverySession(
@@ -285,17 +265,7 @@ void BluetoothAdapterMac::PollAdapter() {
                       AdapterPoweredChanged(this, powered_));
   }
 
-  // TODO(isherman): This doesn't detect when a device is unpaired.
-  IOBluetoothDevice* recent_device =
-      [[IOBluetoothDevice recentDevices:1] lastObject];
-  NSDate* access_timestamp = [recent_device recentAccessDate];
-  if (recently_accessed_device_timestamp_ == nil ||
-      access_timestamp == nil ||
-      [recently_accessed_device_timestamp_ compare:access_timestamp] ==
-          NSOrderedAscending) {
-    UpdateDevices();
-    recently_accessed_device_timestamp_.reset([access_timestamp copy]);
-  }
+  UpdateDevices();
 
   ui_task_runner_->PostDelayedTask(
       FROM_HERE,
@@ -304,39 +274,50 @@ void BluetoothAdapterMac::PollAdapter() {
       base::TimeDelta::FromMilliseconds(kPollIntervalMs));
 }
 
+void BluetoothAdapterMac::DeviceAdded(IOBluetoothDevice* device) {
+  std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
+
+  // Only notify observers once per device.
+  if (devices_.count(device_address))
+    return;
+
+  devices_[device_address] = new BluetoothDeviceMac(device);
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
+                    observers_,
+                    DeviceAdded(this, devices_[device_address]));
+}
+
 void BluetoothAdapterMac::UpdateDevices() {
-  // Snapshot the devices observers were previously notified of.
-  // Note that the code below is careful to take ownership of any values that
-  // are erased from the map, since the map owns the memory for all its mapped
-  // devices.
-  DevicesMap old_devices = devices_;
-
-  // Add all the paired devices.
-  devices_.clear();
-  for (IOBluetoothDevice* device in [IOBluetoothDevice pairedDevices]) {
-    std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
-    scoped_ptr<BluetoothDevice> device_mac(old_devices[device_address]);
-    if (!device_mac)
-      device_mac.reset(new BluetoothDeviceMac(device));
-    devices_[device_address] = device_mac.release();
-    old_devices.erase(device_address);
-  }
+  // Notify observers if any previously seen devices are no longer available,
+  // i.e. if they are no longer paired, connected, nor recently discovered via
+  // an inquiry.
+  std::set<std::string> removed_devices;
+  for (DevicesMap::iterator it = devices_.begin(); it != devices_.end(); ++it) {
+    BluetoothDevice* device = it->second;
+    if (device->IsPaired() || device->IsConnected())
+      continue;
 
-  // Add any unpaired connected devices.
-  for (const auto& old_device : old_devices) {
-    if (!old_device.second->IsConnected())
+    NSDate* last_inquiry_update =
+        static_cast<BluetoothDeviceMac*>(device)->GetLastInquiryUpdate();
+    if (last_inquiry_update &&
+        -[last_inquiry_update timeIntervalSinceNow] < kDiscoveryTimeoutSec)
       continue;
 
-    const std::string& device_address = old_device.first;
-    DCHECK(!devices_.count(device_address));
-    devices_[device_address] = old_device.second;
-    old_devices.erase(device_address);
+    FOR_EACH_OBSERVER(
+        BluetoothAdapter::Observer, observers_, DeviceRemoved(this, device));
+    delete device;
+    removed_devices.insert(it->first);
+    // The device will be erased from the map in the loop immediately below.
+  }
+  for (const std::string& device_address : removed_devices) {
+    size_t num_removed = devices_.erase(device_address);
+    DCHECK_EQ(num_removed, 1U);
   }
 
-  // TODO(isherman): Notify observers of any devices that are no longer in
-  // range. Note that it's possible for a device to be neither paired nor
-  // connected, but to still be in range.
-  STLDeleteValues(&old_devices);
+  // Add any new paired devices.
+  for (IOBluetoothDevice* device in [IOBluetoothDevice pairedDevices]) {
+    DeviceAdded(device);
+  }
 }
 
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_device_mac.h b/device/bluetooth/bluetooth_device_mac.h
index 462cd35..68380c2 100644
--- a/device/bluetooth/bluetooth_device_mac.h
+++ b/device/bluetooth/bluetooth_device_mac.h
@@ -69,6 +69,10 @@ class BluetoothDeviceMac : public BluetoothDevice {
       const base::Closure& callback,
       const ErrorCallback& error_callback) OVERRIDE;
 
+  // Returns the timestamp when the device was last seen during an inquiry.
+  // Returns nil if the device has never been seen during an inquiry.
+  NSDate* GetLastInquiryUpdate();
+
   // Returns the Bluetooth address for the |device|. The returned address has a
   // normalized format (see below).
   static std::string GetDeviceAddress(IOBluetoothDevice* device);
diff --git a/device/bluetooth/bluetooth_device_mac.mm b/device/bluetooth/bluetooth_device_mac.mm
index 43d9fb6..a913300 100644
--- a/device/bluetooth/bluetooth_device_mac.mm
+++ b/device/bluetooth/bluetooth_device_mac.mm
@@ -232,6 +232,10 @@ void BluetoothDeviceMac::StartConnectionMonitor(
   NOTIMPLEMENTED();
 }
 
+NSDate* BluetoothDeviceMac::GetLastInquiryUpdate() {
+  return [device_ getLastInquiryUpdate];
+}
+
 int BluetoothDeviceMac::GetHostTransmitPower(
     BluetoothHCITransmitPowerLevelType power_level_type) const {
   IOBluetoothHostController* controller =
diff --git a/device/bluetooth/bluetooth_discovery_manager_mac.h b/device/bluetooth/bluetooth_discovery_manager_mac.h
index 575a033..534646a 100644
--- a/device/bluetooth/bluetooth_discovery_manager_mac.h
+++ b/device/bluetooth/bluetooth_discovery_manager_mac.h
@@ -19,17 +19,15 @@ class BluetoothDiscoveryManagerMac {
   // Interface for being notified of events during a device discovery session.
   class Observer {
    public:
-    // Called when |manager| has found a device through classic device inquiry
-    // in the form of a IOBluetoothDevice.
-    virtual void DeviceFound(BluetoothDiscoveryManagerMac* manager,
-                             IOBluetoothDevice* device) {}
+    // Called when |this| manager has found a device through classic device
+    // inquiry in the form of an IOBluetoothDevice.
+    virtual void DeviceFound(IOBluetoothDevice* device) = 0;
 
     // Called when device discovery is no longer running, due to either a call
     // to BluetoothDiscoveryManagerMac::StopDiscovery or an unexpected reason,
     // such as when a user disables the controller, in which case the value of
     // |unexpected| will be true.
-    virtual void DiscoveryStopped(BluetoothDiscoveryManagerMac* manager,
-                                  bool unexpected) {}
+    virtual void DiscoveryStopped(bool unexpected) = 0;
   };
 
   virtual ~BluetoothDiscoveryManagerMac();
diff --git a/device/bluetooth/bluetooth_discovery_manager_mac.mm b/device/bluetooth/bluetooth_discovery_manager_mac.mm
index 528e22d..2262891 100644
--- a/device/bluetooth/bluetooth_discovery_manager_mac.mm
+++ b/device/bluetooth/bluetooth_discovery_manager_mac.mm
@@ -124,7 +124,7 @@ class BluetoothDiscoveryManagerMacClassic
   void DeviceFound(IOBluetoothDeviceInquiry* inquiry,
                    IOBluetoothDevice* device) {
     DCHECK(observer_);
-    observer_->DeviceFound(this, device);
+    observer_->DeviceFound(device);
   }
 
   void DeviceInquiryComplete(IOBluetoothDeviceInquiry* inquiry,
@@ -138,7 +138,7 @@ class BluetoothDiscoveryManagerMacClassic
     // If discovery is no longer desired, notify observers that discovery
     // has stopped and return.
     if (!should_do_discovery_) {
-      observer_->DiscoveryStopped(this, false /* unexpected */);
+      observer_->DiscoveryStopped(false /* unexpected */);
       return;
     }
 
@@ -147,7 +147,7 @@ class BluetoothDiscoveryManagerMacClassic
     if (error != kIOReturnSuccess) {
       DVLOG(1) << "Inquiry has stopped with an error: " << error;
       should_do_discovery_ = false;
-      observer_->DiscoveryStopped(this, true /* unexpected */);
+      observer_->DiscoveryStopped(true /* unexpected */);
       return;
     }
 
@@ -161,7 +161,7 @@ class BluetoothDiscoveryManagerMacClassic
     DVLOG(1) << "Failed to restart discovery";
     should_do_discovery_ = false;
     DCHECK(observer_);
-    observer_->DiscoveryStopped(this, true /* unexpected */);
+    observer_->DiscoveryStopped(true /* unexpected */);
   }
 
  private: