Closed Bug 775375 Opened 10 years ago Closed 10 years ago

B2G 3G: Handle B2G "restarts" (let's be honest, crashes) gracefully

Categories

(Core :: DOM: Device Interfaces, defect)

Other Branch
ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17
blocking-basecamp +

People

(Reporter: philikon, Assigned: hsinyi)

References

Details

(Whiteboard: [soft])

Attachments

(2 files, 2 obsolete files)

When B2G is restarted, we lose track of which data call belongs to RILNetworkInterface. This is problematic because now we can't bring the data call down anymore, and changes to the status aren't going to through to the NetworkManager.

My preferred solution for this would be to nuke the site from orbit (only way to be sure): when Gecko starts up and finds the radio in anything other than "off", we should turn it off, and then depending on what ril.radio.disabled says, turn it back on or not. (This is how Android does it, btw).
Nom'ing for basecamp: required to let us deal with Gecko crashes gracefully.
blocking-basecamp: --- → ?
Assignee: nobody → htsai
blocking-basecamp: ? → +
Whiteboard: [soft]
Attached file Patch (obsolete) —
Hi Philipp, the patch addresses Comment #0. Does it meet your solution? Thanks.
Attachment #645635 - Flags: feedback?(philipp)
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> Created attachment 645635 [details]
> Patch
> 
This rebases on yoshi's patchi in bug 776480.
> Hi Philipp, the patch addresses Comment #0. Does it meet your solution?
> Thanks.

When I work on this, I notice that 'RadioInterfaceLayer.js' doesn't receive the message 'RIL:GetRilContext'. After investigating, I found that it might result from 'category profile-after-change' in RadioInterfaceLayer.manifest. However, I am not very sure whether something inappropriate in the manifest. Or, this behaviour (I mean, not receiving RIL:GetRilContext) shows nothing wrong?
Attachment #645635 - Attachment is obsolete: true
Attachment #645635 - Attachment is patch: false
Attachment #645635 - Flags: feedback?(philipp)
Attached patch Patch (obsolete) — Splinter Review
reset radioState at boot time.
Attachment #645654 - Flags: review?(philipp)
Comment on attachment 645654 [details] [diff] [review]
Patch

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

This is exactly what I had in mind. But we could do this directly in ril_worker.js, no?
Attachment #645654 - Flags: review?(philipp) → feedback+
Attached patch Patch (v2)Splinter Review
Comment #5 addressed. Thanks!
Attachment #645654 - Attachment is obsolete: true
Attachment #646041 - Flags: review?(philipp)
Comment on attachment 646041 [details] [diff] [review]
Patch (v2)

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

::: dom/system/gonk/ril_worker.js
@@ +3498,5 @@
> +    this._isInitialRadioState = false;
> +    if (radioState != RADIO_STATE_OFF) {
> +      let options = {};
> +      options.on = false;
> +      this.setRadioPower(options);

Can collapse these three lines to

  this.setRadioPower({on: false});
Attachment #646041 - Flags: review?(philipp) → review+
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Comment on attachment 646041 [details] [diff] [review]
> Patch (v2)
> 
> Review of attachment 646041 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +3498,5 @@
> > +    this._isInitialRadioState = false;
> > +    if (radioState != RADIO_STATE_OFF) {
> > +      let options = {};
> > +      options.on = false;
> > +      this.setRadioPower(options);
> 
> Can collapse these three lines to
> 
>   this.setRadioPower({on: false});

Thanks for pointing this out. Address in the final patch!
https://hg.mozilla.org/mozilla-central/rev/dfec8f1941b8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attached patch PatchSplinter Review
Just submit the final patch for further convenient reference.
You need to log in before you can comment on or make changes to this bug.