Added bluetooth LE support on Mac platform

device/bluetooth/bluetooth_low_energy_discovery_manager_mac.mm

+#include "device/bluetooth/bluetooth_low_energy_discovery_manager_mac.h"

[armansito, 2015/01/24 17:59:38]

Is this file missing the copyright header?

[dvh, 2015/01/27 00:14:55]

Done.

+
+#include "base/mac/mac_util.h"
+
+#include "device/bluetooth/bluetooth_low_energy_device_mac.h"
+
+using namespace device;
+
+namespace device {
+
+// This class is a helper to call some protected methods in
+// BluetoothLowEnergyDiscoverManagerMac.
+class BluetoothLowEnergyDiscoverManagerMacDelegate {
+ public:
+  BluetoothLowEnergyDiscoverManagerMacDelegate(
+      BluetoothLowEnergyDiscoverManagerMac *manager)
+      : manager_(manager) {}
+
+  virtual ~BluetoothLowEnergyDiscoverManagerMacDelegate() {}
+
+  virtual void DiscoveredPeripheral(CBPeripheral *peripheral,

[armansito, 2015/01/24 17:59:38]

nit: This should read "CBPeripheral* peripheral" (no space before '*') per
style-guide. Here and everywhere else.

[dvh, 2015/01/27 00:14:56]

Done.

+                                    NSDictionary *advertisementData, int rssi) {
+    manager_->DiscoveredPeripheral(peripheral, advertisementData, rssi);
+  }
+
+  virtual void TryStartDiscovery() { manager_->TryStartDiscovery(); }
+
+ private:
+  BluetoothLowEnergyDiscoverManagerMac *manager_;
+};
+
+}  // namespace device
+
+// This class will serve as the Objective-C delegate of CBCentralManager.
+@interface BluetoothLowEnergyDiscoverManagerMacBridge
+    : NSObject<CBCentralManagerDelegate>
+
+- (id)initWithManager:(BluetoothLowEnergyDiscoverManagerMac *)manager;
+
+@end
+
+@implementation BluetoothLowEnergyDiscoverManagerMacBridge {
+  BluetoothLowEnergyDiscoverManagerMac *manager_;
+  BluetoothLowEnergyDiscoverManagerMacDelegate *delegate_;
+}
+
+- (id)initWithManager:(BluetoothLowEnergyDiscoverManagerMac *)manager {
+  self = [super init];

[armansito, 2015/01/24 17:59:38]

nit: check if init worked here (i.e. set the members inside if (self) { ... })

[dvh, 2015/01/27 00:14:56]

Done.

+  manager_ = manager;
+  delegate_ = new BluetoothLowEnergyDiscoverManagerMacDelegate(manager_);
+  return self;
+}
+
+- (void)dealloc {
+  delete delegate_;
+  [super dealloc];
+}
+
+- (void)centralManager:(CBCentralManager *)central
+    didDiscoverPeripheral:(CBPeripheral *)peripheral
+        advertisementData:(NSDictionary *)advertisementData
+                     RSSI:(NSNumber *)RSSI {
+  // Notifies the discovery of a device.
+  delegate_->DiscoveredPeripheral(peripheral, advertisementData,
+                                  [RSSI intValue]);
+}
+
+- (void)centralManagerDidUpdateState:(CBCentralManager *)central {
+  // Notifies when the powered state of the central manager changed.
+  delegate_->TryStartDiscovery();
+}
+
+@end
+
+BluetoothLowEnergyDiscoverManagerMac::~BluetoothLowEnergyDiscoverManagerMac() {
+  ClearDevices();
+}
+
+bool BluetoothLowEnergyDiscoverManagerMac::IsDiscovering() const {
+  return discovering_;
+}
+
+void BluetoothLowEnergyDiscoverManagerMac::StartDiscovery(
+    BluetoothDevice::UUIDList services_uuids) {
+  ClearDevices();
+  discovering_ = true;
+  pending_ = true;
+  services_uuids_ = services_uuids;
+  TryStartDiscovery();
+}
+
+void BluetoothLowEnergyDiscoverManagerMac::TryStartDiscovery() {
+  if (!discovering_) {
+    return;
+  }
+  if (!pending_) {
+    return;
+  }
+  // Can only start if the bluetooth power is turned on.
+  if ([manager_ state] != CBCentralManagerStatePoweredOn) {
+    return;
+  }

[armansito, 2015/01/24 17:59:38]

Please add new lines where you can to make the code more readable. Here and
elsewhere.

[dvh, 2015/01/27 00:14:56]

Done.

+  // Converts the services UUIDs to a CoreBluetooth data structure.
+  NSMutableArray *services = nil;
+  if (services_uuids_.size() > 0) {

[armansito, 2015/01/24 17:59:38]

nit: if (!services_uuids_.empty()) {

[dvh, 2015/01/27 00:14:55]

Done.

+    services = [NSMutableArray array];
+    for (unsigned int i = 0; i < services_uuids_.size(); i++) {
+      NSString *uuidString = [NSString
+          stringWithUTF8String:services_uuids_[i].canonical_value().c_str()];
+      CBUUID *uuid = [CBUUID UUIDWithString:uuidString];
+      [services addObject:uuid];
+    }
+  }
+  [manager_ scanForPeripheralsWithServices:services options:nil];
+  pending_ = false;
+}
+
+void BluetoothLowEnergyDiscoverManagerMac::StopDiscovery() {
+  if (!pending_) {
+    [manager_ stopScan];

[armansito, 2015/01/24 17:59:38]

Don't call this if |discovering_| is true?

[dvh, 2015/01/27 00:14:55]

Done.

+  }
+  discovering_ = false;
+}
+
+void BluetoothLowEnergyDiscoverManagerMac::DiscoveredPeripheral(
+    CBPeripheral *peripheral, NSDictionary *advertisementData, int rssi) {
+  BluetoothLowEnergyDeviceMac *device =

[armansito, 2015/01/24 17:59:38]

It seems like this is being unnecessarily created and deleted if there's only an
update for an existing device. Move this into the corresponding if-block.

[dvh, 2015/01/27 00:14:55]

The main goal here is to be able to lookup any existing device using
device->GetIdentifier().
Do you have something in mind about how to improve this?

[armansito, 2015/01/29 04:37:18]

It seems strange to me to construct a whole object just to get its identifier,
which you then conditionally delete later. Since you're mostly interested in the
identifier and you already have a pointer to the CBPeripheral, maybe its better
to just define a helper that does the conversion and then call that same helper
from BluetoothLowEnergyDeviceMac::GetIdentifier()?

+      new BluetoothLowEnergyDeviceMac(peripheral, advertisementData, rssi);
+  // Look for existing device.
+  std::map<const std::string, BluetoothLowEnergyDeviceMac *>::iterator iter =
+      devices_.find(device->GetIdentifier());
+  if (iter != devices_.end()) {

[armansito, 2015/01/24 17:59:38]

Do an early return and get rid of the else. I'd actually turn this if statement
around, so

if (iter == device_.end()) {
  BluetoothLowEnergyDeviceMac* device = new BluetoothLowE...;
  devices_.insert(...);
  observer_->DeviceFound(device);
  return;
}

BluetoothLowEnergyDeviceMac* old_device = ...;
...

[dvh, 2015/01/27 00:14:55]

Done.

+    // A device has an update.
+    delete device;
+    BluetoothLowEnergyDeviceMac *old_device = iter->second;
+    old_device->Update(peripheral, advertisementData, rssi);
+    observer_->DeviceUpdated(old_device);
+  } else {
+    // A device has been added.
+    devices_.insert(devices_.begin(),
+                    std::pair<const std::string, BluetoothLowEnergyDeviceMac *>(
+                        device->GetIdentifier(), device));
+    observer_->DeviceFound(device);
+  }
+}
+
+BluetoothLowEnergyDiscoverManagerMac *
+BluetoothLowEnergyDiscoverManagerMac::Create(Observer *observer) {
+  return new BluetoothLowEnergyDiscoverManagerMac(observer);
+}
+
+BluetoothLowEnergyDiscoverManagerMac::BluetoothLowEnergyDiscoverManagerMac(
+    Observer *observer)
+    : observer_(observer) {
+  bridge_.reset([[BluetoothLowEnergyDiscoverManagerMacBridge alloc]
+      initWithManager:this]);
+  // Since CoreBluetooth is only available on OS X 10.7 or later, we
+  // instantiate CBCentralManager only for OS X >= 10.7.
+  if (base::mac::IsOSLionOrLater()) {
+    manager_.reset(
+        [[CBCentralManager alloc] initWithDelegate:bridge_

[armansito, 2015/01/24 17:59:38]

Will this code compile at all in older versions? Will we need to add something
to base/mac/sdk_forward_declarations.*?

[dvh, 2015/01/27 00:14:56]

The build system of Chromium will build using a recent enough version of the Mac
OS X SDK.
Even if the build is running on old version of Mac OS X, it will still build.

Do we need to support old version of the Mac OS X SDK?

[armansito, 2015/01/29 04:37:18]

I don't directly work on Chrome for Mac, so as long as you're confident that
this won't be built on older versions I'm OK with it. Maybe a get a reviewer
from that side confirm this as well?

[groby-ooo-7-16, 2015/01/30 15:10:32]

Unless I'm completely misinformed, we're currently building against the 10.6
SDK. See http://crbug.com/335325 for the ongoing effort to move away from that.

+                                             queue:dispatch_get_main_queue()]);
+  }
+  discovering_ = false;
+}
+
+void BluetoothLowEnergyDiscoverManagerMac::ClearDevices() {
+  for (std::map<const std::string, BluetoothLowEnergyDeviceMac *>::iterator
+           iter = devices_.begin();
+       iter != devices_.end(); ++iter) {
+    delete iter->second;
+  }
+  devices_.clear();
+}