B2G RIL: Enable data connection then enter into no coverage area, the registration status is still 'registered'

RESOLVED FIXED in Firefox 28

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: edgar, Assigned: edgar)

Tracking

unspecified
1.3 C3/1.4 S3(31jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [FT:RIL])

Attachments

(6 attachments, 1 obsolete attachment)

Posted file foo.txt
+++ This bug was initially created as a clone of Bug #961927 +++

If data call is established and enter into no coverage area, the registration status is still 'registered'.
I think it is caused by below error, so that gecko doesn't update status successfully.

$$01-22 16:30:43.000   107   107 I Gecko   : -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"networkinfochanged","voiceRegistrationState":{"regState":0,"state":"notSearching","connected":false,"roaming":false,"emergencyCallsOnly":true,"cell":{"gsmLocationAreaCode":-1,"gsmCellId":-1},"radioTech":0,"type":null,"rilMessageType":"voiceregistrationstatechange"},"dataRegistrationState":{"regState":0,"state":"notSearching","connected":false,"roaming":false,"emergencyCallsOnly":true,"cell":{"gsmLocationAreaCode":-1,"gsmCellId":-1},"radioTech":0,"type":null,"rilMessageType":"dataregistrationstatechange"},"operator":{"rilMessageType":"operatorchange","longName":null,"shortName":null,"mcc":null,"mnc":null},"signal":{"voice":{"signalStrength":null,"relSignalStrength":null},"data":{"signalStrength":null,"relSignalStrength":null},"rilMessageType":"signalstrengthchange"}}
$$01-22 16:30:43.000   107   107 I Gecko   : -*- RadioInterface[0]: RIL is not ready for data connection: Phone's not registered or doesn't have data connection.
$$01-22 16:30:43.000   107   107 E GeckoConsole: [JavaScript Error: "operator is null" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 1690}]
After checking the RadioInterfaceLayer.js, I guess the JavaScript Error is happened in below function.

checkRoamingBetweenOperators: function(registration) {
  let iccInfo = this.rilContext.iccInfo;
  if (!iccInfo || !registration.connected) {
    return;
  }

  let spn = iccInfo.spn && iccInfo.spn.toLowerCase();
  let operator = registration.network;
  let longName = operator.longName && operator.longName.toLowerCase();
  let shortName = operator.shortName && operator.shortName.toLowerCase();

  let equalsLongName = longName && (spn == longName);
  let equalsShortName = shortName && (spn == shortName);
  let equalsMcc = iccInfo.mcc == operator.mcc;

  registration.roaming = registration.roaming &&
                         !(equalsMcc && (equalsLongName || equalsShortName));
}

I think I know the possible case to trigger this error.
But unfortunately I did not reproduce yet. :(
Posted file fugu.txt
Another log reproduced on my fugu device.
(In reply to Edgar Chen [:edgar][:echen] from comment #1)
> After checking the RadioInterfaceLayer.js, I guess the JavaScript Error is
> happened in below function.
> 
> checkRoamingBetweenOperators: function(registration) {
>   let iccInfo = this.rilContext.iccInfo;
>   if (!iccInfo || !registration.connected) {

1). We should check |network| here as well for the case that operator information is not ready yet.
2). Use |connected| as a condition is not so accurate. The |connected| of data is true only if the "default" connection is established. So for the case that device doesn't have "default" connection, the |roaming| of data will not be updated correctly. So I propose check |state| instead.

------
  if (!iccInfo ||
      !registration.network ||
      registartion.state != "registered"){
    return;
  }
-----

>     return;
>   }
> 
>   let spn = iccInfo.spn && iccInfo.spn.toLowerCase();
>   let operator = registration.network;
>   let longName = operator.longName && operator.longName.toLowerCase();
>   let shortName = operator.shortName && operator.shortName.toLowerCase();
> 
>   let equalsLongName = longName && (spn == longName);
>   let equalsShortName = shortName && (spn == shortName);
>   let equalsMcc = iccInfo.mcc == operator.mcc;
> 
>   registration.roaming = registration.roaming &&
>                          !(equalsMcc && (equalsLongName || equalsShortName));
> }
Now I am working on marionette test cases.
Block bug 961927 which is 1.3?, so mark as 1.3? as well.
blocking-b2g: --- → 1.3?
Detailed information about this patch:
1). Put common code in mobile_header.js.
2). Refactor test_mobile_voice_state.js.
- Include mobile_header.js
- Move the cell information tests for voice to a separated file, test_mobile_voice_location.js. (Just like we have test_mobile_data_location.js for data)
- In original cell information tests, we did not restore cell information to default, it will affect subsequent tests. Correct this missing.
3). Refactor test_mobile_data_state.js.
- Include mobile_header.js
- Add more tests for roaming status of data.

