Closed
Bug 758466
Opened 12 years ago
Closed 12 years ago
B2G RIL: ensure radio state and 'ril.radio.disabled' setting value are known before modifying the radio state
Categories
(Core :: DOM: Device Interfaces, defect)
Core
DOM: Device Interfaces
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: jgriffin, Assigned: philikon)
References
Details
Attachments
(3 files)
41.30 KB,
text/plain
|
Details | |
3.90 KB,
patch
|
qdot
:
review+
|
Details | Diff | Splinter Review |
981 bytes,
patch
|
philikon
:
feedback-
|
Details | Diff | Splinter Review |
All three Marionette dialer tests are failing in the same spot, while the receiver emulator is waiting for an incoming call. See http://ec2-107-20-108-245.compute-1.amazonaws.com/jenkins/job/webapi-marionette-test/8/console I can reproduce the failures locally. There is one unusual error in logcat: E/GeckoConsole( 41): [JavaScript Error: "this._messages is null" {file: "resource://gre/modules/DOMRequestHelper.jsm" line: 88}] Full logcat attached. This is a recent regression; builds from a few days ago passed; but with latest m-c they fail. Unfortunately, the CI wasn't running during this time so I can't say what changeset caused the regression. The failing test locations: http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_dial_answer.py#45 http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_dial_between_emulators.py#43 http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_dial_listeners.py#58 My B2G phone doesn't have a sim card, so I can't say whether calls from a B2G phone are actually getting made or not.
Assignee | ||
Comment 1•12 years ago
|
||
Yup. This is a regression from bug 744709. We now no longer turn the radio on automatically but based on a setting. We'll have to modify these tests to turn the radio on first.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #0) > E/GeckoConsole( 41): [JavaScript Error: "this._messages is null" {file: > "resource://gre/modules/DOMRequestHelper.jsm" line: 88}] I think this is unrelated. Potentially fall out from bug 757512.
Assignee | ||
Comment 3•12 years ago
|
||
Alright, I *thought* I was on crack, because bug 744709 shouldn't have regressed us. Turns out, it's susceptible to a race condition that probably doesn't occur on phones but does occur on the emulator with its fake (and therefore probably much faster) RIL. We have two events occurring: asking the settings DB for the initial value of 'ril.radio.disabled', and waiting for the RIL to tell us what the current state is. We can only act and mutate the radio state when we know both! Patch coming up.
Assignee: nobody → philipp
Blocks: 744709
Assignee | ||
Updated•12 years ago
|
Summary: Marionette dialer tests are failing because no incoming event is fired for incoming calls → B2G RIL: ensure radio state and 'ril.radio.disabled' setting value are known before modifying the radio state
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #627072 -
Flags: review?(kyle)
Updated•12 years ago
|
Attachment #627072 -
Flags: review?(kyle) → review+
Comment 5•12 years ago
|
||
Comment on attachment 627072 [details] [diff] [review] v1 Review of attachment 627072 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +456,5 @@ > + }, > + > + _ensureRadioState: function _ensureRadioState() { > + debug("Reported radio state is " + this.radioState.radioState + > + ", desired radio enabled state is " + this.radioEnabled); s/this.radioEnabled/this._radioEnabled
Comment 6•12 years ago
|
||
A way to set up a default value for 'ril.radio.disabled' setting. Ops! I have noticed right now this patch sets always 'ril.radio.disabled' to false. If the initial value is not 'null' is because the user set up a value and we have to keep it. Anyway that's the idea for setting up an initial value for 'ril.radio.disable' setting. I'll change a new WIP asap.
Attachment #627215 -
Flags: feedback?(philipp)
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 627215 [details] [diff] [review] WIP v1 - Part II: Default value for 'ril.radio.disabled' setting. Review of attachment 627215 [details] [diff] [review]: ----------------------------------------------------------------- We set default prefs in the Gaia install part. Talk to gwagner, please. In any case, this should be a separate bug.
Attachment #627215 -
Flags: feedback?(philipp) → feedback-
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #5) > ::: dom/system/gonk/RadioInterfaceLayer.js > @@ +456,5 @@ > > + }, > > + > > + _ensureRadioState: function _ensureRadioState() { > > + debug("Reported radio state is " + this.radioState.radioState + > > + ", desired radio enabled state is " + this.radioEnabled); > > s/this.radioEnabled/this._radioEnabled Thanks for spotting this. I'll fix that before pushing.
Comment 9•12 years ago
|
||
(In reply to Philipp von Weitershausen [:philikon] from comment #7) > Comment on attachment 627215 [details] [diff] [review] > WIP v1 - Part II: Default value for 'ril.radio.disabled' setting. > > Review of attachment 627215 [details] [diff] [review]: > ----------------------------------------------------------------- > > We set default prefs in the Gaia install part. Ok, I did not know that. Thanks.
Assignee | ||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c000fd454e7a
Component: DOM → DOM: Device Interfaces
QA Contact: general → device-interfaces
Target Milestone: --- → mozilla15
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c000fd454e7a
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•