782588 - [b2g-bluetooth] mozBluetooth is not a singleton

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Evelyn Hung]

In System app, if we call mozBluetooth.setEnabled(true), mozBluetooth.enabled will be true in nsIDOMDOMRequest onsuccess callback, but mozBluetooth.enabled won't be changed (still false) on Settings app at the moment.

Comment 1

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

Currently, system and settings are two applications which have called window.navigator.mozBluetooth.setEnabled() to toggle Bluetooth. However, because they are different 'window' object, so they will create different BluetoothManager object. That's why the value of window.navigator.mozBluetooth.enabled in system is not the same as in settings.

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

The problem with "enabled" is that it's the only variable we work with that's not backed by the actual operating system (in this case, bluez/dbus). In every other case, any time we change a property, it's send to the system via a dbus call, and relayed back to us via a dbus signal. Since ALL objects on all windows are registered for the signal, they all get updated.

There's a couple of things we could do here. Easiest would be to stop caching the enabled value and ALWAYS call bluedroid's "isEnabled" function (name may actually be different but you get the idea). We could move to watching another DBus variable for enabled (I'm all for validity via "do we have any adapters available" by checking the DBus Manager object's ListAdapters function, as that also fixes bug 764559).

Comment 3

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

I'll implement choice one first, then I'll try to figure out if there's any other solutions for bug 764559.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I've got this Eric!

Comment 5

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

(In reply to ben turner [:bent] from comment #4)
> I've got this Eric!

No problem, thanks! :D

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Ok, this makes gaia tip work like it's supposed to (in-process). This patch does a few things:

1. Loads the bluetooth service at startup. It reads the settings to see if bluetooth is enabled and then starts the firmware load if it is.

2. Makes the bluetooth service watch for the settings changes to enable or disable bluetooth rather than each individual manager object. Notifies all live managers when the enabled/disabled value changes successfully so that they can fire events.

3. Moves the IsEnabled logic into the service so that managers can't get out of sync.

There's a little cleanup along the way but nothing too egregious (I hope).

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 658761 [details] [diff] [review]
Patch, v1

Oh, and it fixes some shutdown ordering and safety problems that I found along the way.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8d15eedf9ec

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Some kind of build problem: https://hg.mozilla.org/integration/mozilla-inbound/rev/68092493156f

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

/builds/slave/m-in-ics-armv7a-gecko-dbg/build/dom/bluetooth/BluetoothService.cpp:289: error: 'class nsISettingsService' has no member named 'GetLock'

Hoisted by your own petard! :)

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yeah... It's for the best!

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ee117877c93

Comment 13

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Aaaand there was a missing debug-only include in the gonk stuff. Grumble.

https://hg.mozilla.org/integration/mozilla-inbound/rev/de6286cb4539

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0ee117877c93
https://hg.mozilla.org/mozilla-central/rev/de6286cb4539