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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jgriffin, Assigned: philikon)

References

Details

Attachments

(3 files)

Attached file logcat output
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.
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.
(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.
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
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
Attached patch v1Splinter Review
Attachment #627072 - Flags: review?(kyle)
Attachment #627072 - Flags: review?(kyle) → review+
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
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)
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-
(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.
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c000fd454e7a
Component: DOM → DOM: Device Interfaces
QA Contact: general → device-interfaces
Target Milestone: --- → mozilla15
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.