Closed Bug 963516 Opened 6 years ago Closed 6 years ago

B2G RIL: provide matchMvno() API for automatic selection of APN relying on mvnoType and matchData

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set

Tracking

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

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: hsinyi, Assigned: jessica)

References

Details

(Whiteboard: [FT:RIL])

Attachments

(4 files, 15 obsolete files)

6.32 KB, patch
hsinyi
: review+
jessica
: feedback+
Details | Diff | Splinter Review
2.71 KB, patch
jessica
: review+
jessica
: feedback+
Details | Diff | Splinter Review
1.77 KB, patch
jessica
: review+
Details | Diff | Splinter Review
2.63 KB, patch
jessica
: review+
jessica
: feedback+
Details | Diff | Splinter Review
nominate as 1.3+ because it blocks 1.3+ bug 947855, which is required for certification process in a newly launch country.
blocking-b2g: --- → 1.3?
Jessica is working on this bug.
Flags: needinfo?(jjong)
Assignee: nobody → jjong
Flags: needinfo?(jjong)
Attached patch Part 1: interface changes. (obsolete) — Splinter Review
Interface changes to support mvnoMatches().
Attachment #8365797 - Flags: review?(htsai)
Attachment #8365797 - Flags: feedback?(echen)
Attached patch Part 2: dom changes. (obsolete) — Splinter Review
Dom changes to support mvnoMatches().
Attachment #8365799 - Flags: feedback?(htsai)
Attachment #8365799 - Flags: feedback?(echen)
Attached patch Part 3: ril implementation. (obsolete) — Splinter Review
RIL implementation to support mvnoMatches().
Attachment #8365800 - Flags: review?(htsai)
Attachment #8365800 - Flags: feedback?(echen)
Attached patch Part 4: test cases. (obsolete) — Splinter Review
Test cases for mvnoMatches().
Attachment #8365801 - Flags: review?(htsai)
Attachment #8365801 - Flags: feedback?(echen)
Attachment #8365797 - Flags: review?(htsai) → review+
Attachment #8365799 - Flags: feedback?(htsai) → feedback+
Comment on attachment 8365797 [details] [diff] [review]
Part 1: interface changes.

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

::: dom/webidl/MozIcc.webidl
@@ +380,5 @@
> +   *
> +   * @return true if aMatchData matches the ICC's field indicated by aMvnoType,
> +  *          false otherwise.
> +   */
> +  boolean mvnoMatches(DOMString aMvnoType, DOMString aMatchData);

I wonder if we need to use emun for mvno type. We could do it in a followup.
Thank you.
Attachment #8365797 - Flags: feedback?(echen) → feedback+
Comment on attachment 8365800 [details] [diff] [review]
Part 3: ril implementation.

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

