Adapter changes for implementing local GATT attributes and tests.

Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).

Part 3 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/3fb9aa6d505dd9c1e225403c865a482a62df892d
Cr-Commit-Position: refs/heads/master@{#391346}

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

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:28]

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

[commit-bot: I haz the power, 2016-04-25 01:59:27]

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

[commit-bot: I haz the power, 2016-04-25 01:59:27]

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-25 19:09:11]

merge (magic values fix)

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

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:18]

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

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

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

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

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-27 20:07:47]

merge

[rkc, 2016-04-27 20:12:15]

Description was changed from

==========
Adapter changes for implementing local GATT attributes and tests.

This CL is part 3 (final) of https://codereview.chromium.org/1920693002/

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).

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

to

==========
Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).


Part 3 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:28]

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:46]

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

[commit-bot: I haz the power, 2016-04-28 03:13:32]

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

[commit-bot: I haz the power, 2016-04-28 03:13:32]

Dry run: Try jobs failed on following builders:
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

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

Only skimmed over this so far, general shape looks good.

https://codereview.chromium.org/1919683002/diff/100001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_adapter_unittest.cc (right):

https://codereview.chromium.org/1919683002/diff/100001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter_unittest.cc:556: #if defined(OS_ANDROID) ||
defined(OS_MACOSX) || defined(OS_WIN)
Why do these no longer work?

[rkc, 2016-04-28 19:02:44]

https://codereview.chromium.org/1919683002/diff/100001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_adapter_unittest.cc (right):

https://codereview.chromium.org/1919683002/diff/100001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter_unittest.cc:556: #if defined(OS_ANDROID) ||
defined(OS_MACOSX) || defined(OS_WIN)
On 2016/04/28 05:02:29, scheib wrote:
> Why do these no longer work?

These actually never worked. These tests were only passing because we weren't
calling BluetoothTestBase::TearDown() from BluetoothTestBlueZ::TearDown(), hence
never checking the expected callbacks.

The issue is with running the message loop. RunUntilIdle doesn't seem to run the
DiscoverLowEnergyDevice code. We need to use a run_loop and post a quit closure
when done. When done is not quite clear and will need some investigation.

Since these aren't new tests, it seemed more logical to disable them again
rather than try to fix them in this unrelated CL.

[rkc, 2016-04-28 19:03:41]

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

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

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

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

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

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

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-29 08:49:34]

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

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

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

[commit-bot: I haz the power, 2016-04-29 10:13:57]

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

[commit-bot: I haz the power, 2016-04-29 10:13:58]

Dry run: This issue passed the CQ dry run.

[rkc, 2016-04-30 15:15:15]

ping?

[scheib, 2016-05-03 05:55:57]

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluet...
device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc:34:
TestLocalGattServiceDelegate::value_to_write_ = 0x1337;
Instead of statics for ::value_to_write etc, why not just
  delegate_.value_to_write_?

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
File device/bluetooth/bluez/bluetooth_adapter_bluez.cc (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
device/bluetooth/bluez/bluetooth_adapter_bluez.cc:108: void
OnServiceErrorCallback(
Just the VLOG makes this 'register / unregister' specific, but it seems the
method name could be 'register / unregister' specific or the LOG could be
changed to be more generic.

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
device/bluetooth/bluez/bluetooth_adapter_bluez.cc:1085: void
BluetoothAdapterBlueZ::RegisterApplication(
Keep declarations and definitions in same order.

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/test/...
File device/bluetooth/test/bluetooth_gatt_server_test.h (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/test/...
device/bluetooth/test/bluetooth_gatt_server_test.h:30: class
TestLocalGattServiceDelegate
I think move this to it's own .h/.cc; Ratio of large enough / coupled nature to
BluetoothGattServerTest is large enough to warrant being stand alone.

[rkc, 2016-05-03 18:22:15]

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluet...
device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc:34:
TestLocalGattServiceDelegate::value_to_write_ = 0x1337;
On 2016/05/03 05:55:57, scheib wrote:
> Instead of statics for ::value_to_write etc, why not just
>   delegate_.value_to_write_?

Done.

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
File device/bluetooth/bluez/bluetooth_adapter_bluez.cc (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
device/bluetooth/bluez/bluetooth_adapter_bluez.cc:108: void
OnServiceErrorCallback(
On 2016/05/03 05:55:57, scheib wrote:
> Just the VLOG makes this 'register / unregister' specific, but it seems the
> method name could be 'register / unregister' specific or the LOG could be
> changed to be more generic. 

Done.

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/bluez...
device/bluetooth/bluez/bluetooth_adapter_bluez.cc:1085: void
BluetoothAdapterBlueZ::RegisterApplication(
On 2016/05/03 05:55:57, scheib wrote:
> Keep declarations and definitions in same order.

Done.

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/test/...
File device/bluetooth/test/bluetooth_gatt_server_test.h (right):

https://codereview.chromium.org/1919683002/diff/180001/device/bluetooth/test/...
device/bluetooth/test/bluetooth_gatt_server_test.h:30: class
TestLocalGattServiceDelegate
On 2016/05/03 05:55:57, scheib wrote:
> I think move this to it's own .h/.cc; Ratio of large enough / coupled nature
to
> BluetoothGattServerTest is large enough to warrant being stand alone. 

Done.

[rkc, 2016-05-03 18:22:25]

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

[commit-bot: I haz the power, 2016-05-03 18:22:44]

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

[commit-bot: I haz the power, 2016-05-03 18:26:50]

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

[commit-bot: I haz the power, 2016-05-03 18:26:51]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rkc, 2016-05-03 18:28:52]

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

[commit-bot: I haz the power, 2016-05-03 18:29:18]

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

[scheib, 2016-05-03 18:53:04]

LGTM

[commit-bot: I haz the power, 2016-05-03 19:43:10]

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

[commit-bot: I haz the power, 2016-05-03 19:43:11]

Dry run: This issue passed the CQ dry run.

[rkc, 2016-05-03 20:35:32]

The CQ bit was checked by rkc@chromium.org

[commit-bot: I haz the power, 2016-05-03 20:35:55]

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

[commit-bot: I haz the power, 2016-05-03 20:39:54]

Description was changed from

==========
Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).


Part 3 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

==========
Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).


Part 3 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-05-03 20:39:55]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-05-03 20:41:24]

Description was changed from

==========
Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).


Part 3 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

==========
Adapter changes for implementing local GATT attributes and tests.

In this CL, we implement the changes to the Bluetooth adapter for using
all the code added in the previous CLs. This CL also adds the testing
framework needed for testing these changes along with some basic tests.
The major changes from the code on trunk are,

.) The adapter now owns any created services. It also owns the dbus
application provider. Ownership of all other providers is by the
main application service provider and the Adapter doesn't have anything
to do with them.
.) The test framework adds an intermediate BluetoothGattServerTest class
to take out common parts of testing local GATT attributes since there
is a lot of common boilerplate code.
.) The tests are basic right now but the framework verifies that many  of
the different moving parts are working correctly.

What is missing is unit tests for the BluetoothGattApplicationProvider
class's implementation. These will be added later once I figure out a
good way to do so (we currently don't have any direct unit tests for
any of the DBus classes).

Part 3 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/3fb9aa6d505dd9c1e225403c865a482a62df892d
Cr-Commit-Position: refs/heads/master@{#391346}
==========

[commit-bot: I haz the power, 2016-05-03 20:41:26]

Patchset 12 (id:??) landed as
https://crrev.com/3fb9aa6d505dd9c1e225403c865a482a62df892d
Cr-Commit-Position: refs/heads/master@{#391346}