Bluetooth: add Device events, and cleanup JS API

Bluetooth: add Device events, and cleanup JS API

The JavaScript API for getting the set of devices known to the adapter
and dealing with discovery was difficult to use, and had a messy
implementation. By streamlining the API not only does it become much
easier to work with, but the implementation becomes much cleaner as
well.

chrome.bluetooth.getDevices() now simply returns an array of devices in
its callback, it's up to the application to filter these since profile
filtering is unreliable during discovery and on Low Energy.

chrome.bluetooth.startDiscovery() now only has a callback to indicate
a success to the command, the hidden event and listener has been
removed in favor of the new device events.

Add chrome.bluetooth.onDeviceAdded, chrome.bluetooth.onDeviceChanged
and chrome.bluetooth.onDeviceRemoved events. These return updated
device objects when they are first known, changed and removed
respectively.

Take the opportunity of writing a proper documentation page for these
new methods and events showing their usage.

BUG=345050
TEST=BluetoothApiTest updated and included
R=armansito@chromium.org, miket@chromium.org, rpaquay@chromium.org, sky@chromium.org

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

[keybuk, 2014-03-05 20:49:43]

miket: for /extensions/
rpaquay: for /extensions/api/bluetooth/
mkearney: for /docs/
armansito: general bluetooth review

[miket_OOO, 2014-03-05 21:30:08]

LGTM. Thanks!

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth.idl (right):

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:186: // Get a list of bluetooth
devices known to the system, including paired
Because this will become a part of public docs, style matters more. Please
capitalize Bluetooth for consistency & correctness.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:239: // onDeviceAdded event,
previously discovered devices already known to
delete comma, start new sentence.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:261: // Fired when information about
a new Bluetooth Device is available.
lowercase device. If you meant Device as in the class name, then Bluetooth is
redundant.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:268: // out of range for long enough
to be considered unavailable again, and
This would be more useful and readable if the more common case were listed
first. I'll defer to you because you're the domain expert, but I'm guessing
device removal is the more common case.

[armansito, 2014-03-05 21:44:14]

lgtm with nits.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_api.cc (left):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_api.cc:305:
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
Keep this assertion?

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc:394: //
StopDiscovery success will remove the session object, unreferncing the
s/unreferncing/unreferencing/

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/templates/articles/app_bluetooth.html
(right):

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/templates/articles/app_bluetooth.html:69: Finally
the <code>chrome.bluetooth.onDeviceRemoved</code>code> event is sent whenever a
paired device is removed from the system, or a discovered device
Remove the stray tag there (i.e. "</code>code>" -> "</code>")

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/templates/articles/app_bluetooth.html:83:
<code>chrome.bluetooth.startDiscovery</code>code> method, discovery can be
resource intensive so you should call
"</code>code>" -> "</code>"

[rpaquay, 2014-03-06 00:11:19]

lgtm with nits.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router.cc:357:
scoped_ptr<base::ListValue> args(new base::ListValue());
You should be able to use auto-generated code, i.e. 
scoped_ptr<base::ListValue> args =
bluetooth::OnDeviceChanged::Create(extension_device);

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h:128:
device::BluetoothDevice* device) OVERRIDE;
nit: missing space

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
File chrome/browser/extensions/event_names.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
chrome/browser/extensions/event_names.cc:17: const char
kBluetoothOnDeviceRemoved[] = "bluetooth.onDeviceRemoved";
Same as for header file: these constants should not be needed.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
File chrome/browser/extensions/event_names.h (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
chrome/browser/extensions/event_names.h:29: extern const char
kBluetoothOnDeviceAdded[];
These constants are automatically generated in bluetooth.h API file
(extensions::api::bluetooth::OnDevidedAdded::kEventName)

[keybuk, 2014-03-06 21:33:44]

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_api.cc (left):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_api.cc:305:
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
On 2014/03/05 21:44:15, armansito wrote:
> Keep this assertion?

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc:394: //
StopDiscovery success will remove the session object, unreferncing the
On 2014/03/05 21:44:15, armansito wrote:
> s/unreferncing/unreferencing/

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router.cc:357:
scoped_ptr<base::ListValue> args(new base::ListValue());
On 2014/03/06 00:11:19, rpaquay wrote:
> You should be able to use auto-generated code, i.e. 
> scoped_ptr<base::ListValue> args =
> bluetooth::OnDeviceChanged::Create(extension_device);

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router.h:128:
device::BluetoothDevice* device) OVERRIDE;
On 2014/03/06 00:11:19, rpaquay wrote:
> nit: missing space

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
File chrome/browser/extensions/event_names.cc (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
chrome/browser/extensions/event_names.cc:17: const char
kBluetoothOnDeviceRemoved[] = "bluetooth.onDeviceRemoved";
On 2014/03/06 00:11:19, rpaquay wrote:
> Same as for header file: these constants should not be needed.

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
File chrome/browser/extensions/event_names.h (right):

https://codereview.chromium.org/177113013/diff/1/chrome/browser/extensions/ev...
chrome/browser/extensions/event_names.h:29: extern const char
kBluetoothOnDeviceAdded[];
On 2014/03/06 00:11:19, rpaquay wrote:
> These constants are automatically generated in bluetooth.h API file
> (extensions::api::bluetooth::OnDevidedAdded::kEventName)

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth.idl (right):

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:186: // Get a list of bluetooth
devices known to the system, including paired
On 2014/03/05 21:30:09, miket wrote:
> Because this will become a part of public docs, style matters more. Please
> capitalize Bluetooth for consistency & correctness.

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:239: // onDeviceAdded event,
previously discovered devices already known to
On 2014/03/05 21:30:09, miket wrote:
> delete comma, start new sentence.

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth.idl:268: // out of range for long enough
to be considered unavailable again, and
On 2014/03/05 21:30:09, miket wrote:
> This would be more useful and readable if the more common case were listed
> first. I'll defer to you because you're the domain expert, but I'm guessing
> device removal is the more common case.

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/templates/articles/app_bluetooth.html
(right):

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/templates/articles/app_bluetooth.html:69: Finally
the <code>chrome.bluetooth.onDeviceRemoved</code>code> event is sent whenever a
paired device is removed from the system, or a discovered device
On 2014/03/05 21:44:15, armansito wrote:
> Remove the stray tag there (i.e. "</code>code>" -> "</code>")

Done.

https://codereview.chromium.org/177113013/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/templates/articles/app_bluetooth.html:83:
<code>chrome.bluetooth.startDiscovery</code>code> method, discovery can be
resource intensive so you should call
On 2014/03/05 21:44:15, armansito wrote:
> "</code>code>" -> "</code>"

Done.

[keybuk, 2014-03-06 21:33:55]

+sky for chrome/renderer/resources/

[armansito, 2014-03-06 21:45:48]

On 2014/03/06 21:33:55, keybuk wrote:
> +sky for chrome/renderer/resources/

FYI, https://src.chromium.org/viewvc/chrome?revision=255262&view=revision got
reverted, which this CL depends on. Bear with me while I get that CL in again.

[sky, 2014-03-06 22:05:10]

chrome/renderer/resources/renderer_resources.grd LGTM

[keybuk, 2014-03-07 22:13:06]

Committed patchset #5 manually as r255706 (presubmit successful).