Last Comment Bug 775375 - B2G 3G: Handle B2G "restarts" (let's be honest, crashes) gracefully
: B2G 3G: Handle B2G "restarts" (let's be honest, crashes) gracefully
Status: RESOLVED FIXED
[soft]
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Other Branch
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Hsin-Yi Tsai [:hsinyi]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: b2g-3g
  Show dependency treegraph
 
Reported: 2012-07-18 17:33 PDT by Philipp von Weitershausen [:philikon]
Modified: 2012-08-07 01:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch (2.54 KB, text/plain)
2012-07-24 20:30 PDT, Hsin-Yi Tsai [:hsinyi]
no flags Details
Patch (1.62 KB, patch)
2012-07-24 22:40 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: feedback+
Details | Diff | Splinter Review
Patch (v2) (1.65 KB, patch)
2012-07-25 23:16 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review+
Details | Diff | Splinter Review
Patch (1.51 KB, patch)
2012-08-07 01:43 PDT, Hsin-Yi Tsai [:hsinyi]
no flags Details | Diff | Splinter Review

Description Philipp von Weitershausen [:philikon] 2012-07-18 17:33:12 PDT
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).
Comment 1 Philipp von Weitershausen [:philikon] 2012-07-18 17:33:50 PDT
Nom'ing for basecamp: required to let us deal with Gecko crashes gracefully.
Comment 2 Hsin-Yi Tsai [:hsinyi] 2012-07-24 20:30:05 PDT
Created attachment 645635 [details]
Patch

Hi Philipp, the patch addresses Comment #0. Does it meet your solution? Thanks.
Comment 3 Hsin-Yi Tsai [:hsinyi] 2012-07-24 20:35:52 PDT
(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?
Comment 4 Hsin-Yi Tsai [:hsinyi] 2012-07-24 22:40:28 PDT
Created attachment 645654 [details] [diff] [review]
Patch

reset radioState at boot time.
Comment 5 Philipp von Weitershausen [:philikon] 2012-07-25 11:54:10 PDT
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?
Comment 6 Hsin-Yi Tsai [:hsinyi] 2012-07-25 23:16:50 PDT
Created attachment 646041 [details] [diff] [review]
Patch (v2)

Comment #5 addressed. Thanks!
Comment 7 Philipp von Weitershausen [:philikon] 2012-07-26 16:21:50 PDT
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});
Comment 8 Hsin-Yi Tsai [:hsinyi] 2012-07-27 00:50:24 PDT
(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!
Comment 10 Ed Morley [:emorley] 2012-07-27 08:14:08 PDT
https://hg.mozilla.org/mozilla-central/rev/dfec8f1941b8
Comment 11 Hsin-Yi Tsai [:hsinyi] 2012-08-07 01:43:54 PDT
Created attachment 649571 [details] [diff] [review]
Patch

Just submit the final patch for further convenient reference.

Note You need to log in before you can comment on or make changes to this bug.