device/bluetooth: Clean up classic discovery in BluetoothAdapterMac.

device/bluetooth/bluetooth_adapter_mac.mm

Index: device/bluetooth/bluetooth_adapter_mac.mm
diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
index 6fb2b44e6cc072fb0ad8fe01bf0dd41ee29f1d73..8971a72208264d7eb6e88f6a4f6261dc8f181e32 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -5,7 +5,6 @@
 #include "device/bluetooth/bluetooth_adapter_mac.h"
 
 #import <IOBluetooth/objc/IOBluetoothDevice.h>
-#import <IOBluetooth/objc/IOBluetoothDeviceInquiry.h>
 #import <IOBluetooth/objc/IOBluetoothHostController.h>
 
 #include <string>
@@ -33,53 +32,8 @@ MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
 - (BluetoothHCIPowerState)powerState;
 @end
 
-@protocol IOBluetoothDeviceInquiryDelegate
-- (void)deviceInquiryStarted:(IOBluetoothDeviceInquiry*)sender;
-- (void)deviceInquiryDeviceFound:(IOBluetoothDeviceInquiry*)sender
-                          device:(IOBluetoothDevice*)device;
-- (void)deviceInquiryComplete:(IOBluetoothDeviceInquiry*)sender
-                        error:(IOReturn)error
-                      aborted:(BOOL)aborted;
-@end
-
 #endif  // MAC_OS_X_VERSION_10_7
 
