Closed Bug 824458 Opened 7 years ago Closed 7 years ago

[Bluetooth] Unable to search devices again after calling StopDiscovery()

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: echou, Assigned: echou)

Details

Attachments

(2 files, 2 obsolete files)

Currently Gaia calls adapter.StopDiscovery() to stop searching Bluetooth devices whenever leaving Settings page or timed out, but Evelyn found that onsuccess/onerror of the DOM Request returned by StopDiscovery() are usually not fired. That would result in a wrong state of Bluetooth UI (still shows "Searching for devices" and "Search" button wouldn't appear even if discovery has been stopped). Please see the attached photo for more detail.

This is a follow-up to bug 820166. You can also get more information from bug 820166 comment 9.
Assignee: nobody → echou
The root cause is we used the asynchronous dbus function call and hit a known issue of DBus, please see https://bugs.freedesktop.org/show_bug.cgi?id=19796 for details of the bug. It is a timing issue. If the response of the target function returned faster than callback function is set, then the callback would never be invoked.

To solve this, I replaced original async function call with blocking calls on non-main thread.
Attachment #695450 - Flags: review?(gyeh)
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Target Milestone: --- → B2G C3 (12dec-1jan)
Comment on attachment 695450 [details] [diff] [review]
patch 1: v1: the callback function set to dubs is usually not called

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

Looks good to me.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +161,5 @@
> +                   BluetoothReplyRunnable* aRunnable)
> +    : mAdapterPath(aAdapterPath)
> +    , mDeviceObjectPath(aDeviceObjectPath)
> +    , mRunnable(aRunnable)
> +  {

We'd like to make sure that either aDeviceObjectPath or aRunnable is not null.
MOZ_ASSERT(aDeviceObjectPath && aRunnable);

@@ +192,5 @@
> +
> +private:
> +  nsString mAdapterPath;
> +  const char* mDeviceObjectPath;
> +  BluetoothReplyRunnable* mRunnable;

How about using reference counting pointers nsRefPtr here?
nsRefPtr<BluetoothReplyRunnable> mRunnable;

@@ +203,5 @@
> +                    BluetoothReplyRunnable* aRunnable)
> +    : mAdapterPath(aAdapterPath)
> +    , mMessageName(aMessageName)
> +    , mRunnable(aRunnable)
> +  {

Add assertion to make sure both aMessageName and aRunnable aren't null.
MOZ_ASSERT(aMessageName && aRunnable);

@@ +230,5 @@
> +
> +private:
> +  nsString mAdapterPath;
> +  const char* mMessageName;
> +  BluetoothReplyRunnable* mRunnable;

Using nsRefPtr<BluetoothReplyRunnable>
Attachment #695450 - Flags: review?(gyeh) → review+
Problems solved
Attachment #695450 - Attachment is obsolete: true
Updated because of build error
Attachment #695553 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/914889120beb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.