Add bluetooth.getName API.

Add bluetooth.getName API.

This allows Chrome Apps to get the name of the Bluetooth adapter.

BUG=none
TEST=browser_tests --gtest_filter=BluetoothApiTest.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152470

[bryeung, 2012-08-16 22:34:23]

keybuk: for c/b/chromeos/*
asargent: for c/b/extensions/*

Thanks!

[keybuk, 2012-08-17 18:27:06]

lgtm

[keybuk, 2012-08-17 18:27:44]

(noting that name is likely to be one of the inspiring following three values:
  ""
  "localhost-0"
  "Bluetooth USB Host Controller"

:p)

[Mihai Parparita -not on Chrome, 2012-08-17 20:30:07]

LGTM

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/bluetooth/bluetooth_adapter.h (right):

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/bluetooth/bluetooth_adapter.h:101: virtual const
std::string& address() const;
Nit: lowercase names are for basic getters/setters that can be inlined. Virtual
methods are by definition not inlineable, so this should be called GetAddress()
(and the same for name() below)

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/bluetooth/bluetooth_api.cc (right):

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/bluetooth/bluetooth_api.cc:91: bool
BluetoothGetNameFunction::RunImpl() {
Does this need a stub implementation in the #else block below?

[keybuk, 2012-08-17 20:31:36]

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/bluetooth/bluetooth_adapter.h (right):

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/bluetooth/bluetooth_adapter.h:101: virtual const
std::string& address() const;
Is there any particular reason this method has been made virtual?

For testing can't you create the structure with the right member value - I can't
see a reason you'd need to ever "verify that address() or name() are called" ?

[asargent_no_longer_on_chrome, 2012-08-20 19:07:19]

LGTM

In looking into the code to figure out whether |name| can be null for my
documentation nit, I was worried that we aren't handling the possibility of the
adapter being null. It looks like it can't be, but
BluetoothEventRouter::adapter() returns a pointer instead of a reference which
raises the question to a reader unfamiliar with that code. Would it be worth
doing a separate CL to either change that to return a reference, or just add a
comment that it will always be valid (maybe include a CHECK/DCHECK somewhere
enforcing that)?

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/common/extensio...
File chrome/common/extensions/api/experimental_bluetooth.idl (right):

https://chromiumcodereview.appspot.com/10837294/diff/1/chrome/common/extensio...
chrome/common/extensions/api/experimental_bluetooth.idl:146: // |callback| :
Called with the name, or null if there was an error.
documentation nit: from a glance at the implementation, it looks like there
aren't any possible errors (apart from a null pointer dereference in the browser
process if the bluetooth adapter were somehow null), but even if there were
|name| would just end up as the empty string instead of null. Finally, for null
to be a valid return value, I think you'd need to change the signature of
NameCallback to:

callback NameCallback = void (DOMString? result);

[bryeung, 2012-08-20 23:13:19]

Antony: I'll address your comments about the adapter being a pointer in a
follow-up CL.

http://codereview.chromium.org/10837294/diff/1/chrome/browser/chromeos/blueto...
File chrome/browser/chromeos/bluetooth/bluetooth_adapter.h (right):

http://codereview.chromium.org/10837294/diff/1/chrome/browser/chromeos/blueto...
chrome/browser/chromeos/bluetooth/bluetooth_adapter.h:101: virtual const
std::string& address() const;
On 2012/08/17 20:31:36, keybuk wrote:
> Is there any particular reason this method has been made virtual?
> 
> For testing can't you create the structure with the right member value - I
can't
> see a reason you'd need to ever "verify that address() or name() are called" ?

Done.

http://codereview.chromium.org/10837294/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/bluetooth/bluetooth_api.cc (right):

http://codereview.chromium.org/10837294/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/bluetooth/bluetooth_api.cc:91: bool
BluetoothGetNameFunction::RunImpl() {
On 2012/08/17 20:30:07, Mihai Parparita wrote:
> Does this need a stub implementation in the #else block below?

Good catch!  Done.

http://codereview.chromium.org/10837294/diff/1/chrome/common/extensions/api/e...
File chrome/common/extensions/api/experimental_bluetooth.idl (right):

http://codereview.chromium.org/10837294/diff/1/chrome/common/extensions/api/e...
chrome/common/extensions/api/experimental_bluetooth.idl:146: // |callback| :
Called with the name, or null if there was an error.
On 2012/08/20 19:07:19, Antony Sargent wrote:
> documentation nit: from a glance at the implementation, it looks like there
> aren't any possible errors (apart from a null pointer dereference in the
browser
> process if the bluetooth adapter were somehow null), but even if there were
> |name| would just end up as the empty string instead of null. Finally, for
null
> to be a valid return value, I think you'd need to change the signature of
> NameCallback to:
> 
> callback NameCallback = void (DOMString? result);
> 

Done.

[commit-bot: I haz the power, 2012-08-20 23:13:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryeung@chromium.org/10837294/8001

[commit-bot: I haz the power, 2012-08-21 01:31:53]

Change committed as 152470