bluetooth: Invalidate services upon disconnection

bluetooth: Invalidate services upon disconnection

Remove services from attribute instance map when a device disconnects or when disconnect is called.
Check that the service is in the attribute instance map and reject if it isn't.

Small refactor of disconnect() and dispatchGattServerDisconnected() to more closely
match the spec.

BUG=654950
Committed: https://crrev.com/344468c89036d7eff858c39c7fbb0e360f0ea71e
Cr-Commit-Position: refs/heads/master@{#430206}

[ortuno, 2016-11-04 03:04:09]

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

[commit-bot: I haz the power, 2016-11-04 03:05:14]

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

[ortuno, 2016-11-04 03:06:06]

Description was changed from

==========
bluetooth: Invalidate services upon disconnection

BUG=654950
==========

to

==========
bluetooth: Invalidate services upon disconnection

Remove services from attribute instance map when a device disconnects or when
disconnect is called.
Check that the service is in the attribute instance map and reject if it isn't.

Small refactor of disconnect() and dispatchGattServerDisconnected() to more
closely
match the spec.

BUG=654950
==========

[ortuno, 2016-11-04 03:09:47]

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

[ortuno, 2016-11-04 03:09:48]

scheib: PTAL

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

https://codereview.chromium.org/2478013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp:103:
device()->CleanupDisconnectedDeviceAndFireEvent();
Note how we only clear the active algorithms map once: in
CleanupDisconectedDeviceAndFireEvent. The spec clears it twice once in
disconnect()[1] and once in Clean up disconnected device[2]. I believe that's
not necessary so I opened an issue about it[3].

[1]
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver...
[2]
https://webbluetoothcg.github.io/web-bluetooth/#clean-up-the-disconnected-device
[3] https://github.com/WebBluetoothCG/web-bluetooth/issues/333

[commit-bot: I haz the power, 2016-11-04 04:59:24]

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

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

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

[scheib, 2016-11-04 22:53:51]

LGTM

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp (right):

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp:72:
m_gatt->setConnected(false);
Here can we either if (connected), or DCHECK(connected)? I suspect
dispatchGattServerDisconnected below can make an unguarded call.

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h (right):

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h:71: // Should only
be used when the object is being garbage collected or the
Explain why it is only to be used in those cases, and that
CleanupDisconnectedDeviceAndFireEvent should be used for other reasons.

[ortuno, 2016-11-06 22:43:30]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-11-06 22:43:30]

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/2478013002/#ps60001
(title: "Address scheib's comments")

[commit-bot: I haz the power, 2016-11-06 22:43:36]

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

[ortuno, 2016-11-06 22:44:12]

Thanks!

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp (right):

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp:72:
m_gatt->setConnected(false);
On 2016/11/04 at 22:53:50, scheib wrote:
> Here can we either if (connected), or DCHECK(connected)? I suspect
dispatchGattServerDisconnected below can make an unguarded call.

Done.

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h (right):

https://codereview.chromium.org/2478013002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h:71: // Should only
be used when the object is being garbage collected or the
On 2016/11/04 at 22:53:50, scheib wrote:
> Explain why it is only to be used in those cases, and that
CleanupDisconnectedDeviceAndFireEvent should be used for other reasons.

Done.

[commit-bot: I haz the power, 2016-11-06 23:55:50]

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

[commit-bot: I haz the power, 2016-11-06 23:55:51]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[ortuno, 2016-11-07 00:01:14]

The CQ bit was checked by ortuno@chromium.org

[commit-bot: I haz the power, 2016-11-07 00:01:46]

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

[ortuno, 2016-11-07 00:36:28]

The CQ bit was unchecked by ortuno@chromium.org

[ortuno, 2016-11-07 00:36:34]

The CQ bit was checked by ortuno@chromium.org

[commit-bot: I haz the power, 2016-11-07 00:36:45]

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-11-07 01:17:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-07 01:20:37]

Description was changed from

==========
bluetooth: Invalidate services upon disconnection

Remove services from attribute instance map when a device disconnects or when
disconnect is called.
Check that the service is in the attribute instance map and reject if it isn't.

Small refactor of disconnect() and dispatchGattServerDisconnected() to more
closely
match the spec.

BUG=654950
==========

to

==========
bluetooth: Invalidate services upon disconnection

Remove services from attribute instance map when a device disconnects or when
disconnect is called.
Check that the service is in the attribute instance map and reject if it isn't.

Small refactor of disconnect() and dispatchGattServerDisconnected() to more
closely
match the spec.

BUG=654950

Committed: https://crrev.com/344468c89036d7eff858c39c7fbb0e360f0ea71e
Cr-Commit-Position: refs/heads/master@{#430206}
==========

[commit-bot: I haz the power, 2016-11-07 01:20:38]

Patchset 4 (id:??) landed as
https://crrev.com/344468c89036d7eff858c39c7fbb0e360f0ea71e
Cr-Commit-Position: refs/heads/master@{#430206}