r=me with comments addressed, thank you!

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1642,5 @@
>  
>      return iccId;
>    },
>  
> +  imsiMatches: function(mvnoData) {

Please add a comment explaining 'x' and 'X'.
Ex: https://android.googlesource.com/platform/frameworks/opt/telephony/+/master/src/java/com/android/internal/telephony/dataconnection/DcTracker.java

@@ +1652,5 @@
> +      return false;
> +    }
> +
> +    for (let i = 0; i < mvnoData.length; i++) {
> +      let c = mvnoData.charAt(i);

c = mvnoData[i]

@@ +1653,5 @@
> +    }
> +
> +    for (let i = 0; i < mvnoData.length; i++) {
> +      let c = mvnoData.charAt(i);
> +      if ((c == 'x') || (c == 'X') || (c == imsi.charAt(i))) {

c == imsi[i];
Attachment #8365800 - Flags: review?(htsai) → review+
Comment on attachment 8365801 [details] [diff] [review]
Part 4: test cases.

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

r=me with comment addressed, thank you!

::: dom/icc/tests/marionette/test_icc_mvno_matches.js
@@ +16,5 @@
> +  let testCases = [
> +    // mvno type, mvno data, expected result
> +    ["imsi", "3102600",            true ],
> +    ["imsi", "31026xx0",           true ], // x means skip the comparison.
> +    ["imsi", "310260x0x",          true ],

Please add one more case that mvnoData is exactly the same as IMSI.
Attachment #8365801 - Flags: review?(htsai) → review+
Comment on attachment 8365797 [details] [diff] [review]
Part 1: interface changes.

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

After looked all patch again and discussed with Hsinyi, it seems we need to provide more information to API user.
Currently we only support IMSI, returning a boolean seems enough. However, consider we may support 'spn' matching in the future, boolean is not clear enough.
For example, if sim card doesn't have SPN information insdie, |mvnoMatches('spn', 'foo')| returns only false, API user cannot tell this false is caused by matching fail or actually the SPN is not even existed.
In such case, we could consider use a DOMRequest and provide more information by dispatching error event.
Thank you.
(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> Comment on attachment 8365797 [details] [diff] [review]
> Part 1: interface changes.
> 
> Review of attachment 8365797 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> After looked all patch again and discussed with Hsinyi, it seems we need to
> provide more information to API user.
> Currently we only support IMSI, returning a boolean seems enough. However,
> consider we may support 'spn' matching in the future, boolean is not clear
> enough.
> For example, if sim card doesn't have SPN information insdie,
> |mvnoMatches('spn', 'foo')| returns only false, API user cannot tell this
> false is caused by matching fail or actually the SPN is not even existed.
> In such case, we could consider use a DOMRequest and provide more
> information by dispatching error event.
> Thank you.

Thanks for the summary, Edgar!

Just talked to jaoo on IRC, he also thought returning a DOMRequest helps gaia. So, let us do this as Edger suggested.
Thanks, Edgar and Hsinyi.
I will provide the new patches using DOMRequest. Regarding to the enum for mvno type, as discussed, we will note as a TODO and maybe fix it after we change to use IPDL, which will be more meaningful.
Attached patch Part 1: interface changes, v2. (obsolete) — Splinter Review
Attachment #8365797 - Attachment is obsolete: true
Attached patch Part 2: dom changes, v2. (obsolete) — Splinter Review
Attachment #8365799 - Attachment is obsolete: true
Attachment #8365799 - Flags: feedback?(echen)
Attached patch Part 3: ril implementation, v2. (obsolete) — Splinter Review
Attachment #8365800 - Attachment is obsolete: true
Attachment #8365800 - Flags: feedback?(echen)
Attached patch Part 4: test cases, v2. (obsolete) — Splinter Review
Attachment #8365801 - Attachment is obsolete: true
Attachment #8365801 - Flags: feedback?(echen)
Attached patch Part 1: interface changes, v3. (obsolete) — Splinter Review
Refine comments.
Attachment #8365895 - Attachment is obsolete: true
Attached patch Part 4: test cases, v3. (obsolete) — Splinter Review
Attachment #8365898 - Attachment is obsolete: true
Attachment #8366366 - Flags: review?(htsai)
Attachment #8366366 - Flags: feedback?(echen)
Attachment #8365896 - Flags: feedback?(htsai)
Attachment #8365896 - Flags: feedback?(echen)
Attachment #8365897 - Flags: review?(htsai)
Attachment #8365897 - Flags: feedback?(echen)
Attachment #8366367 - Flags: review?(htsai)
Attachment #8366367 - Flags: feedback?(echen)
Comment on attachment 8365897 [details] [diff] [review]
Part 3: ril implementation, v2.

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

Please see my below comments, thank you.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1646,5 @@
> +  // Matches the mvnoData pattern with imsi. Characters 'x' and 'X' are skipped
> +  // and not compared. E.g., if the mvnoData passed is '310260x10xxxxxx',
> +  // then the function returns true only if imsi has the same first 6 digits,
> +  // 8th and 9th digit.
> +  imsiMatches: function(target, message) {

I prefer to make this function only returns true/false and set |message.errorMsg| if any error occurred. Then handle sending async message part in mvnoMatches. In this way, we can avoid duplicated code. :)

@@ +1653,5 @@
> +      message.errorMsg = RIL.GECKO_ERROR_GENERIC_FAILURE;
> +      target.sendAsyncMessage("RIL:MvnoMatches", {
> +        clientId: this.clientId,
> +        data: message
> +      });

ditto.

@@ +1663,5 @@
> +      message.result = false;
> +      target.sendAsyncMessage("RIL:MvnoMatches", {
> +        clientId: this.clientId,
> +        data: message
> +      });

ditto.

@@ +1674,5 @@
> +      if ((c == 'x') || (c == 'X') || (c == imsi[i])) {
> +        continue;
> +      } else {
> +        result = false;
> +      }

bail-out easier,

if ((c !== 'x') && (c !== 'X') && (c !== imsi[i])) {
  result = false;
  break;
}

@@ +1681,5 @@
> +    message.result = result;
> +    target.sendAsyncMessage("RIL:MvnoMatches", {
> +      clientId: this.clientId,
> +      data: message
> +    });

Handle sending async message part in mvnoMatches
Attachment #8365897 - Flags: feedback?(echen)
Attachment #8365896 - Flags: feedback?(echen) → feedback+
(In reply to Edgar Chen [:edgar][:echen] from comment #19)
> Comment on attachment 8365897 [details] [diff] [review]
> Part 3: ril implementation, v2.
> 
> Review of attachment 8365897 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please see my below comments, thank you.
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +1646,5 @@
> > +  // Matches the mvnoData pattern with imsi. Characters 'x' and 'X' are skipped
> > +  // and not compared. E.g., if the mvnoData passed is '310260x10xxxxxx',
> > +  // then the function returns true only if imsi has the same first 6 digits,
> > +  // 8th and 9th digit.
> > +  imsiMatches: function(target, message) {
> 
> I prefer to make this function only returns true/false and set
> |message.errorMsg| if any error occurred. Then handle sending async message
> part in mvnoMatches. In this way, we can avoid duplicated code. :)

Thanks for the feedback.
I struggled for this too. It seems better for 'imsiMatches()' to be a helper function that returns true/false only, but then we would have to check imsi in 'mvnoMatches()', since we should return error in this case. If it's okay this way, I will change it.

> 
> @@ +1653,5 @@
> > +      message.errorMsg = RIL.GECKO_ERROR_GENERIC_FAILURE;
> > +      target.sendAsyncMessage("RIL:MvnoMatches", {
> > +        clientId: this.clientId,
> > +        data: message
> > +      });
> 
> ditto.
> 
> @@ +1663,5 @@
> > +      message.result = false;
> > +      target.sendAsyncMessage("RIL:MvnoMatches", {
> > +        clientId: this.clientId,
> > +        data: message
> > +      });
> 
> ditto.
> 
> @@ +1674,5 @@
> > +      if ((c == 'x') || (c == 'X') || (c == imsi[i])) {
> > +        continue;
> > +      } else {
> > +        result = false;
> > +      }
> 
> bail-out easier,
> 
> if ((c !== 'x') && (c !== 'X') && (c !== imsi[i])) {
>   result = false;
>   break;
> }
> 
> @@ +1681,5 @@
> > +    message.result = result;
> > +    target.sendAsyncMessage("RIL:MvnoMatches", {
> > +      clientId: this.clientId,
> > +      data: message
> > +    });
> 
> Handle sending async message part in mvnoMatches
(In reply to Jessica Jong [:jjong] [:jessica] from comment #20)
> (In reply to Edgar Chen [:edgar][:echen] from comment #19)
> > Comment on attachment 8365897 [details] [diff] [review]
> > Part 3: ril implementation, v2.
> > 
> > Review of attachment 8365897 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Please see my below comments, thank you.
> > 
> > ::: dom/system/gonk/RadioInterfaceLayer.js
> > @@ +1646,5 @@
> > > +  // Matches the mvnoData pattern with imsi. Characters 'x' and 'X' are skipped
> > > +  // and not compared. E.g., if the mvnoData passed is '310260x10xxxxxx',
> > > +  // then the function returns true only if imsi has the same first 6 digits,
> > > +  // 8th and 9th digit.
> > > +  imsiMatches: function(target, message) {
> > 
> > I prefer to make this function only returns true/false and set
> > |message.errorMsg| if any error occurred. Then handle sending async message
> > part in mvnoMatches. In this way, we can avoid duplicated code. :)
> 
> Thanks for the feedback.
> I struggled for this too. It seems better for 'imsiMatches()' to be a helper
> function that returns true/false only, but then we would have to check imsi
> in 'mvnoMatches()', since we should return error in this case. If it's okay
> this way, I will change it.

I am okay with either way:
1). For the error case, it depends on |errorMsg| is set or not. So you can still check imsi in 'imsiMatches()' and set |message.errorMsg| for error case. (Of course, in this case you need pass |message| to 'imsiMatches()')
2). Or you can move the checking to 'mvnoMatches()'.

Thank you.
Comment on attachment 8366367 [details] [diff] [review]
Part 4: test cases, v3.

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

f=me with below comment addressed, thank you.

::: dom/icc/tests/marionette/test_icc_mvno_matches.js
@@ +8,5 @@
> +let testCases = [
> +  // mvno type, mvno data, request success, expected result
> +  ["imsi", "3102600",            true, true               ],
> +  // x means skip the comparison.
> +  ["imsi", "31026xx0",           true, true               ],

Please add some tests for 'X'.
Attachment #8366367 - Flags: feedback?(echen) → feedback+
Attachment #8366366 - Flags: feedback?(echen) → feedback+
(In reply to Edgar Chen [:edgar][:echen] from comment #21)
> (In reply to Jessica Jong [:jjong] [:jessica] from comment #20)
> > (In reply to Edgar Chen [:edgar][:echen] from comment #19)
> > > Comment on attachment 8365897 [details] [diff] [review]
> > > Part 3: ril implementation, v2.
> > > 
> > > Review of attachment 8365897 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > Please see my below comments, thank you.
> > > 
> > > ::: dom/system/gonk/RadioInterfaceLayer.js
> > > @@ +1646,5 @@
> > > > +  // Matches the mvnoData pattern with imsi. Characters 'x' and 'X' are skipped
> > > > +  // and not compared. E.g., if the mvnoData passed is '310260x10xxxxxx',
> > > > +  // then the function returns true only if imsi has the same first 6 digits,
> > > > +  // 8th and 9th digit.
> > > > +  imsiMatches: function(target, message) {
> > > 
> > > I prefer to make this function only returns true/false and set
> > > |message.errorMsg| if any error occurred. Then handle sending async message
> > > part in mvnoMatches. In this way, we can avoid duplicated code. :)
> > 
> > Thanks for the feedback.
> > I struggled for this too. It seems better for 'imsiMatches()' to be a helper
> > function that returns true/false only, but then we would have to check imsi
> > in 'mvnoMatches()', since we should return error in this case. If it's okay
> > this way, I will change it.
> 
> I am okay with either way:
> 1). For the error case, it depends on |errorMsg| is set or not. So you can
> still check imsi in 'imsiMatches()' and set |message.errorMsg| for error
> case. (Of course, in this case you need pass |message| to 'imsiMatches()')
> 2). Or you can move the checking to 'mvnoMatches()'.
> 
> Thank you.

I will use the 2) to make 'imsiMatches()' function's objective clearer. Thank you.
Attached patch Part 3: ril implementation, v3. (obsolete) — Splinter Review
Address review comment 19.
Please help check again. Thank you!
Attachment #8365897 - Attachment is obsolete: true
Attachment #8365897 - Flags: review?(htsai)
Attachment #8366491 - Flags: review?(htsai)
Attachment #8366491 - Flags: feedback?(echen)
Attachment #8366366 - Flags: review?(htsai) → review+
Comment on attachment 8366366 [details] [diff] [review]
Part 1: interface changes, v3.

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

::: dom/webidl/MozIcc.webidl
@@ +385,5 @@
> +   * TODO: change param mvnoType to WebIDL enum after Bug 864489 -
> +   *       B2G RIL: use ipdl as IPC in MozIccManager
> +   */
> +  [Throws]
> +  nsISupports mvnoMatches(DOMString mvnoType, DOMString matchData);

mvnoMatches doesn't sound that proper to me as it no longer returns a boolean. I'd like to change the name to matchMvno(). Please modify it in your final patch.

r=me with this addressed.
Comment on attachment 8366491 [details] [diff] [review]
Part 3: ril implementation, v3.

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1690,5 @@
> +    message.result = this.imsiMatches(message.mvnoData);
> +    target.sendAsyncMessage("RIL:MvnoMatches", {
> +      clientId: this.clientId,
> +      data: message
> +    });

How about,
------
if (!message.errorMsg) {
  message.result = this.imsiMatches(message.mvnoData);
}

target.sendAsyncMessage("RIL:MvnoMatches", {
 ....
});
------
Attachment #8366491 - Flags: feedback?(echen) → feedback+
Comment on attachment 8365896 [details] [diff] [review]
Part 2: dom changes, v2.

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

It looks good but don't forget to change the function name.
Attachment #8365896 - Flags: feedback?(htsai) → feedback+
(In reply to Edgar Chen [:edgar][:echen] from comment #26)
> Comment on attachment 8366491 [details] [diff] [review]
> Part 3: ril implementation, v3.
> 
> Review of attachment 8366491 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +1690,5 @@
> > +    message.result = this.imsiMatches(message.mvnoData);
> > +    target.sendAsyncMessage("RIL:MvnoMatches", {
> > +      clientId: this.clientId,
> > +      data: message
> > +    });
> 
> How about,
> ------
> if (!message.errorMsg) {
>   message.result = this.imsiMatches(message.mvnoData);
> }
> 
> target.sendAsyncMessage("RIL:MvnoMatches", {
>  ....
> });
> ------

Uff! should have thought of that.. Thank you, I will provide a new patch.
Comment on attachment 8366491 [details] [diff] [review]
Part 3: ril implementation, v3.

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

It looks good but I'd like to take a look again after function renaming. Thanks.

::: dom/system/gonk/RILContentHelper.js
@@ +636,5 @@
>      let context = this.getRilContext(clientId);
>      return context && context.cardState;
>    },
>  
> +  mvnoMatches: function(clientId, window, mvnoType, mvnoData) {

rename it matchMvno()

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1399,5 @@
>        case "RIL:UpdateIccContact":
>          this.workerMessenger.sendWithIPCMessage(msg, "updateICCContact");
>          break;
> +      case "RIL:MvnoMatches":
> +        this.mvnoMatches(msg.target, msg.json.data);

rename it matchMvno()

@@ +1646,5 @@
> +  // Matches the mvnoData pattern with imsi. Characters 'x' and 'X' are skipped
> +  // and not compared. E.g., if the mvnoData passed is '310260x10xxxxxx',
> +  // then the function returns true only if imsi has the same first 6 digits,
> +  // 8th and 9th digit.
> +  imsiMatches: function(mvnoData) {

rename it isImsiMatches()
Attachment #8366491 - Flags: review?(htsai)
Comment on attachment 8366367 [details] [diff] [review]
Part 4: test cases, v3.

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

Looks good. Thanks and sorry that you'd need to rebase on the next version due to function renaming.
Attachment #8366367 - Flags: review?(htsai) → review+
Attached patch Part 1: interface changes, v4. (obsolete) — Splinter Review
Address review comment 25: use 'matchMvno()' instead of 'mvnoMatches()'.
Attachment #8366366 - Attachment is obsolete: true
Attachment #8366547 - Flags: review+
Attachment #8366547 - Flags: feedback+
Attached patch Part 2: dom changes, v3. (obsolete) — Splinter Review
Address review comment 27: use 'matchMvno()' instead of 'mvnoMatches()'.
Attachment #8365896 - Attachment is obsolete: true
Attachment #8366550 - Flags: feedback+
Address review comment 26 and comment 29:
- avoid duplicate code.
- use 'matchMvno()' instead of 'mvnoMatches()'.
Attachment #8366491 - Attachment is obsolete: true
Attachment #8366553 - Flags: feedback+
Attached patch Part 4: test cases, v4. (obsolete) — Splinter Review
Address review comment 24:
- add tests for 'X'.
- use 'matchMvno()' instead of 'mvnoMatches()', including file name.
Attachment #8366367 - Attachment is obsolete: true
Attachment #8366554 - Flags: review+
Attachment #8366554 - Flags: feedback+
Attached patch Part 4: test cases, v5. (obsolete) — Splinter Review
Fixed some comments.
Attachment #8366554 - Attachment is obsolete: true
Attachment #8366556 - Flags: review+
Attachment #8366556 - Flags: feedback+
Attachment #8366553 - Flags: review?(htsai)
Removed redundant semicolon.
Attachment #8366556 - Attachment is obsolete: true
Attachment #8366559 - Flags: review+
Attachment #8366559 - Flags: feedback+
Comment on attachment 8366553 [details] [diff] [review]
Part 3: ril implementation, v4.

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

Thank you!
Attachment #8366553 - Flags: review?(htsai) → review+
Comment on attachment 8366550 [details] [diff] [review]
Part 2: dom changes, v3.

Olli,

We are adding a new API in MozIcc to support selection of APN relying on the SIM's IMSI. Please find more details at bug 947855 comment 7 and 8. 

Thank you.
Attachment #8366550 - Flags: review?(bugs)
Summary: B2G RIL: provide mvnoMatches() API for automatic selection of APN relying on mvnoType and matchPattern → B2G RIL: provide matchMvno() API for automatic selection of APN relying on mvnoType and matchData
Comment on attachment 8366550 [details] [diff] [review]
Part 2: dom changes, v3.


>+
>+already_AddRefed<nsISupports>
>+Icc::MatchMvno(const nsAString& mvnoType,
aMvnoType

>+               const nsAString& mvnoData,
aMvnoData
Attachment #8366550 - Flags: review?(bugs) → review+
Address review comment 40 and add r=smaug.
Attachment #8366550 - Attachment is obsolete: true
Attachment #8367093 - Flags: review+
Whiteboard: [FT:RIL]
Comment on attachment 8366547 [details] [diff] [review]
Part 1: interface changes, v4.

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

::: dom/webidl/MozIcc.webidl
@@ +378,5 @@
> +   * @param matchData
> +   *        Data to be compared with ICC's field.
> +   *
> +   * @return true if matchData matches the ICC's field indicated by mvnoType,
> +   *         false otherwise.

Since the return type is going to be a DOMRequest this comment should be updated, shouldn't be?
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #43)
> Comment on attachment 8366547 [details] [diff] [review]
> Part 1: interface changes, v4.
> 
> Review of attachment 8366547 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/webidl/MozIcc.webidl
> @@ +378,5 @@
> > +   * @param matchData
> > +   *        Data to be compared with ICC's field.
> > +   *
> > +   * @return true if matchData matches the ICC's field indicated by mvnoType,
> > +   *         false otherwise.
> 
> Since the return type is going to be a DOMRequest this comment should be
> updated, shouldn't be?

Oh yes, the comments in the v3 patch was lost somehow. Thank you.
Restore comments in v3.
Attachment #8366547 - Attachment is obsolete: true
Attachment #8367317 - Flags: review+
Attachment #8367317 - Flags: feedback+
(In reply to Jessica Jong (OOO Jan. 30 ~ Feb. 4) [:jjong] [:jessica] from comment #42)
> Re-run full try: https://tbpl.mozilla.org/?tree=Try&rev=2f26146b40d9

I think we are good to go! The failure (Mochitest 5) in B2G ICS Emulator Debug is already tracked in b2g-inbound.
Keywords: checkin-needed
I will help to push into b2g-inbound.
Keywords: checkin-needed
Beatriz,

This change requires a RIl interface change. Our CS with QC is underway and very close to finish.

I understand this is a blocker for TEF but please help understand if this can be taken in 1.4.
Flags: needinfo?(brg)
Preeti,
I know this is an important change, coming from https://bugzilla.mozilla.org/show_bug.cgi?id=947855 requirement. We need this feature supported by 1.3 to be ready to launch in a new country. We want to avoid having to wait for 1.4 to add the new country.
Let me copy people from the partner RIL so they can include this support if they were not doing it yet!
Flags: needinfo?(brg)
(In reply to Beatriz Rodríguez [:brg] from comment #50)
> Preeti,
> I know this is an important change, coming from
> https://bugzilla.mozilla.org/show_bug.cgi?id=947855 requirement. We need
> this feature supported by 1.3 to be ready to launch in a new country. We
> want to avoid having to wait for 1.4 to add the new country.
> Let me copy people from the partner RIL so they can include this support if
> they were not doing it yet!

Okay - blocker for a target country then, so blocking+
blocking-b2g: 1.3? → 1.3+
You need to log in before you can comment on or make changes to this bug.