bluetooth: Close and null out BluetoothGatt on the UI Thread

bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task http://crrev.com/2707823002
BUG=691407
Review-Url: https://codereview.chromium.org/2702163002
Cr-Commit-Position: refs/heads/master@{#455038}
Committed: https://chromium.googlesource.com/chromium/src/+/1c929fc283a435c85b073e317bfb65c5957a1d81

[ortuno, 2017-02-20 05:00:08]

Description was changed from

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

BUG=691407
==========

to

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task.
BUG=691407
==========

[ortuno, 2017-02-20 05:55:24]

Description was changed from

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task.
BUG=691407
==========

to

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task
http://crrev.com/2707823002
BUG=691407
==========

[ortuno, 2017-02-20 05:56:03]

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

[commit-bot: I haz the power, 2017-02-20 05:56: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, 2017-02-20 07:01:26]

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

[commit-bot: I haz the power, 2017-02-20 07:01:27]

Dry run: This issue passed the CQ dry run.

[ortuno, 2017-02-21 10:44:51]

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

[ortuno, 2017-02-21 10:44:52]

jyasskin: PTAL

[ortuno, 2017-02-27 23:06:38]

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

[ortuno, 2017-02-27 23:06:38]

scheib: PTAL

[scheib, 2017-03-04 18:52:25]

lgtm

https://codereview.chromium.org/2702163002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc (right):

https://codereview.chromium.org/2702163002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:2388: // Don't
run the disconnect task.
// Do not yet call RunUntilIdle() to process the disconnect task.
(and for below, just being more explicit)

[ortuno, 2017-03-07 03:45:49]

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

[commit-bot: I haz the power, 2017-03-07 03:46:45]

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, 2017-03-07 04:17:00]

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

[commit-bot: I haz the power, 2017-03-07 04:17:00]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[ortuno, 2017-03-07 04:23:33]

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

[commit-bot: I haz the power, 2017-03-07 04:24:07]

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

[ortuno, 2017-03-07 05:06:47]

Thanks!

https://codereview.chromium.org/2702163002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc (right):

https://codereview.chromium.org/2702163002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:2388: // Don't
run the disconnect task.
On 2017/03/04 at 18:52:24, scheib wrote:
> // Do not yet call RunUntilIdle() to process the disconnect task.
> (and for below, just being more explicit)

Makes sense. Done.

[commit-bot: I haz the power, 2017-03-07 05:19:31]

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

[commit-bot: I haz the power, 2017-03-07 05:19:32]

Dry run: This issue passed the CQ dry run.

[ortuno, 2017-03-07 05:27:31]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2017-03-07 05:27:31]

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/2702163002/#ps40001
(title: "Fix test")

[commit-bot: I haz the power, 2017-03-07 05:28:17]

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, 2017-03-07 05:34:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488864451402040,
"parent_rev": "afc9c240ef4f83bf23dfeec704f231f08c5c8121", "commit_rev":
"1c929fc283a435c85b073e317bfb65c5957a1d81"}

[commit-bot: I haz the power, 2017-03-07 05:35:23]

Description was changed from

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task
http://crrev.com/2707823002
BUG=691407
==========

to

==========
bluetooth: Close and null out BluetoothGatt on the UI Thread

There was a race between posting a task to notify of disconnection and gatt
operations:

1. On Bluetooht thread, device disconnects event arrives, nulls
   mBluetoothGatt and posts a task to the UI thread to notify of
   disconnection.
2. Before the disconnection task runs on the UI thread, Gatt Operation
   thinks device is still connected and tries to use mBluetoothGatt. This
   causes a NullPointerException.

Changes OnConnectionStateChanged immediately post to the UI thread.

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread (This patch).
 [2.2] Copy characteristic's value before posting task
http://crrev.com/2707823002
BUG=691407

Review-Url: https://codereview.chromium.org/2702163002
Cr-Commit-Position: refs/heads/master@{#455038}
Committed:
https://chromium.googlesource.com/chromium/src/+/1c929fc283a435c85b073e317bfb...
==========

[commit-bot: I haz the power, 2017-03-07 05:35:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1c929fc283a435c85b073e317bfb...