bluetooth: Reject getPrimaryService(s) if frame is not connected

bluetooth: Reject getPrimaryService(s) if frame is not connected

First of three patches to reject promises if the frame is not connected
or the frame disconnected while the promise was not fulfilled.

Implements:

1. The ActiveAlgorithm set[1] as m_activeAlgorithms in BluetoothRemoteGATTServer
2. gattServer-connection-checking wrapper[2] as GetPrimaryServicesCallback in
   BluetoothRemoteGattServer.cpp
3. Connection check of query bluetooth cache[3] in getPrimaryServicesImpl.

[1] https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver-activealgorithms-slot
[2] https://webbluetoothcg.github.io/web-bluetooth/#connection-checking-wrapper
[3] https://webbluetoothcg.github.io/web-bluetooth/#query-the-bluetooth-cache

BUG=608538
Committed: https://crrev.com/ababc28cc3a0b4cc1f842548d1c5c31fdd4d2792
Cr-Commit-Position: refs/heads/master@{#408729}

[ortuno, 2016-07-26 17:46:20]

Description was changed from

==========
bluetooth: Reject getPrimaryService(s) if frame is not connected

First of three patches to reject promises if the frame is not connected
or the frame disconnected while the promise was not fulfilled.

BUG=608538
==========

to

==========
bluetooth: Reject getPrimaryService(s) if frame is not connected

First of three patches to reject promises if the frame is not connected
or the frame disconnected while the promise was not fulfilled.

Implements:

1. The ActiveAlgorithm set[1] as m_activeAlgorithms in BluetoothRemoteGATTServer
2. gattServer-connection-checking wrapper[2] as GetPrimaryServicesCallback in
   BluetoothRemoteGattServer.cpp
3. Connection check of query bluetooth cache[3] in getPrimaryServicesImpl.

[1]
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver...
[2] https://webbluetoothcg.github.io/web-bluetooth/#connection-checking-wrapper
[3] https://webbluetoothcg.github.io/web-bluetooth/#query-the-bluetooth-cache

BUG=608538
==========

[ortuno, 2016-07-26 18:06:33]

ortuno@chromium.org changed reviewers:
+ jyasskin@chromium.org

[ortuno, 2016-07-26 18:06:34]

jyasskin: PTAL

[Jeffrey Yasskin, 2016-07-28 22:45:14]

LGTM

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/device-disconnects-during.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/device-disconnects-during.html:18:
requestDisconnection.writeValue(new Uint8Array([0]));
Maybe call .connect() again as soon as the 'gattserverdisconnected' event
arrives. That won't do a great job of testing what happens when the reconnection
beats the getPrimaryService call, since getPrimaryService() will be fast, but if
it's ever flaky, it'll be an indication that something's wrong.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnect-called-during.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnect-called-during.html:17:
gattServer.disconnect();
Calling .connect() immediately after this will be a slightly better test, since
it'll get to start in the same turn. It'd be best if another frame were holding
the connection open so that .connect() would succeed immediately.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnected-device.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnected-device.html:15:
}, 'Call getPrimaryService before connecting. Reject with NetworkError.');
"Called getPrimaryService", I think.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryServices/disconnected-device.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryServices/disconnected-device.html:15:
}, 'Call getPrimaryServices before connecting. Reject with NetworkError.');
"Called"

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:102:
m_activeAlgorithms.clear();
Maybe use ClearActiveAlgorithms() here in case that function ever changes.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:117:
if (m_device->gatt()->connected()) {
Maybe CHECK that it's connected, since the caller needs to reject the promise if
it wasn't?

[ortuno, 2016-07-29 16:11:51]

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/device-disconnects-during.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/device-disconnects-during.html:18:
requestDisconnection.writeValue(new Uint8Array([0]));
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> Maybe call .connect() again as soon as the 'gattserverdisconnected' event
arrives. That won't do a great job of testing what happens when the reconnection
beats the getPrimaryService call, since getPrimaryService() will be fast, but if
it's ever flaky, it'll be an indication that something's wrong.

Added specific tests for the reconnection case. WDYT?

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnect-called-during.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnect-called-during.html:17:
gattServer.disconnect();
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> Calling .connect() immediately after this will be a slightly better test,
since it'll get to start in the same turn. It'd be best if another frame were
holding the connection open so that .connect() would succeed immediately.

I added some tests specifically for this. WDYT?

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnected-device.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryService/disconnected-device.html:15:
}, 'Call getPrimaryService before connecting. Reject with NetworkError.');
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> "Called getPrimaryService", I think.

