Closed Bug 972730 Opened 6 years ago Closed 6 years ago

[BT][Mnw] We should not disable BT during BT enable procedure in marionette test.

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jaliu, Assigned: jaliu)

References

Details

Attachments

(1 file, 2 obsolete files)

The current Bluetooth API can't allow user to disable BT before the BT adapter is initialized.
In practice, We block the clicking event to prevent user to turn off BT before BT enable procedure complete.
If we use marionette test to force emulator turn on and turn off BT in a short period of time, it may crash emulator.
Assignee: nobody → jaliu
Status: NEW → ASSIGNED
Attachment #8376071 - Flags: review?(echou)
Attachment #8376071 - Flags: feedback?(vyang)
The marionette test test_dom_BluetoothManager_enabled.js would set BT enable and reset back to disabled in a short period of time. There is a 25 percent chance to crash the emulator with this test on my PC.

The design of the test is logical and rational, however, the current BT API couldn't handle it.
If user try to toggle BT switch twice in a short period of time, we would block it in Gaia layer.

To avoid crashing emulator on Tinterbox, I modified the test behavior to fit the normal user behavior.
Comment on attachment 8376071 [details] [diff] [review]
[Mnw] Avoid to disable BT before the BT adapter is initialized in marionette test. (v1)

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

I'm ok with the change. Jamin, per our offline discussion, please file a followup for tracking the Gecko issue. When the 'enabled' event is fired, we should be able to call disable() without any problems.
Attachment #8376071 - Flags: review?(echou) → review+
See Also: → 973482
Comment on attachment 8376071 [details] [diff] [review]
[Mnw] Avoid to disable BT before the BT adapter is initialized in marionette test. (v1)

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

Have no strong opinion here, but please embed the follow-up bug id in the comments, so that it reminds us every time you read this file.
Attachment #8376071 - Flags: feedback?(vyang) → feedback+
In this patch, the test case listens 'adapteradded' rather than 'enabled' since the current API can't
disable BT before the BT adapter is initialized.

We should listen to 'enabled' when gecko can handle the case I mentioned above.
Please refer to the follow-up bug 973482.
(I wrote the bug id and explanation into file as code comments, in case we forgot it .)
Attachment #8376071 - Attachment is obsolete: true
Keywords: checkin-needed
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #5)
> Comment on attachment 8376071 [details] [diff] [review]
> [Mnw] Avoid to disable BT before the BT adapter is initialized in marionette
> test. (v1)
> 
> Review of attachment 8376071 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Have no strong opinion here, but please embed the follow-up bug id in the
> comments, so that it reminds us every time you read this file.

Good idea.
Thank you.
patching file dom/bluetooth/tests/marionette/test_dom_BluetoothManager_enabled.js
bad hunk #1 @@ -10,16 +10,18 @@ MARIONETTE_HEAD_JS = 'head.js';
 (16 16 20 18)
patch failed, rejects left in working dir
Flags: needinfo?(jaliu)
Keywords: checkin-needed
Fix the hg patch metadata.
Sorry for the inconvenience caused.
Attachment #8381318 - Attachment is obsolete: true
Flags: needinfo?(jaliu)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b07a62b9c5ab
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.