(((Note: I think we should also refactor other tests. Let's do it in another bug. ;))))
Comment on attachment 8364953 [details] [diff] [review]
Part 1: Fix the JavaScript error thrown in checkRoamingBetweenOperators() when the operator information is not available, v1

Please see comment #3. Thank you.
Attachment #8364953 - Flags: review?(htsai)
Comment on attachment 8365846 [details] [diff] [review]
Part 2: Refactor test_mobile_{voice|data}_state.js and add more tests for data roaming status, v1

Please see comment #7. Thank you.
Attachment #8365846 - Flags: review?(htsai)
Comment on attachment 8364953 [details] [diff] [review]
Part 1: Fix the JavaScript error thrown in checkRoamingBetweenOperators() when the operator information is not available, v1

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

Makes sense, thanks.
Attachment #8364953 - Flags: review?(htsai) → review+
Comment on attachment 8365846 [details] [diff] [review]
Part 2: Refactor test_mobile_{voice|data}_state.js and add more tests for data roaming status, v1

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

Thanks for the patch! Please remember to file another bug for refactoring the remaining test cases.

::: dom/mobileconnection/tests/marionette/mobile_header.js
@@ +44,5 @@
> +    this.pendingCommandCount++;
> +    runEmulatorCmd(cmd, function(results) {
> +      this.pendingCommandCount--;
> +
> +      let result = results[results.length - 1]

A semicolon is missing.

::: dom/mobileconnection/tests/marionette/test_mobile_data_state.js
@@ +70,4 @@
>  
> +      let cell = data.cell;
> +      if (!expect.cell) {
> +        is(cell, expect.cell, "check data.cell");

nit: ok(!cell, "check data.cell");

::: dom/mobileconnection/tests/marionette/test_mobile_voice_state.js
@@ +68,4 @@
>  
> +      let cell = voice.cell;
> +      if (!expect.cell) {
> +        is(cell, expect.cell, "check voice.cell");

nit: ok(!cell, "check voice.cell");
Attachment #8365846 - Flags: review?(htsai) → review+
(In reply to Hsin-Yi Tsai (OOO Jan. 29 ~ Feb. 5th) [:hsinyi] from comment #11)
> Comment on attachment 8365846 [details] [diff] [review]
> Part 2: Refactor test_mobile_{voice|data}_state.js and add more tests for
> data roaming status, v1
> 
> Review of attachment 8365846 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the patch! Please remember to file another bug for refactoring
> the remaining test cases.
Bug 964680.

> 
> ::: dom/mobileconnection/tests/marionette/mobile_header.js
> @@ +44,5 @@
> > +    this.pendingCommandCount++;
> > +    runEmulatorCmd(cmd, function(results) {
> > +      this.pendingCommandCount--;
> > +
> > +      let result = results[results.length - 1]
> 
> A semicolon is missing.
Aha, thanks for catching this.
(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> Block bug 961927 which is 1.3?, so mark as 1.3? as well.

This bug blocks bug 961927 which was marked as 1.3+ already.
I think this bug should be 1.3+ as well. Could someone help to triage this?
Thank you.
(In reply to Edgar Chen [:edgar][:echen] from comment #15)
> (In reply to Edgar Chen [:edgar][:echen] from comment #6)
> > Block bug 961927 which is 1.3?, so mark as 1.3? as well.
> 
> This bug blocks bug 961927 which was marked as 1.3+ already.
> I think this bug should be 1.3+ as well. Could someone help to triage this?
> Thank you.

I already +ed the bug.
(In reply to Jason Smith [:jsmith] from comment #16)
> (In reply to Edgar Chen [:edgar][:echen] from comment #15)
> > (In reply to Edgar Chen [:edgar][:echen] from comment #6)
> > > Block bug 961927 which is 1.3?, so mark as 1.3? as well.
> > 
> > This bug blocks bug 961927 which was marked as 1.3+ already.
> > I think this bug should be 1.3+ as well. Could someone help to triage this?
> > Thank you.
> 
> I already +ed the bug.

Oh wait - wrong bug - this one I haven't triaged yet. Fixing this now.
blocking-b2g: 1.3? → 1.3+
Whiteboard: [FT:RIL]
https://hg.mozilla.org/mozilla-central/rev/b8a673c86b0b
https://hg.mozilla.org/mozilla-central/rev/53ed3e402dca
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Duplicate of this bug: 759549
You need to log in before you can comment on or make changes to this bug.