Refactor device::BluetoothGattXXX classes to split into remote/local.

Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.

Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.

R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org, rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
Committed: https://crrev.com/1222397542d62a15aa133930d7ef311223a6e5e0
Cr-Commit-Position: refs/heads/master@{#388604}

[rkc, 2016-04-18 02:37:09]

Description was changed from

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

to

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should do
the same for other platforms too so the //device/bluetooth directory isn't as
crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

[rkc, 2016-04-18 02:37:32]

Description was changed from

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should do
the same for other platforms too so the //device/bluetooth directory isn't as
crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

to

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't as
crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

[rkc, 2016-04-18 02:37:49]

Description was changed from

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't as
crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

to

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

[Ken Rockot(use gerrit already), 2016-04-18 15:23:05]

c/b/e LGTM

[rkc, 2016-04-18 20:00:37]

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-18 20:01:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/10001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/10001

[Luis Héctor Chávez, 2016-04-18 20:16:20]

rs-lgtm in components/arc

[commit-bot: I haz the power, 2016-04-18 20:18:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-18 20:18:29]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[rkc, 2016-04-18 22:08:54]

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-18 22:09:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/20001

[tbarzic, 2016-04-18 22:12:03]

Lgtm

https://codereview.chromium.org/1898643002/diff/1/components/proximity_auth/b...
File components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h
(right):

https://codereview.chromium.org/1898643002/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h:6:
#define
COMPONENTS_PROXIMITY_AUTH_BLE_BLUETOOTH_LOW_ENERGY_CHARACTERISTICS_FINDER_H_
nit: update #endif line

[commit-bot: I haz the power, 2016-04-19 00:23:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 00:23:48]

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-19 07:03:39]

Vincent, this latest patch moves the main accessors back into the base classes.
I believe you're right about this being too aggressive with moving the accessors
out. They will almost definitely be needed.

I haven't moved GetCharacteristic() and GetService() into the base classes
though because the remote and local versions of these will have different return
types (remote/local). I've left the Remote versions of these functions in their
place, will add Local versions in a further CL when they are being used.

https://codereview.chromium.org/1898643002/diff/1/components/proximity_auth/b...
File components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h
(right):

https://codereview.chromium.org/1898643002/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h:6:
#define
COMPONENTS_PROXIMITY_AUTH_BLE_BLUETOOTH_LOW_ENERGY_CHARACTERISTICS_FINDER_H_
On 2016/04/18 22:12:02, tbarzic wrote:
> nit: update #endif line

Done.

[rkc, 2016-04-19 07:04:05]

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 07:04:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/30001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/30001

[commit-bot: I haz the power, 2016-04-19 08:45:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 08:45:50]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-04-20 01:23:30]

https://codereview.chromium.org/1898643002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_gatt_characteristic.h (right):

