797972 - B2G RIL: False roaming situation

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Beatriz Rodríguez [:brg]]

Attachment: device-2012-10-02-223022_SP-SIM_card.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

I am in Rio with a VIVO sim card that belongs to VIVO SaoPaulo area.
The mcc/mnc in Sao Paulo Vivo SIM card is: 724 10
The mcc/mnc in Rio is: 724 11

I am not in Roaming. Other devices with the Vivo SaoPaulo SIM card are not reporting roaming while staying in Rio.


Actual results:

I can see Roaming indicator ON while i am under VIVO coverage


Expected results:

I am not in Roaming.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

As I check the log
...
I/Gecko   (  107): RIL Worker: SPN: spn=VIVO, spnDisplayCondition=0
...
I/Gecko   (  255): -*- RILContentHelper: Received message 'RIL:VoiceInfoChanged': {"connected":true,"emergencyCallsOnly":false,"roaming":true,"network":{"rilMessageType":"operatorchange","longName":"Vivo S.A.","shortName":"Vivo","mcc":724,"mnc":11},"cell":{"gsmLocationAreaCode":42421,"gsmCellId":34170378},"type":"gsm","signalStrength":-51,"relSignalStrength":100,"state":"registered"}

SPN from is "VIVO", and short name is "Vivo"

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 668211 [details] [diff] [review]
Patch

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

I'll r+ this patch, but please fix nullity of short/long name.
Beside, since we can test operator names as done in bug 785988, please also have a test case for this issue before landing.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +618,5 @@
>      if (!icc || !registration.connected) {
>        return;
>      }
>  
> +    let spn = icc.spn.toLowerCase();

Could SPN be null?

@@ +623,3 @@
>      let operator = registration.network;
> +    let longName = operator.longName.toLowerCase();
> +    let shortName = operator.shortName.toLowerCase();

See bug 785988, longName, shortName could be null

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch v2.

check for null.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 2: test cases

Comment 6

[Marshall Culpepper [:marshall_law]]

Comment on attachment 671253 [details] [diff] [review]
Part 2: test cases

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

Looks good!

::: dom/network/tests/marionette/test_mobile_operator_names.js
@@ +105,5 @@
> +function doTestRoamingCheck(longName, shortName, callback) {
> +  log("Testing roaming check '" + longName + "', '" + shortName + "':");
> +
> +  setEmulatorOperatorNames(OPERATOR_ROAMING, longName, shortName,
> +                           window.setTimeout.bind(window, function () {

ooh. clever. :)

@@ +144,5 @@
> +            doTestRoamingCheck("android", "Xxx", function () {
> +              doTestRoamingCheck("Xxx", "Android", function () {
> +                doTestRoamingCheck("Xxx", "android", function () {
> +                  setEmulatorOperatorNames(OPERATOR_ROAMING, "TelKila", "TelKila",
> +                                           window.setTimeout.bind(window, cleanUp, 3000));

sigh. This deeply embedded callback pattern probably isn't a good choice for long term maintainability. I really wish we had some simple async helpers to do this kind of ordering for us..

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Marshall Culpepper [:marshall_law] from comment #6)
> @@ +144,5 @@
> > +            doTestRoamingCheck("android", "Xxx", function () {
> > +              doTestRoamingCheck("Xxx", "Android", function () {
> > +                doTestRoamingCheck("Xxx", "android", function () {
> > +                  setEmulatorOperatorNames(OPERATOR_ROAMING, "TelKila", "TelKila",
> > +                                           window.setTimeout.bind(window, cleanUp, 3000));
> 
> sigh. This deeply embedded callback pattern probably isn't a good choice for
> long term maintainability. I really wish we had some simple async helpers to
> do this kind of ordering for us..

I can have something like:

  function helper(func, array, oncomplete) {
    (function do_call(index) {
      let next = index < (array.length - 1) ? do_call.bind(null, index + 1) : oncomplete;
      func.apply(null, array[index].concat(next));
    })(0);
  }

  let argsArray = [
    ["1", "2"],
    ["3", "4"],
    ["5", "6"],
    ...
  ];
  helper(doTestRoamingCheck, argsArray, oncomplete);

How do you think? I can fire another bug to rewrite them all.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1602ef5c86e2
https://hg.mozilla.org/integration/mozilla-inbound/rev/4430d306f3f7

Comment 9

[Marshall Culpepper [:marshall_law]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7)
> I can have something like:
> 
>   function helper(func, array, oncomplete) {
>     (function do_call(index) {
>       let next = index < (array.length - 1) ? do_call.bind(null, index + 1)
> : oncomplete;
>       func.apply(null, array[index].concat(next));
>     })(0);
>   }
> 
>   let argsArray = [
>     ["1", "2"],
>     ["3", "4"],
>     ["5", "6"],
>     ...
>   ];
>   helper(doTestRoamingCheck, argsArray, oncomplete);
> 
> How do you think? I can fire another bug to rewrite them all.

That looks ok, but it's probably in the "nice to have" category. Go ahead and file, and we can ask jgriffin where the best place for such an async helper might live..

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1602ef5c86e2
https://hg.mozilla.org/mozilla-central/rev/4430d306f3f7