[Bluetooth] Improve the adapter's awareness of connected devices.

Inform the adapter when a new, potentially unpaired device connects.
In addition to notifying any observers of this event, this prevents a
NULL pointer dereference in the existing logic.

BUG=385493
TEST=Establish a Bluetooth connection from an unpaired device.
     chrome.bluetooth.onDeviceAdded() should be called.
R=keybuk@chromium.org

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

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

device/bluetooth/bluetooth_adapter_mac.h

@@ -75,6 +75,9 @@ class BluetoothAdapterMac : public BluetoothAdapter,
   virtual void DiscoveryStopped(BluetoothDiscoveryManagerMac* manager,
                                 bool unexpected) OVERRIDE;
 
+  // Registers that a new |device| has connected to the local host.
+  void DeviceConnected(IOBluetoothDevice* device);
+
  protected:
   // BluetoothAdapter:
   virtual void RemovePairingDelegateInternal(
@@ -98,8 +101,10 @@ class BluetoothAdapterMac : public BluetoothAdapter,
   void InitForTest(scoped_refptr<base::SequencedTaskRunner> ui_task_runner);
   void PollAdapter();
 
-  // Updates |devices_| to be consistent with |devices|.
-  void UpdateDevices(NSArray* devices);
+  // 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.
+  void UpdateDevices();
 
   std::string address_;
   std::string name_;

device/bluetooth/bluetooth_adapter_mac.mm

@@ -138,6 +138,10 @@ void BluetoothAdapterMac::CreateL2capService(
 
 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);
@@ -161,6 +165,25 @@ void BluetoothAdapterMac::DiscoveryStopped(
                     AdapterDiscoveringChanged(this, false));
 }
 
+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);
+  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();
+}
+
 void BluetoothAdapterMac::AddDiscoverySession(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
@@ -262,6 +285,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];
@@ -269,7 +293,7 @@ void BluetoothAdapterMac::PollAdapter() {
       access_timestamp == nil ||
       [recently_accessed_device_timestamp_ compare:access_timestamp] ==
           NSOrderedAscending) {
-    UpdateDevices([IOBluetoothDevice pairedDevices]);
+    UpdateDevices();
     recently_accessed_device_timestamp_.reset([access_timestamp copy]);
   }
 
@@ -280,15 +304,39 @@ void BluetoothAdapterMac::PollAdapter() {
       base::TimeDelta::FromMilliseconds(kPollIntervalMs));
 }
 
-void BluetoothAdapterMac::UpdateDevices(NSArray* devices) {
-  // TODO(armansito): This code never calls
-  // BluetoothAdapter::Observer::DeviceRemoved. It should, if a device
-  // no longer exists.
-  STLDeleteValues(&devices_);
-  for (IOBluetoothDevice* device in devices) {
+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);
-    devices_[device_address] = new BluetoothDeviceMac(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);
+  }
+
+  // Add any unpaired connected devices.
+  for (const auto& old_device : old_devices) {
+    if (!old_device.second->IsConnected())
+      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);
   }
+
+  // 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);
 }
 
 }  // namespace device

device/bluetooth/bluetooth_channel_mac.h

@@ -11,6 +11,8 @@
 
 #include "base/macros.h"
 
