DBus changes for implementing local GATT attributes.

DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.

Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests

R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
Committed: https://crrev.com/a7a149d373a58c0e324353175feb0cefb0cb9c8f
Cr-Commit-Position: refs/heads/master@{#390601}

[rkc, 2016-04-25 00:50:14]

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

[commit-bot: I haz the power, 2016-04-25 00:50:20]

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

[commit-bot: I haz the power, 2016-04-25 01:55:07]

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

[commit-bot: I haz the power, 2016-04-25 01:55:08]

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-25 19:08:24]

merge (magic values fix)

[rkc, 2016-04-25 19:10:31]

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

[commit-bot: I haz the power, 2016-04-25 19:11:17]

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

[commit-bot: I haz the power, 2016-04-25 20:30:00]

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

[commit-bot: I haz the power, 2016-04-25 20:30:01]

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-27 20:11:50]

Description was changed from

==========
DBus changes for implementing local GATT attributes.

This CL is part 2 of https://codereview.chromium.org/1920693002/

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.

R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

to

==========
DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.


Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

[rkc, 2016-04-28 03:10:13]

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

[commit-bot: I haz the power, 2016-04-28 03:10:34]

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

[commit-bot: I haz the power, 2016-04-28 04:12:42]

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

[commit-bot: I haz the power, 2016-04-28 04:12:43]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-04-28 04:54:28]

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc:54:
void BluetoothGattApplicationServiceProviderImpl::WriteObjectStruct(
Add unit tests code coverage for the Write* methods and GetManagedObjects.

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.cc:334:
dbus::Response::FromMethodCall(method_call);
Are protections against the callback being called twice prudent?

[rkc, 2016-04-28 18:13:23]

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc:54:
void BluetoothGattApplicationServiceProviderImpl::WriteObjectStruct(
On 2016/04/28 04:54:28, scheib wrote:
> Add unit tests code coverage for the Write* methods and GetManagedObjects.

There is no simple way to test real DBus code. If this were just one service
provider, it would be complicated to test but still doable, but since this is
really an object manager bringing up multiple service providers, this needs some
support code to get working.

I can create a bug for this and add it as a TODO, but the amount of code
required to be able to test this directly is probably not a good idea to put in
this CL. It ideally belongs in its own CL.

WDYT?

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.cc:334:
dbus::Response::FromMethodCall(method_call);
On 2016/04/28 04:54:28, scheib wrote:
> Are protections against the callback being called twice prudent?

I'm pretty certain that our DBus client code has protections against multiple
responses being sent for the same request. I'll verify though.

[scheib, 2016-04-28 20:46:29]

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc:54:
void BluetoothGattApplicationServiceProviderImpl::WriteObjectStruct(
On 2016/04/28 18:13:23, Rahul Chaturvedi wrote:
> On 2016/04/28 04:54:28, scheib wrote:
> > Add unit tests code coverage for the Write* methods and GetManagedObjects.
> 
> There is no simple way to test real DBus code. If this were just one service
> provider, it would be complicated to test but still doable, but since this is
> really an object manager bringing up multiple service providers, this needs
some
> support code to get working.
> 
> I can create a bug for this and add it as a TODO, but the amount of code
> required to be able to test this directly is probably not a good idea to put
in
> this CL. It ideally belongs in its own CL.
> 
> WDYT?

I'm suggesting that just the serialization code in the writer methos be tested,
not actual dbus details. Example:
https://code.google.com/p/chromium/codesearch#chromium/src/dbus/message_unitt...

[rkc, 2016-04-29 03:53:12]

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc
(right):

https://codereview.chromium.org/1914893002/diff/120001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc:54:
void BluetoothGattApplicationServiceProviderImpl::WriteObjectStruct(
On 2016/04/28 20:46:29, scheib wrote:
> On 2016/04/28 18:13:23, Rahul Chaturvedi wrote:
> > On 2016/04/28 04:54:28, scheib wrote:
> > > Add unit tests code coverage for the Write* methods and GetManagedObjects.
> > 
> > There is no simple way to test real DBus code. If this were just one service
> > provider, it would be complicated to test but still doable, but since this
is
> > really an object manager bringing up multiple service providers, this needs
> some
> > support code to get working.
> > 
> > I can create a bug for this and add it as a TODO, but the amount of code
> > required to be able to test this directly is probably not a good idea to put
> in
> > this CL. It ideally belongs in its own CL.
> > 
> > WDYT?
> 
> I'm suggesting that just the serialization code in the writer methos be
tested,
> not actual dbus details. Example:
>
https://code.google.com/p/chromium/codesearch#chromium/src/dbus/message_unitt...

1700 LoC more, but we have a unit test now. :)

[rkc, 2016-04-29 03:53:22]

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

[commit-bot: I haz the power, 2016-04-29 03:53:33]

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

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

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

[commit-bot: I haz the power, 2016-04-29 04:02:40]

Dry run: 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-29 04:54:35]

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

[commit-bot: I haz the power, 2016-04-29 04:54:44]

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

[scheib, 2016-04-29 05:05:20]

https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn
File device/BUILD.gn (right):

https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn#newcod...
device/BUILD.gn:169:
"bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc",
DEVICE_BLUETOOTH_EXPORT the classes?

[rkc, 2016-04-29 05:10:17]

https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn
File device/BUILD.gn (right):

https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn#newcod...
device/BUILD.gn:169:
"bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc",
On 2016/04/29 05:05:20, scheib wrote:
> DEVICE_BLUETOOTH_EXPORT the classes? 

Already done in a later patch.

[scheib, 2016-04-29 05:55:02]

On 2016/04/29 05:10:17, Rahul Chaturvedi wrote:
> https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn
> File device/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1914893002/diff/140001/device/BUILD.gn#newcod...
> device/BUILD.gn:169:
> "bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.cc",
> On 2016/04/29 05:05:20, scheib wrote:
> > DEVICE_BLUETOOTH_EXPORT the classes? 
> 
> Already done in a later patch.

LGTM

[commit-bot: I haz the power, 2016-04-29 06:27:48]

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

[commit-bot: I haz the power, 2016-04-29 06:27:49]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rkc, 2016-04-29 06:35:44]

The CQ bit was checked by rkc@chromium.org

[rkc, 2016-04-29 06:35:45]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/1914893002/#ps180001
(title: "test leak fix")

[commit-bot: I haz the power, 2016-04-29 06:35:56]

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

[commit-bot: I haz the power, 2016-04-29 08:15:50]

Description was changed from

==========
DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.


Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

to

==========
DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.


Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

[commit-bot: I haz the power, 2016-04-29 08:15:52]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-04-30 17:24:36]

Patchset 10 (id:??) landed as
https://crrev.com/a7a149d373a58c0e324353175feb0cefb0cb9c8f
Cr-Commit-Position: refs/heads/master@{#390601}

[commit-bot: I haz the power, 2016-04-30 17:59:51]

Description was changed from

==========
DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.


Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

to

==========
DBus changes for implementing local GATT attributes.

In this CL, we implement the changes to the DBus managers and providers
needed to support local GATT services. The major changes from the code
on trunk are,

.) Adds a new BluetoothGattApplicationServiceProvider. This service
provider provides an object manager which manages all the other exported
GATT attributes. It is responsible for setting up all the individual
attribute service providers correctly.
.) Adds the correct fake implementations for this service provider, the
other attribute service providers and the Gatt Manager client.
.) Does various cleanup changes in touched code.

Part 2 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes
https://crrev.com/1914893002 DBus class changes <<<
https://crrev.com/1919683002 Adapter changes + tests

R=ortuno@chromium.org, scheib@chromium.org
BUG=601935

Committed: https://crrev.com/a7a149d373a58c0e324353175feb0cefb0cb9c8f
Cr-Commit-Position: refs/heads/master@{#390601}
==========