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..0ee87f42ad738464be3a68e4406a7389598e9d3f 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -44,38 +44,38 @@ MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
 
 #endif  // MAC_OS_X_VERSION_10_7
 
-@interface BluetoothAdapterMacDelegate
-    : NSObject <IOBluetoothDeviceInquiryDelegate> {
+@interface BluetoothDeviceInquiryDelegate
+    : NSObject<IOBluetoothDeviceInquiryDelegate> {
  @private
-  device::BluetoothAdapterMac* adapter_;  // weak
+  device::BluetoothMacClassicDiscoveryManager* manager_;  // weak
 }
 
-- (id)initWithAdapter:(device::BluetoothAdapterMac*)adapter;
+- (id)initWithManager:(device::BluetoothMacClassicDiscoveryManager*)manager;
 
 @end
 
-@implementation BluetoothAdapterMacDelegate
+@implementation BluetoothDeviceInquiryDelegate
 
-- (id)initWithAdapter:(device::BluetoothAdapterMac*)adapter {
+- (id)initWithManager:(device::BluetoothMacClassicDiscoveryManager*)manager {
   if ((self = [super init]))
-    adapter_ = adapter;
+    manager_ = manager;
 
   return self;
 }
 
 - (void)deviceInquiryStarted:(IOBluetoothDeviceInquiry*)sender {
-  adapter_->DeviceInquiryStarted(sender);
+  manager_->DeviceInquiryStarted(sender);
 }
 
 - (void)deviceInquiryDeviceFound:(IOBluetoothDeviceInquiry*)sender
                           device:(IOBluetoothDevice*)device {
-  adapter_->DeviceFound(sender, device);
+  manager_->DeviceFound(sender, device);
 }
 
 - (void)deviceInquiryComplete:(IOBluetoothDeviceInquiry*)sender
                         error:(IOReturn)error
                       aborted:(BOOL)aborted {
-  adapter_->DeviceInquiryComplete(sender, error, aborted);
+  manager_->DeviceInquiryComplete(sender, error, aborted);
 }
 
 @end
@@ -101,15 +101,145 @@ base::WeakPtr<BluetoothAdapter> BluetoothAdapterMac::CreateAdapter() {
   return adapter->weak_ptr_factory_.GetWeakPtr();
 }
 
+BluetoothMacClassicDiscoveryManager::BluetoothMacClassicDiscoveryManager(
+    BluetoothAdapterMac* adapter)
+    : should_do_discovery_(false),
+      inquiry_running_(false),
+      adapter_(adapter),
+      inquiry_delegate_(
+          [[BluetoothDeviceInquiryDelegate alloc] initWithManager:this]),
+      inquiry_([[IOBluetoothDeviceInquiry alloc]
+          initWithDelegate:inquiry_delegate_]) {
+}
+
+BluetoothMacClassicDiscoveryManager::~BluetoothMacClassicDiscoveryManager() {
+}
+
+bool BluetoothMacClassicDiscoveryManager::IsDiscovering() const {
+  return should_do_discovery_;
+}
+
+bool BluetoothMacClassicDiscoveryManager::StartDiscovery() {
+  VLOG(1) << "Bluetooth Classic: StartDiscovery";
+  if (should_do_discovery_) {
+    VLOG(1) << "Already discovering";
+    return true;
+  }
+
+  VLOG(1) << "Discovery requested";
+  should_do_discovery_ = true;
+
+  if (inquiry_running_) {
+    VLOG(1) << "Device inquiry already running";
+    return true;
+  }
+
+  VLOG(1) << "Requesting to start device inquiry";
+  if ([inquiry_ start] != kIOReturnSuccess) {
+    VLOG(1) << "Failed to start device inquiry";
+
+    // Set |should_do_discovery_| to false here. Since we're reporting an
+    // error, we're indicating that the adapter call StartDiscovery again
+    // if needed.
+    should_do_discovery_ = false;
+    return false;
+  }
+
+  VLOG(1) << "Device inquiry start was successful";
+  return true;
+}
+
+bool BluetoothMacClassicDiscoveryManager::StopDiscovery() {
+  VLOG(1) << "Bluetooth Classic: StopDiscovery";
+  if (!should_do_discovery_) {
+    VLOG(1) << "Discovery already stopped";
+    return true;
+  }
+
+  should_do_discovery_ = false;
+
+  if (!inquiry_running_) {
+    VLOG(1) << "No device inquiry running; discovery stopped";
+    return true;
+  }
+
+  VLOG(1) << "Requesting to stop device inquiry";
+  IOReturn status = [inquiry_ stop];
+  if (status == kIOReturnSuccess) {
+    VLOG(1) << "Device inquiry stop was successful";
+    return true;
+  }
+
+  if (status == kIOReturnNotPermitted) {
+    VLOG(1) << "Device inquiry was already stopped";
+    return true;
+  }
+
+  LOG(WARNING) << "Failed to stop device inquiry";
+  return false;
+}
+
+void BluetoothMacClassicDiscoveryManager::DeviceInquiryStarted(
+    IOBluetoothDeviceInquiry* inquiry) {
+  DCHECK(!inquiry_running_);
+
+  VLOG(1) << "Device inquiry started!";
+
+  // If discovery was requested to stop in the mean time, stop the inquiry.
+  if (!should_do_discovery_) {
+    VLOG(1) << "Discovery stop was requested earlier. Stopping inquiry";
+    [inquiry stop];
+    return;
+  }
+
+  inquiry_running_ = true;
+}
+
+void BluetoothMacClassicDiscoveryManager::DeviceFound(
+    IOBluetoothDeviceInquiry* inquiry,
+    IOBluetoothDevice* device) {
+  adapter_->DeviceFound(device);
+}
+
+void BluetoothMacClassicDiscoveryManager::DeviceInquiryComplete(
+    IOBluetoothDeviceInquiry* inquiry,
+    IOReturn error,
+    bool aborted) {
+  DCHECK_EQ(inquiry_, inquiry);
+  VLOG(1) << "Device inquiry complete";
+  inquiry_running_ = false;
+
+  // Automatically restart device inquiry if discovery is still desired.
+  if (!should_do_discovery_) {
+    adapter_->ClassicDiscoveryStopped(false /* unexpected */);
+    return;
+  }
+
+  if (error != kIOReturnSuccess) {
+    VLOG(1) << "Inquiry has stopped with an error: " << error;
+    should_do_discovery_ = false;
+    adapter_->ClassicDiscoveryStopped(true /* unexpected */);
+    return;
+  }
+
+  VLOG(1) << "Restarting device inquiry";
+
+  if ([inquiry_ start] == kIOReturnSuccess) {
+    VLOG(1) << "Device inquiry restart was successful";
+    return;
+  }
+
+  VLOG(1) << "Failed to restart discovery";
+  should_do_discovery_ = false;
+  adapter_->ClassicDiscoveryStopped(true /* unexpected */);
+}
+
 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_session_manager_(
+          new BluetoothMacClassicDiscoveryManager(this)),
       weak_ptr_factory_(this) {
 }
 
@@ -171,8 +301,7 @@ void BluetoothAdapterMac::SetDiscoverable(
 }
 
 bool BluetoothAdapterMac::IsDiscovering() const {
-  return discovery_status_ == DISCOVERING ||
-      discovery_status_ == DISCOVERY_STOPPING;
+  return classic_discovery_session_manager_->IsDiscovering();
 }
 
 void BluetoothAdapterMac::CreateRfcommService(
@@ -194,29 +323,82 @@ void BluetoothAdapterMac::CreateL2capService(
   NOTIMPLEMENTED();
 }
 
+void BluetoothAdapterMac::DeviceFound(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::ClassicDiscoveryStopped(bool unexpected) {
+  if (unexpected) {
+    VLOG(1) << "Discovery stopped unexpectedly";
+    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);
+
+  if (!classic_discovery_session_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.
+  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_session_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 +464,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 +474,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