+@class IOBluetoothDevice;
+
 namespace device {
 
 class BluetoothSocketMac;
@@ -28,7 +30,10 @@ class BluetoothChannelMac {
 
   // Returns the Bluetooth address for the device associated with |this|
   // channel.
-  virtual std::string GetDeviceAddress() = 0;
+  std::string GetDeviceAddress();
+
+  // Returns the Bluetooth device associated with |this| channel.
+  virtual IOBluetoothDevice* GetDevice() = 0;
 
   // Returns the outgoing MTU (maximum transmission unit) for the channel.
   virtual uint16_t GetOutgoingMTU() = 0;

device/bluetooth/bluetooth_channel_mac.mm

@@ -4,7 +4,10 @@
 
 #include "device/bluetooth/bluetooth_channel_mac.h"
 
+#import <IOBluetooth/IOBluetooth.h>
+
 #include "base/logging.h"
+#include "device/bluetooth/bluetooth_device_mac.h"
 
 namespace device {
 
@@ -19,4 +22,8 @@ void BluetoothChannelMac::SetSocket(BluetoothSocketMac* socket) {
   socket_ = socket;
 }
 
+std::string BluetoothChannelMac::GetDeviceAddress() {
+  return BluetoothDeviceMac::GetDeviceAddress(GetDevice());
+}
+
 }  // namespace device

device/bluetooth/bluetooth_l2cap_channel_mac.h

@@ -38,7 +38,7 @@ class BluetoothL2capChannelMac : public BluetoothChannelMac {
 
   // BluetoothChannelMac:
   virtual void SetSocket(BluetoothSocketMac* socket) OVERRIDE;
-  virtual std::string GetDeviceAddress() OVERRIDE;
+  virtual IOBluetoothDevice* GetDevice() OVERRIDE;
   virtual uint16_t GetOutgoingMTU() OVERRIDE;
   virtual IOReturn WriteAsync(void* data,
                               uint16_t length,

device/bluetooth/bluetooth_l2cap_channel_mac.mm

@@ -115,8 +115,8 @@ void BluetoothL2capChannelMac::SetSocket(BluetoothSocketMac* socket) {
   [channel_ setDelegate:delegate_];
 }
 
-std::string BluetoothL2capChannelMac::GetDeviceAddress() {
-  return BluetoothDeviceMac::GetDeviceAddress([channel_ getDevice]);
+IOBluetoothDevice* BluetoothL2capChannelMac::GetDevice() {
+  return [channel_ getDevice];
 }
 
 uint16_t BluetoothL2capChannelMac::GetOutgoingMTU() {

device/bluetooth/bluetooth_rfcomm_channel_mac.h

@@ -38,7 +38,7 @@ class BluetoothRfcommChannelMac : public BluetoothChannelMac {
 
   // BluetoothChannelMac:
   virtual void SetSocket(BluetoothSocketMac* socket) OVERRIDE;
-  virtual std::string GetDeviceAddress() OVERRIDE;
+  virtual IOBluetoothDevice* GetDevice() OVERRIDE;
   virtual uint16_t GetOutgoingMTU() OVERRIDE;
   virtual IOReturn WriteAsync(void* data,
                               uint16_t length,

device/bluetooth/bluetooth_rfcomm_channel_mac.mm

@@ -110,8 +110,8 @@ void BluetoothRfcommChannelMac::SetSocket(BluetoothSocketMac* socket) {
   [channel_ setDelegate:delegate_];
 }
 
-std::string BluetoothRfcommChannelMac::GetDeviceAddress() {
-  return BluetoothDeviceMac::GetDeviceAddress([channel_ getDevice]);
+IOBluetoothDevice* BluetoothRfcommChannelMac::GetDevice() {
+  return [channel_ getDevice];
 }
 
 uint16_t BluetoothRfcommChannelMac::GetOutgoingMTU() {

device/bluetooth/bluetooth_socket_mac.h

@@ -29,7 +29,7 @@ class IOBufferWithSize;
 
 namespace device {
 
-class BluetoothAdapter;
+class BluetoothAdapterMac;
 class BluetoothChannelMac;
 
 // Implements the BluetoothSocket class for the Mac OS X platform.
@@ -52,7 +52,7 @@ class BluetoothSocketMac : public BluetoothSocket {
   // |success_callback| will be called if the service is successfully
   // registered, |error_callback| on failure with a message explaining the
   // cause.
-  void ListenUsingRfcomm(scoped_refptr<BluetoothAdapter> adapter,
+  void ListenUsingRfcomm(scoped_refptr<BluetoothAdapterMac> adapter,
                          const BluetoothUUID& uuid,
                          int channel_id,
                          const base::Closure& success_callback,
@@ -63,7 +63,7 @@ class BluetoothSocketMac : public BluetoothSocket {
   // |success_callback| will be called if the service is successfully
   // registered, |error_callback| on failure with a message explaining the
   // cause.
-  void ListenUsingL2cap(scoped_refptr<BluetoothAdapter> adapter,
+  void ListenUsingL2cap(scoped_refptr<BluetoothAdapterMac> adapter,
                         const BluetoothUUID& uuid,
                         int psm,
                         const base::Closure& success_callback,
@@ -155,7 +155,7 @@ class BluetoothSocketMac : public BluetoothSocket {
 
   // Adapter the socket is registered against. This is only present when the
   // socket is listening.
-  scoped_refptr<BluetoothAdapter> adapter_;
+  scoped_refptr<BluetoothAdapterMac> adapter_;
 
   // UUID of the profile being connected to, or that the socket is listening on.
   device::BluetoothUUID uuid_;

device/bluetooth/bluetooth_socket_mac.mm

@@ -22,6 +22,7 @@
 #include "base/strings/sys_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "device/bluetooth/bluetooth_adapter.h"
+#include "device/bluetooth/bluetooth_adapter_mac.h"
 #include "device/bluetooth/bluetooth_channel_mac.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_device_mac.h"
@@ -466,7 +467,7 @@ void BluetoothSocketMac::Connect(
 }
 
 void BluetoothSocketMac::ListenUsingRfcomm(
-    scoped_refptr<BluetoothAdapter> adapter,
+    scoped_refptr<BluetoothAdapterMac> adapter,
     const BluetoothUUID& uuid,
     int channel_id,
     const base::Closure& success_callback,
@@ -494,7 +495,7 @@ void BluetoothSocketMac::ListenUsingRfcomm(
 }
 
 void BluetoothSocketMac::ListenUsingL2cap(
-    scoped_refptr<BluetoothAdapter> adapter,
+    scoped_refptr<BluetoothAdapterMac> adapter,
     const BluetoothUUID& uuid,
     int psm,
     const base::Closure& success_callback,
@@ -575,7 +576,7 @@ void BluetoothSocketMac::OnSDPQueryComplete(
   }
 
   // Note: It's important to set the connect callbacks *prior* to opening the
-  // channel as the delegate is passed in and can synchronously call into
+  // channel, as opening the channel can synchronously call into
   // OnChannelOpenComplete().
   connect_callbacks_.reset(new ConnectCallbacks());
   connect_callbacks_->success_callback = success_callback;
@@ -590,6 +591,7 @@ void BluetoothSocketMac::OnSDPQueryComplete(
         BluetoothL2capChannelMac::OpenAsync(this, device, l2cap_psm, &status);
   }
   if (status != kIOReturnSuccess) {
+    ReleaseChannel();
     std::stringstream error;
     error << "Failed to connect bluetooth socket ("
           << BluetoothDeviceMac::GetDeviceAddress(device) << "): (" << status
@@ -854,8 +856,7 @@ void BluetoothSocketMac::AcceptConnectionRequest() {
   linked_ptr<BluetoothChannelMac> channel = accept_queue_.front();
   accept_queue_.pop();
 
-  // TODO(isherman): It isn't guaranteed that the adapter knows about the device
-  // at this point. Fix this logic.
+  adapter_->DeviceConnected(channel->GetDevice());
   BluetoothDevice* device = adapter_->GetDevice(channel->GetDeviceAddress());
   DCHECK(device);