Done.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/getPrimaryServices/disconnected-device.html
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/getPrimaryServices/disconnected-device.html:15:
}, 'Call getPrimaryServices before connecting. Reject with NetworkError.');
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> "Called"

Done.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
(right):

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:102:
m_activeAlgorithms.clear();
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> Maybe use ClearActiveAlgorithms() here in case that function ever changes.

Done.

https://codereview.chromium.org/2183903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:117:
if (m_device->gatt()->connected()) {
On 2016/07/28 at 22:45:14, Jeffrey Yasskin wrote:
> Maybe CHECK that it's connected, since the caller needs to reject the promise
if it wasn't?

Done.

[ortuno, 2016-07-29 16:11:54]

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

[commit-bot: I haz the power, 2016-07-29 16:12:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 17:14:33]

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

[commit-bot: I haz the power, 2016-07-29 17:14:33]

Dry run: This issue passed the CQ dry run.

[Jeffrey Yasskin, 2016-07-29 18:17:25]

Perfect, thanks.

https://codereview.chromium.org/2183903002/diff/60001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
(right):

https://codereview.chromium.org/2183903002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:583:
// 2. Disconnect the device.
*If |disconnect| is true,

[ortuno, 2016-07-29 18:22:30]

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

[commit-bot: I haz the power, 2016-07-29 18:22:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ortuno, 2016-07-29 19:06:14]

Thanks!

https://codereview.chromium.org/2183903002/diff/60001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
(right):

https://codereview.chromium.org/2183903002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:583:
// 2. Disconnect the device.
On 2016/07/29 at 18:17:24, Jeffrey Yasskin wrote:
> *If |disconnect| is true,

Done.

[ortuno, 2016-07-29 19:06:25]

The CQ bit was unchecked by ortuno@chromium.org

[ortuno, 2016-07-29 19:06:30]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-07-29 19:06:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jyasskin@chromium.org
Link to the patchset: https://codereview.chromium.org/2183903002/#ps80001
(title: "Docs")

[commit-bot: I haz the power, 2016-07-29 19:07:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 19:35:46]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-29 19:37:55]

Description was changed from

==========
bluetooth: Reject getPrimaryService(s) if frame is not connected

First of three patches to reject promises if the frame is not connected
or the frame disconnected while the promise was not fulfilled.

Implements:

1. The ActiveAlgorithm set[1] as m_activeAlgorithms in BluetoothRemoteGATTServer
2. gattServer-connection-checking wrapper[2] as GetPrimaryServicesCallback in
   BluetoothRemoteGattServer.cpp
3. Connection check of query bluetooth cache[3] in getPrimaryServicesImpl.

[1]
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver...
[2] https://webbluetoothcg.github.io/web-bluetooth/#connection-checking-wrapper
[3] https://webbluetoothcg.github.io/web-bluetooth/#query-the-bluetooth-cache

BUG=608538
==========

to

==========
bluetooth: Reject getPrimaryService(s) if frame is not connected

First of three patches to reject promises if the frame is not connected
or the frame disconnected while the promise was not fulfilled.

Implements:

1. The ActiveAlgorithm set[1] as m_activeAlgorithms in BluetoothRemoteGATTServer
2. gattServer-connection-checking wrapper[2] as GetPrimaryServicesCallback in
   BluetoothRemoteGattServer.cpp
3. Connection check of query bluetooth cache[3] in getPrimaryServicesImpl.

[1]
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver...
[2] https://webbluetoothcg.github.io/web-bluetooth/#connection-checking-wrapper
[3] https://webbluetoothcg.github.io/web-bluetooth/#query-the-bluetooth-cache

BUG=608538

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

[commit-bot: I haz the power, 2016-07-29 19:37:57]

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