bluetooth: Detect and fix incorrect thread usage of BluetoothDispatcherHost.

bluetooth: Detect and fix incorrect thread usage of BluetoothDispatcherHost.

Android GATT connection state changes may arrive on threads other than UI.
However, this wasn't previously detected.

This patch adds a WeakPtr instance to ensure all future WeakPtrs created
by the factory enforce UI thread usage.

Android device implementation is adjusted to run callbacks from UI thread.

The nature of WeakPtr has been more clearly documented here:
https://codereview.chromium.org/1384863002/

BUG=537471
Committed: https://crrev.com/0e9bbf5fff69296187af509f460b813fc6c64e67
Cr-Commit-Position: refs/heads/master@{#352462}

[scheib, 2015-10-04 04:30:16]

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

[scheib, 2015-10-04 04:30:16]

qinmin: Please check my thread hopping in
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java

ortuno: PTAL overall.

[qinmin, 2015-10-05 18:38:54]

https://codereview.chromium.org/1389553002/diff/1/content/browser/bluetooth/b...
File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right):

https://codereview.chromium.org/1389553002/diff/1/content/browser/bluetooth/b...
content/browser/bluetooth/bluetooth_dispatcher_host.cc:182:
weak_ptr_factory_.GetWeakPtr()));
use weak_ptr_on_ui_thread_ and everywhere else in this file?

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
(right):

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:142:
if (mNativeBluetoothDeviceAndroid != 0) {
this is not threadsafe, see my comments below

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:146:
nativeOnConnectionStateChange(mNativeBluetoothDeviceAndroid, status,
when this gets called, mNativeBluetoothDeviceAndroid could become 0. So what
about doing the if (mNativeBluetoothDeviceAndroid!=0) here instead of in
onConnectionStateChange()?

[scheib, 2015-10-05 21:01:34]

https://codereview.chromium.org/1389553002/diff/1/content/browser/bluetooth/b...
File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right):

https://codereview.chromium.org/1389553002/diff/1/content/browser/bluetooth/b...
content/browser/bluetooth/bluetooth_dispatcher_host.cc:182:
weak_ptr_factory_.GetWeakPtr()));
On 2015/10/05 18:38:54, qinmin wrote:
> use weak_ptr_on_ui_thread_ and everywhere else in this file?

Done. [Obviously] I didn't think of that, but it is much clearer that way,
thanks.

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
(right):

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:142:
if (mNativeBluetoothDeviceAndroid != 0) {
On 2015/10/05 18:38:54, qinmin wrote:
> this is not threadsafe, see my comments below

Done.

https://codereview.chromium.org/1389553002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:146:
nativeOnConnectionStateChange(mNativeBluetoothDeviceAndroid, status,
On 2015/10/05 18:38:54, qinmin wrote:
> when this gets called, mNativeBluetoothDeviceAndroid could become 0. So what
> about doing the if (mNativeBluetoothDeviceAndroid!=0) here instead of in
> onConnectionStateChange()?

Done.

[ortuno, 2015-10-05 21:03:10]

LGTM. Does it make sense to add the DCHECK_CURRENTLY_ONs in this patch as well?

[qinmin, 2015-10-05 21:33:29]

lgtm

[scheib, 2015-10-05 21:45:12]

On 2015/10/05 21:03:10, ortuno wrote:
> LGTM. Does it make sense to add the DCHECK_CURRENTLY_ONs in this patch as
well?

Thanks, OK I've done that too.

[scheib, 2015-10-05 21:45:38]

The CQ bit was checked by scheib@chromium.org

[scheib, 2015-10-05 21:45:38]

The patchset sent to the CQ was uploaded after l-g-t-m from ortuno@chromium.org,
qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/1389553002/#ps40001
(title: "DCHECK_CURRENTLY_ON(BrowserThread::UI);")

[commit-bot: I haz the power, 2015-10-05 21:47:19]

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

[commit-bot: I haz the power, 2015-10-05 23:36:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-05 23:36:54]

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