838421 - [b2g-bluetooth] After an instance of BluetoothAdapter is destroyed, no events can be received by other instances

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

There are at least two BluetoothAdapters in Settings app due to lazy loading. (One for the main page of Settings, the other for the subpage of Bluetooth). After entering the Bluetooth subpage, the first BluetoothAdapter will be released and unregistered. The root cause here is that other instances of BluetoothAdapter are accidentally removed in the meantime.

Comment 1

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

Attachment: Patch 1(v1): Register/Unregister BluetoothManager in constructor/destructor

No need to maintain a list of live managers in bluetooth service. For events onenabled/ondisabled, just dispatch them to instances of BluetoothManager.

For each instance of BluetoothManager, register signal handler after creation and unregister signal handler in destroy method. When turning off Bluetooth, all signal handlers except BluetoothManager are removed, so we don't need the original re-register mechanism during turn-on process.

Comment 2

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

Comment on attachment 710480 [details] [diff] [review]
Patch 1(v1): Register/Unregister BluetoothManager in constructor/destructor

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

r=me with nits addressed.

Looks decent! Thanks for cleaning it up!

::: dom/bluetooth/BluetoothManager.cpp
@@ +93,4 @@
>  private:
>    nsRefPtr<BluetoothManager> mManagerPtr;
>  };
>  

Since this function will not used anymore and is removed, we can also take it off from BluetoothManager.h

@@ +166,5 @@
>    nsRefPtr<BluetoothManager> manager = new BluetoothManager(aWindow);
>  
>    BluetoothService* bs = BluetoothService::Get();
>    NS_ENSURE_TRUE(bs, nullptr);
> +  bs->RegisterBluetoothSignalHandler(manager->GetPath(), manager);

We could move the registration function call into the ctor of BluetoothManager. That would be more consistent to register in ctor and unregister in dtor.

::: dom/bluetooth/BluetoothService.cpp
@@ +122,5 @@
>  
>      if (!gInShutdown) {
>        // Notify all the managers about the state change.
>        gBluetoothService->SetEnabled(mEnabled);
> +

Not-even-a-nit: The comment before the function call SetEnabled() "// Notify all the managers about the state change." should be moved here since the notifying managers part of code has been moved here.

Comment 3

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

Attachment: Final patch: Register/Unregister BluetoothManager in constructor/destructor, r=echou

Comment 4

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b5f6654d2a3

Comment 5

[Mozilla RelEng Bot]

Try run for 1505af3d10b3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=1505af3d10b3
Results (out of 14 total builds):
    success: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1505af3d10b3

Comment 6

[Mozilla RelEng Bot]

Try run for a370bff816fb is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a370bff816fb
Results (out of 18 total builds):
    success: 17
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a370bff816fb

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b5f6654d2a3

Comment 8

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

We have to land this patch on b2g18, otherwise events cannot be fired correctly. Nominate as tef?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please provide a branch-specific patch for uplift.

Comment 11

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

Attachment: final patch: for b2g18

b2g18-specific patch

Comment 12

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

Thanks, Eric. :)

Comment 13

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

Attachment: final patch: for b2g18

Patch updated. Please use this one for b2g18.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/744b05a192ad
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/c1cb31a7dc3b

Comment 15

[Allen Maxwell]

Attachment: Logcat of attempted headset pairings

Unable to pair to pairable, bt-enabled devices not found

This issue persists on the Inari v1.0.1 Mozilla RIL:
Platform version: 18.0
Build ID: 20130425070201
Gecko: 5d454864b668
Gaia: 052253176f3ad495a1086e47a28fffbf07613388

logcat attached

Comment 16

[Allen Maxwell]

Reopening bug as not resolved for Inari v1.0.1 Mozilla RIL

Comment 17

[Jason Smith [:jsmith]]

Please file a followup bug, don't reopen bugs. Link it as a dependency.

Comment 18

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

I tried the latest pvt build (20130425070201) and had no luck to reproduce this issue.

Here's some information extracted from sources.xml in the downloaded package.

Gecko: <project name="releases/mozilla-b2g18_v1_0_1" path="gecko" remote="hgmozillaorg" revision="5d454864b668"/> 
Gaia: <project name="gaia.git" path="gaia" remote="mozillaorg" revision="052253176f3ad495a1086e47a28fffbf07613388"/>



Please file a new bug if it happens again, thanks.