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

RESOLVED FIXED in Firefox 19

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ericchou, Assigned: ericchou)

Tracking

unspecified
B2G C3 (12dec-1jan)
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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)

Comment 1

6 years ago
Created attachment 695449 [details]
screenshot after low-level discovery has been already stopped
Assignee: nobody → echou
(Assignee)

Comment 2

6 years ago
Created attachment 695450 [details] [diff] [review]
patch 1: v1: the callback function set to dubs is usually not called

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)
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 4

6 years ago
Created attachment 695553 [details] [diff] [review]
patch 1: final: the callback function set to dubs is usually not called, r=gyeh

Problems solved
Attachment #695450 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 695575 [details] [diff] [review]
patch 1: final: the callback function set to dubs is usually not called, r=gyeh

Updated because of build error
Attachment #695553 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/914889120beb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/fa89be38bac7
https://hg.mozilla.org/releases/mozilla-b2g18/rev/b357c8c75904
status-b2g18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
You need to log in before you can comment on or make changes to this bug.