bluetooth: Fix logic mistake in DeviceChanged

bluetooth: Fix logic mistake in DeviceChanged

Fixes an issue in which we always disconnected a device even if the connection
is still active.

BUG=615383
Committed: https://crrev.com/bb0a85c376fc57c99d6b2e8df8af6ea5f8ab5775
Cr-Commit-Position: refs/heads/master@{#396877}

[ortuno, 2016-05-27 16:09:49]

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

[ortuno, 2016-05-27 16:09:50]

jyasskin: PTAL

[ortuno, 2016-05-27 16:09:53]

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

[commit-bot: I haz the power, 2016-05-27 16:10:13]

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

[commit-bot: I haz the power, 2016-05-27 17:50:45]

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

[commit-bot: I haz the power, 2016-05-27 17:50:45]

Dry run: This issue passed the CQ dry run.

[Jeffrey Yasskin, 2016-05-28 02:48:01]

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
content/browser/bluetooth/web_bluetooth_service_impl.cc:220: if
(!device->IsGattConnected() && !device->IsConnected()) {
Should we only check one of these? All of our connections are GATT-based, so
maybe just IsGattConnected()?

[chromium-reviews, 2016-05-28 03:24:44]

IsGattConnected is not implemented on Chrome OS yet.

On Fri, May 27, 2016, 7:48 PM <jyasskin@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
> File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):
>
>
>
https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
> content/browser/bluetooth/web_bluetooth_service_impl.cc:220: if
>
> (!device->IsGattConnected() && !device->IsConnected()) {
> Should we only check one of these? All of our connections are
> GATT-based, so maybe just IsGattConnected()?
>
> https://codereview.chromium.org/2016973003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jeffrey Yasskin, 2016-05-28 03:49:52]

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
content/browser/bluetooth/web_bluetooth_service_impl.cc:220: if
(!device->IsGattConnected() && !device->IsConnected()) {
On 2016/05/28 03:24:44, chromium-reviews wrote:
> IsGattConnected is not implemented on Chrome OS yet.

Does it make sense to implement it to forward to IsConnected() there, and then
have this call site do the long-term thing?

[ortuno, 2016-05-31 14:57:44]

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

[commit-bot: I haz the power, 2016-05-31 14:57:58]

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

[ortuno, 2016-05-31 16:33:21]

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2016973003/diff/1/content/browser/bluetooth/w...
content/browser/bluetooth/web_bluetooth_service_impl.cc:220: if
(!device->IsGattConnected() && !device->IsConnected()) {
On 2016/05/28 at 03:49:52, Jeffrey Yasskin wrote:
> On 2016/05/28 03:24:44, chromium-reviews wrote:
> > IsGattConnected is not implemented on Chrome OS yet.
> 
> Does it make sense to implement it to forward to IsConnected() there, and then
have this call site do the long-term thing?

Done. I thought it was going to be much harder so I went for the one line change
here, since we have to request a merge for this. But it turns out implementing
IsGattConnected was also a one line change :)

[Jeffrey Yasskin, 2016-05-31 16:36:19]

Thanks! LGTM.

[ortuno, 2016-05-31 16:36:59]

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

[ortuno, 2016-05-31 16:37:00]

scheib: PTAL at device/bluetooth

[commit-bot: I haz the power, 2016-05-31 16:44:21]

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

[commit-bot: I haz the power, 2016-05-31 16:44:22]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-05-31 18:34:26]

LGTM

[ortuno, 2016-05-31 18:38:18]

The CQ bit was checked by ortuno@chromium.org

[commit-bot: I haz the power, 2016-05-31 18:38:52]

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

[commit-bot: I haz the power, 2016-05-31 18:44:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-31 18:45:20]

Description was changed from

==========
bluetooth: Fix logic mistake in DeviceChanged

Fixes an issue in which we always disconnected a device even if the connection
is still active.

BUG=615383
==========

to

==========
bluetooth: Fix logic mistake in DeviceChanged

Fixes an issue in which we always disconnected a device even if the connection
is still active.

BUG=615383

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

[commit-bot: I haz the power, 2016-05-31 18:45:21]

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