-@interface BluetoothAdapterMacDelegate
-    : NSObject <IOBluetoothDeviceInquiryDelegate> {
- @private
-  device::BluetoothAdapterMac* adapter_;  // weak
-}
-
-- (id)initWithAdapter:(device::BluetoothAdapterMac*)adapter;
-
-@end
-
-@implementation BluetoothAdapterMacDelegate
-
-- (id)initWithAdapter:(device::BluetoothAdapterMac*)adapter {
-  if ((self = [super init]))
-    adapter_ = adapter;
-
-  return self;
-}
-
-- (void)deviceInquiryStarted:(IOBluetoothDeviceInquiry*)sender {
-  adapter_->DeviceInquiryStarted(sender);
-}
-
-- (void)deviceInquiryDeviceFound:(IOBluetoothDeviceInquiry*)sender
-                          device:(IOBluetoothDevice*)device {
-  adapter_->DeviceFound(sender, device);
-}
-
-- (void)deviceInquiryComplete:(IOBluetoothDeviceInquiry*)sender
-                        error:(IOReturn)error
-                      aborted:(BOOL)aborted {
-  adapter_->DeviceInquiryComplete(sender, error, aborted);
-}
-
-@end
-
 namespace {
 
 const int kPollIntervalMs = 500;
@@ -104,16 +58,15 @@ base::WeakPtr<BluetoothAdapter> BluetoothAdapterMac::CreateAdapter() {
 BluetoothAdapterMac::BluetoothAdapterMac()
     : BluetoothAdapter(),
       powered_(false),
-      discovery_status_(NOT_DISCOVERING),
-      adapter_delegate_(
-          [[BluetoothAdapterMacDelegate alloc] initWithAdapter:this]),
-      device_inquiry_(
-          [[IOBluetoothDeviceInquiry
-              inquiryWithDelegate:adapter_delegate_] retain]),
+      num_discovery_sessions_(0),
+      classic_discovery_manager_(BluetoothMacDiscoveryManager::CreateClassic()),
       weak_ptr_factory_(this) {
+  DCHECK(classic_discovery_manager_.get());
+  classic_discovery_manager_->AddObserver(this);
 }
 
 BluetoothAdapterMac::~BluetoothAdapterMac() {
+  classic_discovery_manager_->RemoveObserver(this);
 }
 
 void BluetoothAdapterMac::AddObserver(BluetoothAdapter::Observer* observer) {
@@ -171,8 +124,7 @@ void BluetoothAdapterMac::SetDiscoverable(
 }
 
 bool BluetoothAdapterMac::IsDiscovering() const {
-  return discovery_status_ == DISCOVERING ||
-      discovery_status_ == DISCOVERY_STOPPING;
+  return classic_discovery_manager_->IsDiscovering();
 }
 
 void BluetoothAdapterMac::CreateRfcommService(
@@ -194,29 +146,89 @@ void BluetoothAdapterMac::CreateL2capService(
   NOTIMPLEMENTED();
 }
 
+void BluetoothAdapterMac::DeviceFound(BluetoothMacDiscoveryManager* manager,
+                                      IOBluetoothDevice* device) {
+  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::DiscoveryStopped(
+    BluetoothMacDiscoveryManager* manager,
+    bool unexpected) {
+  if (manager != classic_discovery_manager_.get()) {
+    VLOG(1) << "Received \"DiscoveryStopped\" from unknown discovery manager.";
+    return;
+  }

[Ilya Sherman, 2014/06/10 01:10:59]

Can this be a DCHECK rather than a VLOG?

[armansito, 2014/06/10 21:56:50]

Done.

+  if (unexpected) {
+    VLOG(1) << "Discovery stopped unexpectedly";

[Ilya Sherman, 2014/06/10 01:10:59]

Out of curiousity, why do we use VLOG rather than DVLOG?  In general, debug-only
logging is preferred in Chrome.  Is release logging more useful for Bluetooth
development?

[keybuk, 2014/06/10 01:13:12]

CHROME OS debug builds haven't worked for something like 14+ months, so on that
platform DVLOG is useless

[Ilya Sherman, 2014/06/10 01:14:04]

Ok, good to know -- thanks :)

[keybuk, 2014/06/10 01:21:03]

but this is _mac, so doesn't really apply, I was just explaining why arman and I
might be in the habit of using VLOG ;-)

[armansito, 2014/06/10 21:56:50]

Done.

+    num_discovery_sessions_ = 0;
+    MarkDiscoverySessionsAsInactive();
+  }
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
+                    observers_,
+                    AdapterDiscoveringChanged(this, false));
+}
+
 void BluetoothAdapterMac::AddDiscoverySession(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  if (discovery_status_ == DISCOVERING) {
-    num_discovery_listeners_++;
+  VLOG(1) << __func__;
+  if (num_discovery_sessions_ > 0) {
+    DCHECK(IsDiscovering());
+    num_discovery_sessions_++;
     callback.Run();
     return;
   }
-  on_start_discovery_callbacks_.push_back(
-      std::make_pair(callback, error_callback));
-  MaybeStartDeviceInquiry();
+
+  DCHECK(num_discovery_sessions_ == 0);

[Ilya Sherman, 2014/06/10 01:10:59]

nit: DCHECK_EQ

[armansito, 2014/06/10 21:56:50]

Done.

+
+  if (!classic_discovery_manager_->StartDiscovery()) {
+    VLOG(1) << "Failed to add a discovery session";
+    error_callback.Run();
+    return;
+  }
+
+  VLOG(1) << "Added a discovery session";
+  num_discovery_sessions_++;
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
+                    observers_,
+                    AdapterDiscoveringChanged(this, true));
+  callback.Run();
 }
 
 void BluetoothAdapterMac::RemoveDiscoverySession(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  if (discovery_status_ == NOT_DISCOVERING) {
+  VLOG(1) << __func__;
+
+  // There are active sessions other than the one currently being removed.

[Ilya Sherman, 2014/06/10 01:10:59]

nit: Please move this into the if-stmt.  Typically, comments above if-stmts are
of the form "Check for X", and inside of if-stmts are of the form "X [therefore
Y]".

[armansito, 2014/06/10 21:56:50]

Done.

+  if (num_discovery_sessions_ > 1) {
+    DCHECK(IsDiscovering());
+    num_discovery_sessions_--;
+    callback.Run();
+    return;
+  }
+
+  if (num_discovery_sessions_ == 0) {
+    VLOG(1) << "No active discovery sessions. Returning error.";
+    error_callback.Run();
+    return;
+  }
+
+  if (!classic_discovery_manager_->StopDiscovery()) {
+    VLOG(1) << "Failed to stop discovery";
     error_callback.Run();
     return;
   }
-  on_stop_discovery_callbacks_.push_back(
-      std::make_pair(callback, error_callback));
-  MaybeStopDeviceInquiry();
+
+  VLOG(1) << "Discovery stopped";
+  num_discovery_sessions_--;
+  callback.Run();
 }
 
 void BluetoothAdapterMac::RemovePairingDelegateInternal(
@@ -282,6 +294,9 @@ void BluetoothAdapterMac::PollAdapter() {
 }
 
 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) {
     std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
@@ -289,95 +304,4 @@ void BluetoothAdapterMac::UpdateDevices(NSArray* devices) {
   }
 }
 
-void BluetoothAdapterMac::DeviceInquiryStarted(
-    IOBluetoothDeviceInquiry* inquiry) {
-  DCHECK_EQ(device_inquiry_, inquiry);
-  if (discovery_status_ == DISCOVERING)
-    return;
-
-  discovery_status_ = DISCOVERING;
-  RunCallbacks(on_start_discovery_callbacks_, true);
-  num_discovery_listeners_ = on_start_discovery_callbacks_.size();
-  on_start_discovery_callbacks_.clear();
-
-  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
-                    AdapterDiscoveringChanged(this, true));
-  MaybeStopDeviceInquiry();
-}
-
-void BluetoothAdapterMac::DeviceFound(IOBluetoothDeviceInquiry* inquiry,
-                                      IOBluetoothDevice* device) {
-  DCHECK_EQ(device_inquiry_, inquiry);
-  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::DeviceInquiryComplete(
-    IOBluetoothDeviceInquiry* inquiry,
-    IOReturn error,
-    bool aborted) {
-  DCHECK_EQ(device_inquiry_, inquiry);
-  if (discovery_status_ == DISCOVERING &&
-      [device_inquiry_ start] == kIOReturnSuccess) {
-    return;
-  }
-
-  // Device discovery is done.
-  discovered_devices_.clear();
-  discovery_status_ = NOT_DISCOVERING;
-  RunCallbacks(on_stop_discovery_callbacks_, error == kIOReturnSuccess);
-  num_discovery_listeners_ = 0;
-  on_stop_discovery_callbacks_.clear();
-  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
-                    AdapterDiscoveringChanged(this, false));
-  MaybeStartDeviceInquiry();
-}
-
-void BluetoothAdapterMac::MaybeStartDeviceInquiry() {
-  if (discovery_status_ == NOT_DISCOVERING &&
-      !on_start_discovery_callbacks_.empty()) {
-    discovery_status_ = DISCOVERY_STARTING;
-    if ([device_inquiry_ start] != kIOReturnSuccess) {
-      discovery_status_ = NOT_DISCOVERING;
-      RunCallbacks(on_start_discovery_callbacks_, false);
-      on_start_discovery_callbacks_.clear();
-    }
-  }
-}
-
-void BluetoothAdapterMac::MaybeStopDeviceInquiry() {
-  if (discovery_status_ != DISCOVERING)
-    return;
-
-  if (on_stop_discovery_callbacks_.size() < num_discovery_listeners_) {
-    RunCallbacks(on_stop_discovery_callbacks_, true);
-    num_discovery_listeners_ -= on_stop_discovery_callbacks_.size();
-    on_stop_discovery_callbacks_.clear();
-    return;
-  }
-
-  discovery_status_ = DISCOVERY_STOPPING;
-  if ([device_inquiry_ stop] != kIOReturnSuccess) {
-    RunCallbacks(on_stop_discovery_callbacks_, false);
-    on_stop_discovery_callbacks_.clear();
-  }
-}
-
-void BluetoothAdapterMac::RunCallbacks(
-    const DiscoveryCallbackList& callback_list, bool success) const {
-  for (DiscoveryCallbackList::const_iterator iter = callback_list.begin();
-       iter != callback_list.end();
-       ++iter) {
-    if (success)
-      ui_task_runner_->PostTask(FROM_HERE, iter->first);
-    else
-      ui_task_runner_->PostTask(FROM_HERE, iter->second);
-  }
-}
-
 }  // namespace device