936995 - [bluedroid] Bluetooth gets stuck at turning-on state if users turns on/off Bluetooth fast

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Bluetooth gets stuck at turning-on state when fast turning on/off.

Comment 1

[Eric Chou [:ericchou] [:echou]]

Steal! :)

Comment 2

[Eric Chou [:ericchou] [:echou]]

Attachment: patch 1: v1: fire event 'Disable' after Bluetooth is really disabled

The root cause of this issue is that, when Bluedroid is our BT backend, turning on/off Bluetooth becomes an async call, which means that event 'Disable' would be fired earlier than the adapter state actually changed to BT_STATE_OFF. I used a lock to wait for AdapterStateChangeCallback being invoked, so that both StopInternal() in BluetoothDBusService and BluetoothServiceBludroid would be sync call.

Comment 3

[Eric Chou [:ericchou] [:echou]]

I've tested my patch on both Nexus 4(bluedroid) and Unagi(blueZ). Both worked fine.

Comment 4

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 832857 [details] [diff] [review]
patch 1: v1: fire event 'Disable' after Bluetooth is really disabled

Review of attachment 832857 [details] [diff] [review]:
-----------------------------------------------------------------

This patch reminds me that we couldn't guarantee event |onenabled| is always dispatched before event |onadapteradded|. Please file a follow-up. Thanks.

::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp
@@ +180,5 @@
> +                           NS_LITERAL_STRING(KEY_MANAGER), true);
> +    nsRefPtr<DistributeBluetoothSignalTask>
> +      t = new DistributeBluetoothSignalTask(signal);
> +    if (NS_FAILED(NS_DispatchToMainThread(t))) {
> +      NS_WARNING("Failed to dispatch to main thread!");

nit: BT_WARNING

@@ +649,3 @@
>    int ret = aShouldEnable ? sBtInterface->enable() : sBtInterface->disable();
> +  if (ret == BT_STATUS_SUCCESS) {
> +    lock.Wait();

We could just return NS_OK inside the if-statement, and, in the following statement, return NS_ERROR_FAILURE directly.

Comment 5

[Eric Chou [:ericchou] [:echou]]

> 
> This patch reminds me that we couldn't guarantee event |onenabled| is always
> dispatched before event |onadapteradded|. Please file a follow-up. Thanks.
> 

Hm, good point. Just checked the mechanism and found out that this could happen on both BlueZ and Bluedroid implementation. Although it shouldn't be a problem for now, still worth to improve it. I'll file a followup, thanks.

Comment 6

[Eric Chou [:ericchou] [:echou]]

followup: bug 940834

Comment 7

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/integration/b2g-inbound/rev/40ecdab580b3

Comment 8

[Eric Chou [:ericchou] [:echou]]

Backed out: https://hg.mozilla.org/integration/b2g-inbound/rev/20bd40612db5

Comment 9

[Eric Chou [:ericchou] [:echou]]

b2g-inbound again: https://hg.mozilla.org/integration/b2g-inbound/rev/a67b11842906

Comment 10

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/mozilla-central/rev/a67b11842906

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

already in master 1.3, clear flag