device/bluetooth: Clean up classic discovery in BluetoothAdapterMac.

device/bluetooth/bluetooth_adapter_mac.mm

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "device/bluetooth/bluetooth_adapter_mac.h"
 
 #import <IOBluetooth/objc/IOBluetoothDevice.h>
-#import <IOBluetooth/objc/IOBluetoothDeviceInquiry.h>
 #import <IOBluetooth/objc/IOBluetoothHostController.h>
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "device/bluetooth/bluetooth_device_mac.h"
 #include "device/bluetooth/bluetooth_socket_mac.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 
 // Replicate specific 10.7 SDK declarations for building with prior SDKs.
 #if !defined(MAC_OS_X_VERSION_10_7) || \
 MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
 
 @interface IOBluetoothHostController (LionSDKDeclarations)
 - (NSString*)nameAsString;
 - (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;
 
 }  // namespace
 
 namespace device {
 
 // static
 base::WeakPtr<BluetoothAdapter> BluetoothAdapter::CreateAdapter(
     const InitCallback& init_callback) {
   return BluetoothAdapterMac::CreateAdapter();
 }
 
 // static
 base::WeakPtr<BluetoothAdapter> BluetoothAdapterMac::CreateAdapter() {
   BluetoothAdapterMac* adapter = new BluetoothAdapterMac();
   adapter->Init();
   return adapter->weak_ptr_factory_.GetWeakPtr();
 }
 
 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) {
   DCHECK(observer);
   observers_.AddObserver(observer);
 }
 
 void BluetoothAdapterMac::RemoveObserver(BluetoothAdapter::Observer* observer) {
   DCHECK(observer);
   observers_.RemoveObserver(observer);
@@ skipping 37 matching lines @@
 }
 
 void BluetoothAdapterMac::SetDiscoverable(
     bool discoverable,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   NOTIMPLEMENTED();
 }
 
 bool BluetoothAdapterMac::IsDiscovering() const {
-  return discovery_status_ == DISCOVERING ||
-      discovery_status_ == DISCOVERY_STOPPING;
+  return classic_discovery_manager_->IsDiscovering();
 }
 
 void BluetoothAdapterMac::CreateRfcommService(
     const BluetoothUUID& uuid,
     int channel,
     bool insecure,
     const CreateServiceCallback& callback,
     const CreateServiceErrorCallback& error_callback) {
   // TODO(keybuk): implement.
   NOTIMPLEMENTED();
 }
 
 void BluetoothAdapterMac::CreateL2capService(
     const BluetoothUUID& uuid,
     int psm,
     const CreateServiceCallback& callback,
     const CreateServiceErrorCallback& error_callback) {
   // TODO(keybuk): implement.
   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;
   }
-  on_stop_discovery_callbacks_.push_back(
-      std::make_pair(callback, error_callback));
-  MaybeStopDeviceInquiry();
+
+  if (!classic_discovery_manager_->StopDiscovery()) {
+    VLOG(1) << "Failed to stop discovery";
+    error_callback.Run();
+    return;
+  }
+
+  VLOG(1) << "Discovery stopped";
+  num_discovery_sessions_--;
+  callback.Run();
 }
 
 void BluetoothAdapterMac::RemovePairingDelegateInternal(
     BluetoothDevice::PairingDelegate* pairing_delegate) {
 }
 
 void BluetoothAdapterMac::Init() {
   ui_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   PollAdapter();
 }
@@ skipping 45 matching lines @@
   }
 
   ui_task_runner_->PostDelayedTask(
       FROM_HERE,
       base::Bind(&BluetoothAdapterMac::PollAdapter,
                  weak_ptr_factory_.GetWeakPtr()),
       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) {
     std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
     devices_[device_address] = new BluetoothDeviceMac(device);
   }
 }
 
-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