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

RESOLVED FIXED in Firefox 21

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: gyeh, Assigned: gyeh)

Tracking

21 Branch
mozilla21
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

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.
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.
Attachment #710480 - Flags: review?(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.
Attachment #710480 - Flags: review?(echou) → review+
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/gyeh@mozilla.com-1505af3d10b3
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/gyeh@mozilla.com-a370bff816fb
https://hg.mozilla.org/mozilla-central/rev/3b5f6654d2a3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
We have to land this patch on b2g18, otherwise events cannot be fired correctly. Nominate as tef?
blocking-b2g: --- → tef?
Duplicate of this bug: 856224
blocking-b2g: tef? → tef+
Attachment #711719 - Flags: approval-mozilla-b2g18+
Please provide a branch-specific patch for uplift.
Posted patch final patch: for b2g18 (obsolete) — Splinter Review
b2g18-specific patch
Thanks, Eric. :)
Patch updated. Please use this one for b2g18.
Attachment #734309 - Attachment is obsolete: true
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
Reopening bug as not resolved for Inari v1.0.1 Mozilla RIL
Status: RESOLVED → REOPENED
Keywords: smoketest
Resolution: FIXED → ---
Please file a followup bug, don't reopen bugs. Link it as a dependency.
Status: REOPENED → RESOLVED
Closed: 7 years ago6 years ago
Keywords: smoketest
Resolution: --- → FIXED
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.
You need to log in before you can comment on or make changes to this bug.