Bluetooth: remove private members from BluetoothAdapter

Bluetooth: remove private members from BluetoothAdapter

BluetoothAdapter had two private member variables: address_ and name_
and expected implementations to fill them in so its address() and
name() member functions could return const refs to them.

That's a bad design, and prevented implementations from just fetching
these properties on demand, so make the functions pure virtual and
expect implementations to provide them.

BUG=none
TEST=device_unittests, unit_tests, browser_tests


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

[keybuk, 2013-03-28 22:25:31]

+miket for /api/ OWNERS

[keybuk, 2013-03-28 22:26:13]

+mark for obj-c check

[miket_OOO, 2013-03-29 16:31:21]

On 2013/03/28 22:25:31, keybuk wrote:
> +miket for /api/ OWNERS

LGTM OWNERS

[youngki, 2013-04-02 12:30:30]

lgtm

Please address the comments below. I don't have to recheck them.

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
File chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc (right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc:59: mock_adapter_ =
new testing::StrictMock<MockBluetoothAdapter>;
I think using an empty parenthesis is prevalent (i.e. new
testing::StrictMock<MockBluetoothAdapter>(); )

https://code.google.com/p/chromium/codesearch#search/&q=testing::StrictMock%3...

https://code.google.com/p/chromium/codesearch#search/&q=testing::StrictMock%3...

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc
(right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:16: :
mock_adapter_(new testing::StrictMock<device::MockBluetoothAdapter>),
put parentheses () .

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
File device/bluetooth/bluetooth_adapter_chromeos_experimental.h (right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
device/bluetooth/bluetooth_adapter_chromeos_experimental.h:6: #define
DEVICE_BLUETOOTH_BLUETOOTH_ADAPTER_CHROMEOS_EXPERIMENTAL_H_
Do we still need this file since we are going to change
bluetooth_adapter_chromeos.{h|cc}?

If we still need this, could we rename this to
bluetooth_adapter_experimental_chromeos.{h|cc} so that these files should only
be compiled when chromeos=1?

[Mark Mentovai, 2013-04-02 18:05:26]

There wasn’t actually any Objective-C in this change.

LGTM with these comments addressed.

https://codereview.chromium.org/13224004/diff/8017/chrome/browser/extensions/...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc
(right):

https://codereview.chromium.org/13224004/diff/8017/chrome/browser/extensions/...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:15:
ExtensionBluetoothEventRouterTest()
This constructor should be explicit.

https://codereview.chromium.org/13224004/diff/8017/chrome/browser/extensions/...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:16: :
mock_adapter_(new testing::StrictMock<device::MockBluetoothAdapter>),
Nobody deletes this?

https://codereview.chromium.org/13224004/diff/8017/device/bluetooth/bluetooth...
File device/bluetooth/bluetooth_adapter_chromeos_experimental.cc (right):

https://codereview.chromium.org/13224004/diff/8017/device/bluetooth/bluetooth...
device/bluetooth/bluetooth_adapter_chromeos_experimental.cc:30: return "";
return std::string();

Same on line 30.

See https://codereview.chromium.org/13145003/ .

[keybuk, 2013-04-02 19:03:09]

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
File chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc (right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_apitest.cc:59: mock_adapter_ =
new testing::StrictMock<MockBluetoothAdapter>;
On 2013/04/02 12:30:31, youngki wrote:
> I think using an empty parenthesis is prevalent (i.e. new
> testing::StrictMock<MockBluetoothAdapter>(); )
> 
>
https://code.google.com/p/chromium/codesearch#search/&q=testing::StrictMock%2...
> 
>
https://code.google.com/p/chromium/codesearch#search/&q=testing::StrictMock%2...

Done.

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc
(right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:15:
ExtensionBluetoothEventRouterTest()
On 2013/04/02 18:05:27, Mark Mentovai wrote:
> This constructor should be explicit.

I'm confused as to why this would be explicit?

explicit is meaningless for a constructor that has no arguments, since it would
never be involved in an implicit conversion ?

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:16: :
mock_adapter_(new testing::StrictMock<device::MockBluetoothAdapter>),
On 2013/04/02 12:30:31, youngki wrote:
> put parentheses () .

Done.

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
File device/bluetooth/bluetooth_adapter_chromeos_experimental.cc (right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
device/bluetooth/bluetooth_adapter_chromeos_experimental.cc:30: return "";
On 2013/04/02 18:05:27, Mark Mentovai wrote:
> return std::string();
> 
> Same on line 30.
> 
> See https://codereview.chromium.org/13145003/ .

Done.

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
File device/bluetooth/bluetooth_adapter_chromeos_experimental.h (right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/device/bluetooth/bl...
device/bluetooth/bluetooth_adapter_chromeos_experimental.h:6: #define
DEVICE_BLUETOOTH_BLUETOOTH_ADAPTER_CHROMEOS_EXPERIMENTAL_H_
discussed renaming offline, will address in a new CL

[commit-bot: I haz the power, 2013-04-02 19:07:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keybuk@chromium.org/13224004/17002

[Mark Mentovai, 2013-04-02 19:30:24]

LGTM

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
File chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc
(right):

https://chromiumcodereview.appspot.com/13224004/diff/8017/chrome/browser/exte...
chrome/browser/extensions/api/bluetooth/bluetooth_event_router_unittest.cc:15:
ExtensionBluetoothEventRouterTest()
keybuk wrote:
> On 2013/04/02 18:05:27, Mark Mentovai wrote:
> > This constructor should be explicit.
> 
> I'm confused as to why this would be explicit?
> 
> explicit is meaningless for a constructor that has no arguments, since it
would
> never be involved in an implicit conversion ?

Yes, this was a mistake.

[commit-bot: I haz the power, 2013-04-02 23:59:32]

Change committed as 191941