Simplified BluetoothAdapterMac.

Simplified BluetoothAdapterMac.

Mainly removed the complicate but unnecessary logic of updating |devices_| with paired devices. Since Bluetooth APIs do not need to listen to any of Device* observer calls for paired devices, I simplified the logic by just repopulating |devices_| when there is a change to those devices.

This is the first CL to implement Bluetooth 4.0 APIs and since there might be more changes, I'd rather implement the corresponding test in the later CLs once the shape of this file looks stable.


BUG=229636
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193313

[youngki, 2013-04-09 20:50:39]

Mark, could you review this CL for objective-c?

[Mark Mentovai, 2013-04-09 20:57:03]

LGTM

https://chromiumcodereview.appspot.com/13862020/diff/2001/device/bluetooth/bl...
File device/bluetooth/bluetooth_adapter_mac.h (right):

https://chromiumcodereview.appspot.com/13862020/diff/2001/device/bluetooth/bl...
device/bluetooth/bluetooth_adapter_mac.h:124: // Used to determine if we need to
update |devices_|.
Avoid “we.” This is clearer as

“Used to determine if |devices_| needs an update.”

https://chromiumcodereview.appspot.com/13862020/diff/2001/device/bluetooth/bl...
File device/bluetooth/bluetooth_adapter_mac.mm (right):

https://chromiumcodereview.appspot.com/13862020/diff/2001/device/bluetooth/bl...
device/bluetooth/bluetooth_adapter_mac.mm:220: IOBluetoothDevice*  recent_device
=
Extra space.

[youngki, 2013-04-09 21:01:38]

Thanks Mark for a quick review.

Scott, could you take a look?

[keybuk, 2013-04-09 21:06:00]

This doesn't seem to send any Observer methods now?

[youngki, 2013-04-09 21:16:36]

On 2013/04/09 21:06:00, keybuk wrote:
> This doesn't seem to send any Observer methods now?

It sends DeviceAdded() when a device is discovered.

From our last discussion we decided we'd call observer calls only for device
discovery and for |GetDevices()|, we'd rely on the system API and remove the
complicated logic of updating paired devices (and Windows and Mac do not really
need those observer calls during GetDevices() anyways) unless I am mistaken..?

[keybuk, 2013-04-09 21:39:32]

Could you point at that DeviceAdded() call? I'm missing it

You should probably also have DeviceRemoved() calls before clearing the list for
what was in there before?

How does the JS API discovery code learn about new devices?

[youngki, 2013-04-09 22:25:09]

On 2013/04/09 21:39:32, keybuk wrote:
> Could you point at that DeviceAdded() call? I'm missing it

https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...

> 
> You should probably also have DeviceRemoved() calls before clearing the list
for
> what was in there before?

So.. I actually prefer not to implement DeviceRemoved() because

1) For JS API, DeviceRemoved() is not used at all; the only observer method that
matters in JS API is DeviceAdded(), which is called for onDeviceDiscovered
events. So DeviceRemoved() is only used for ChromeOS UI.

2) Calling DeviceRemoved() requires caching devices discovered during device
discovery, which means it requires one more array to maintain. I prefer not to
add a data structure / logic for the features that are not used in the system.

> 
> How does the JS API discovery code learn about new devices?

The only BluetoothAdapterObserver is BluetoothEventRouter, where it only
implements DeviceAdded:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

this method dispatches the "hidden" chrome event:
"bluetooth.onDeviceDiscovered".
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

And the only JS API that listens to bluetooth.onDeviceDiscovered is
startDiscovery:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...

[keybuk, 2013-04-09 22:28:49]

lgtm

[youngki, 2013-04-09 23:41:35]

Thanks for the review. Putting this into CQ now.

[commit-bot: I haz the power, 2013-04-09 23:41:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/youngki@chromium.org/13862020/7001

[commit-bot: I haz the power, 2013-04-10 03:39:31]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-04-10 03:41:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/youngki@chromium.org/13862020/7001

[commit-bot: I haz the power, 2013-04-10 04:07:44]

Change committed as 193313