https://codereview.chromium.org/1898643002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_gatt_characteristic.h:26: // used:
Complete comment.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_gatt_service.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_gatt_service.h:56: virtual std::string
GetIdentifier() const = 0;
Keep GetIdentifier above GetUUID

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_characteristic.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_characteristic.h:23: //
Maybe chime in on lifetime management. The Service will NOT be taking on
ownership of this object, right? BTW what will happen after Register is called
and some objects are destroyed?

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_characteristic.h:41: // read
wrapping

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:19: //
BluetoothLocalGattDescriptor represents a local or remote GATT characteristic
local only.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:32: // local
wrapping

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:34: // associate the returned
descriptor with a characteristic, provide an pointer
a pointer
^

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:52: const
std::vector<uint8_t>& value,
Because value can be empty on reads it is valid to create without a value. And
since often (always in our immediate needs) a delegate will exist why require
value here?  I think we can drop value from the Create methods for
Characteristics and Descriptors. -- OK in a follow up patch of course.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_service.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:24: // BluetoothLocalGattService
represents a local  GATT service.
"  " -> " "

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:63: const
BluetoothRemoteGattCharacteristic* characteristic,
Local Characteristic (and several more below)

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:134: // local
wrapping

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_remote_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_descriptor.h:23: // descriptor.
Might as well keep the "A GATT characteristic descriptor provides..."
description.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:19: //
BluetoothRemoteGattCharacteristic for remote GATT characteristics for
implements ...Local...

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h
(right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h:48:
device::BluetoothRemoteGattService* GetService() const override;
Order these to match bluetooth_remote_gatt_characteristic.h, and have the impl
in the .cc file match the method order of this header.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/test/m...
File device/bluetooth/test/mock_bluetooth_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/test/m...
device/bluetooth/test/mock_bluetooth_gatt_descriptor.h:24: class
MockBluetoothGattDescriptor : public BluetoothRemoteGattDescriptor {
Intentionally not using MockBluetoothRemoteGattDescriptor? 
Will we end up using a MockBluetoothLocalGattDescriptor?
OK to punt on this until later.

[rkc, 2016-04-20 16:31:55]

https://codereview.chromium.org/1898643002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_gatt_characteristic.h (right):

https://codereview.chromium.org/1898643002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_gatt_characteristic.h:26: // used:
On 2016/04/20 01:23:30, scheib wrote:
> Complete comment.

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_gatt_service.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_gatt_service.h:56: virtual std::string
GetIdentifier() const = 0;
On 2016/04/20 01:23:30, scheib wrote:
> Keep GetIdentifier above GetUUID

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_characteristic.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_characteristic.h:23: //
On 2016/04/20 01:23:30, scheib wrote:
> Maybe chime in on lifetime management. The Service will NOT be taking on
> ownership of this object, right? BTW what will happen after Register is called
> and some objects are destroyed? 

In a later CL, these objects are now refcounted. I'll be adding an explanation
of their lifetime management in the header in the same CL.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_characteristic.h:41: // read
On 2016/04/20 01:23:30, scheib wrote:
> wrapping

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:19: //
BluetoothLocalGattDescriptor represents a local or remote GATT characteristic
On 2016/04/20 01:23:30, scheib wrote:
> local only.

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:32: // local
On 2016/04/20 01:23:30, scheib wrote:
> wrapping

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:34: // associate the returned
descriptor with a characteristic, provide an pointer
On 2016/04/20 01:23:30, scheib wrote:
> a pointer
> ^

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_descriptor.h:52: const
std::vector<uint8_t>& value,
On 2016/04/20 01:23:30, scheib wrote:
> Because value can be empty on reads it is valid to create without a value. And
> since often (always in our immediate needs) a delegate will exist why require
> value here?  I think we can drop value from the Create methods for
> Characteristics and Descriptors. -- OK in a follow up patch of course. 

Agreed. This parameter is gone in a later patch.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_service.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:24: // BluetoothLocalGattService
represents a local  GATT service.
On 2016/04/20 01:23:30, scheib wrote:
> "  " -> " "

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:63: const
BluetoothRemoteGattCharacteristic* characteristic,
On 2016/04/20 01:23:30, scheib wrote:
> Local Characteristic (and several more below)

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:134: // local
On 2016/04/20 01:23:30, scheib wrote:
> wrapping

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_remote_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_descriptor.h:23: // descriptor.
On 2016/04/20 01:23:30, scheib wrote:
> Might as well keep the "A GATT characteristic descriptor provides..."
> description.

Its there in the comment for the base class. I am trying to have header comments
for derived classes that just describe things that are different or not present
in the base class.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:19: //
BluetoothRemoteGattCharacteristic for remote GATT characteristics for
On 2016/04/20 01:23:30, scheib wrote:
> implements ...Local...

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h
(right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h:48:
device::BluetoothRemoteGattService* GetService() const override;
On 2016/04/20 01:23:30, scheib wrote:
> Order these to match bluetooth_remote_gatt_characteristic.h, and have the impl
> in the .cc file match the method order of this header. 

Done.

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/test/m...
File device/bluetooth/test/mock_bluetooth_gatt_descriptor.h (right):

https://codereview.chromium.org/1898643002/diff/40001/device/bluetooth/test/m...
device/bluetooth/test/mock_bluetooth_gatt_descriptor.h:24: class
MockBluetoothGattDescriptor : public BluetoothRemoteGattDescriptor {
On 2016/04/20 01:23:30, scheib wrote:
> Intentionally not using MockBluetoothRemoteGattDescriptor? 
> Will we end up using a MockBluetoothLocalGattDescriptor?
> OK to punt on this until later.

This class isn't getting used anywhere. I've put a TODO to make sure that either
this class is used in a test (and renamed correctly), otherwise removed.

[rkc, 2016-04-20 16:32:52]

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-20 16:33:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/50001

[commit-bot: I haz the power, 2016-04-20 16:38:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 16:38:42]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[rkc, 2016-04-20 16:56:28]

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-20 16:56:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/50001

[commit-bot: I haz the power, 2016-04-20 18:03:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 18:03:30]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-04-20 19:36:27]

LGTM

[rkc, 2016-04-20 20:24:36]

The CQ bit was checked by rkc@chromium.org

[rkc, 2016-04-20 20:24:37]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
lhchavez@chromium.org, tbarzic@chromium.org
Link to the patchset: https://codereview.chromium.org/1898643002/#ps50001
(title: " ")

[commit-bot: I haz the power, 2016-04-20 20:24:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/50001

[commit-bot: I haz the power, 2016-04-20 20:34:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 20:34:24]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[rkc, 2016-04-20 21:06:50]

The CQ bit was checked by rkc@chromium.org

[commit-bot: I haz the power, 2016-04-20 21:07:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/50001

[commit-bot: I haz the power, 2016-04-20 21:16:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-20 21:16:25]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[rkc, 2016-04-20 21:29:02]

The CQ bit was checked by rkc@chromium.org

[rkc, 2016-04-20 21:29:03]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
lhchavez@chromium.org, scheib@chromium.org, tbarzic@chromium.org
Link to the patchset: https://codereview.chromium.org/1898643002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-04-20 21:29:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1898643002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1898643002/60001

[commit-bot: I haz the power, 2016-04-20 23:59:23]

Description was changed from

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

to

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

[commit-bot: I haz the power, 2016-04-20 23:59:27]

Committed patchset #7 (id:60001)

[commit-bot: I haz the power, 2016-04-22 19:27:44]

Description was changed from

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.


Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.


R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935
==========

to

==========
Refactor device::BluetoothGattXXX classes to split into remote/local.

Currently we have only one type of BluetoothGattService,
BluetoothGattCharacterstic and BluetoothGattDescriptor. Each of these objects
have significantly divergent functionality depending on whether they are
objects representing a remote or local version. For example, ReadRemoteValue
or WriteRemoteValue makes no sense on a local characteristic. Similarly,
delegates for receiving read/write requests for characteristics and
descriptors doesn't make any sense for a remote service. Trying to accomodate
this led to very ugly code with lots of NOTIMPLEMENTED()s, varying
implementations of the same code and a lot of repeated but unused code.

Seperating these out into remote and local interfaces allows us to have platform
specific implementations which only contain code relevant to remote and local
objects (with common implementation deriving the common interface).

This is just a refactor with no actual code change. To keep reviewing easier,
any fixes to the code that is being moved around is being reserved for other
CLs.

This CL also moves all BlueZ specific code into its own directory. We should
do the same for other platforms too so the //device/bluetooth directory isn't
as crowded.

Reviews requested,

OWNERS (should most probably be rubberstamps),
chrome/browser/extensions/api/* - rockot
components/arc/bluetooth/* - elijahtaylor,lhchavez
components/proximity_auth/* - tbarzic

scheib/ortuno for the general review.

R=elijahtaylor@chromium.org, lhchavez@chromium.org, ortuno@chromium.org,
rockot@chromium.org, scheib@chromium.org, tbarzic@chromium.org
BUG=601935

Committed: https://crrev.com/1222397542d62a15aa133930d7ef311223a6e5e0
Cr-Commit-Position: refs/heads/master@{#388604}
==========

[commit-bot: I haz the power, 2016-04-22 19:27:46]

Patchset 7 (id:??) landed as
https://crrev.com/1222397542d62a15aa133930d7ef311223a6e5e0
Cr-Commit-Position: refs/heads/master@{#388604}