Closed
Bug 972730
Opened 10 years ago
Closed 10 years ago
[BT][Mnw] We should not disable BT during BT enable procedure in marionette test.
Categories
(Firefox OS Graveyard :: Bluetooth, defect)
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 | ||
Comment 1•10 years ago
|
||
Assignee: nobody → jaliu
Status: NEW → ASSIGNED
Attachment #8376071 -
Flags: review?(echou)
Attachment #8376071 -
Flags: feedback?(vyang)
Assignee | ||
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
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+
Assignee | ||
Comment 4•10 years ago
|
||
It looks fine on TBPL https://tbpl.mozilla.org/?tree=Try&rev=4035975c0cc8
Comment 5•10 years ago
|
||
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+
Assignee | ||
Comment 6•10 years ago
|
||
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
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 7•10 years ago
|
||
(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.
Comment 8•10 years ago
|
||
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
Assignee | ||
Comment 9•10 years ago
|
||
Fix the hg patch metadata. Sorry for the inconvenience caused.
Attachment #8381318 -
Attachment is obsolete: true
Flags: needinfo?(jaliu)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/b07a62b9c5ab
Flags: in-testsuite+
Keywords: checkin-needed
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b07a62b9c5ab
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•