Closed Bug 1113476 Opened 7 years ago Closed 7 years ago

B2G RIL: support nsck/pck for SIM Lock types.

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0M+, b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 affected, b2g-v2.1S affected, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S3 (9jan)
blocking-b2g 2.0M+
Tracking Status
b2g-v2.0 --- affected
b2g-v2.0M --- affected
b2g-v2.1 --- affected
b2g-v2.1S --- affected
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: sku, Assigned: sku)

References

Details

Attachments

(3 files, 8 obsolete files)

3.31 KB, patch
sku
: review+
Details | Diff | Splinter Review
5.52 KB, patch
sku
: review+
Details | Diff | Splinter Review
11.28 KB, patch
sku
: review+
Details | Diff | Splinter Review
Per 3GPP 22.022, there should be five types of key that SIM Lock should support.

NCK - Network Control Key
CCK - Corporate Control Key
SPCK - Service Provider Control Key
NSCK - Network Subset Control Key
PCK - Personalisation Control Key

Currently, gecko only support NCK/CCK/SPCK. we still need to make NCSK and PCK up to complete the features.



[1] nck - https://github.com/mozilla/gecko-dev/blob/master/dom/webidl/MozIcc.webidl#L59
[2] cck - https://github.com/mozilla/gecko-dev/blob/master/dom/webidl/MozIcc.webidl#L63
[3] spck - https://github.com/mozilla/gecko-dev/blob/master/dom/webidl/MozIcc.webidl#L64
Assignee: nobody → sku
Summary: B2G RIL: support nsck/pck from SIM Lock type. → B2G RIL: support nsck/pck for SIM Lock types.
blocking-b2g: --- → 2.0M?
Attached patch 2.0m Patch... (obsolete) — Splinter Review
Will provide m-c patch for review first, and up lift to 2.0m then.
Blocks: 1116072
See Also: → 1116072
Attachment #8542064 - Flags: review?(echen)
Attachment #8542066 - Flags: review?(echen)
Attachment #8542067 - Flags: review?(htsai)
Attachment #8542067 - Flags: review?(echen)
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Comment on attachment 8542067 [details] [diff] [review]
Bug 1113476 : webIDL patch - B2G RIL: support nsck/pck for SIM Lock types.

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

Thank you.
Attachment #8542067 - Flags: review?(htsai) → review+
Comment on attachment 8542067 [details] [diff] [review]
Bug 1113476 : webIDL patch - B2G RIL: support nsck/pck for SIM Lock types.

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

Looks good to me. Thank you.
Attachment #8542067 - Flags: review?(echen) → review+
Comment on attachment 8542066 [details] [diff] [review]
Bug 1113476 : DOM patch - B2G RIL: support nsck/pck for SIM Lock types.

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

::: dom/icc/interfaces/nsIIccProvider.idl
@@ +24,1 @@
>  interface nsIIccProvider : nsISupports

This interface doesn't belong to DOM, it more like a RIL internal interface.
Please help to revise the commit message or merge this patch with other part.
Thank you.

@@ +62,5 @@
>    const unsigned long CARD_LOCK_TYPE_PIN2 = 1;
>    const unsigned long CARD_LOCK_TYPE_PUK = 2;
>    const unsigned long CARD_LOCK_TYPE_PUK2 = 3;
>    const unsigned long CARD_LOCK_TYPE_NCK = 4;
> +  const unsigned long CARD_LOCK_TYPE_NSCK = 5;

Please also help to add corresponding assertion in http://dxr.mozilla.org/mozilla-central/source/dom/icc/Assertions.cpp. Thank you.
Attachment #8542066 - Flags: review?(echen)
Comment on attachment 8542064 [details] [diff] [review]
Bug 1113476 : RIL patch - B2G RIL: support nsck/pck for SIM Lock types.

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

Please see my comments below. Thank you.

::: dom/system/gonk/ril_consts.js
@@ +2688,5 @@
>  PERSONSUBSTATE[CARD_PERSOSUBSTATE_RUIM_RUIM_PUK] = GECKO_CARDSTATE_RUIM_PUK_REQUIRED;
>  
>  this.GECKO_PERSO_LOCK_TO_CARD_PERSO_LOCK = {};
>  GECKO_PERSO_LOCK_TO_CARD_PERSO_LOCK[GECKO_CARDLOCK_NCK] = CARD_PERSOSUBSTATE_SIM_NETWORK;
> +GECKO_PERSO_LOCK_TO_CARD_PERSO_LOCK[GECKO_CARDLOCK_NSCK] = CARD_PERSOSUBSTATE_SIM_NETWORK_SUBSET;

It seems we don't need `GECKO_PERSO_LOCK_TO_CARD_PERSO_LOCK` anymore, please help to remove them all.

::: dom/system/gonk/ril_worker.js
@@ +636,3 @@
>        case GECKO_CARDLOCK_RCCK_PUK: // Fall through.
>        case GECKO_CARDLOCK_RSPCK_PUK:
>          options.personlization =

We don't need this anymore, please help to remove it.

@@ +693,5 @@
>     */
>    enterDepersonalization: function(options) {
>      let Buf = this.context.Buf;
>      Buf.newParcel(REQUEST_ENTER_NETWORK_DEPERSONALIZATION_CODE, options);
> +    Buf.writeInt32(1);

Nice catch!!

And the http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/tests/test_ril_worker_icc_CardLock.js needs revision for this fix.

@@ +871,3 @@
>        case GECKO_CARDLOCK_CCK: // Fall through.
>        case GECKO_CARDLOCK_SPCK:
> +      // TODO: Bug 1116072, identify the mapping between RIL_PERSOSUBSTATE_SIM_SIM

nit: indention
Attachment #8542064 - Flags: review?(echen)
Attachment #8543790 - Flags: review?(echen)
Attachment #8543792 - Flags: review?(echen)
Comment on attachment 8543792 [details] [diff] [review]
Bug 1113476 : IDL patch - B2G RIL: support nsck/pck for SIM Lock types. v2.

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

::: dom/icc/Assertions.cpp
@@ +62,5 @@
>  ASSERT_ICC_LOCK_TYPE_EQUALITY(Pin2, CARD_LOCK_TYPE_PIN2);
>  ASSERT_ICC_LOCK_TYPE_EQUALITY(Puk, CARD_LOCK_TYPE_PUK);
>  ASSERT_ICC_LOCK_TYPE_EQUALITY(Puk2, CARD_LOCK_TYPE_PUK2);
>  ASSERT_ICC_LOCK_TYPE_EQUALITY(Nck, CARD_LOCK_TYPE_NCK);
> +ASSERT_ICC_LOCK_TYPE_EQUALITY(Nsck, CARD_LOCK_TYPE_NSCK);

Please also help to add assertion for NSCK_PUK and PCK_PUK. Thank you.
Attachment #8543792 - Flags: review?(echen)
Comment on attachment 8543790 [details] [diff] [review]
Bug 1113476 : RIL patch - B2G RIL: support nsck/pck for SIM Lock types. v2.

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

This patch looks good except the test case part. Please see my comments below.

::: dom/system/gonk/tests/test_ril_worker_icc_CardLock.js
@@ -278,5 @@
>        // Token : we don't care
>        this.readInt32();
>  
>        // Data
> -      do_check_eq(this.readInt32(), GECKO_PERSO_LOCK_TO_CARD_PERSO_LOCK[aLock]);

The first byte is the length of string array, we still need to read it (in this case, it should be `1`). Or you can use readStringList(), like http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/tests/test_ril_worker_icc_CardLock.js#33-36.

@@ -283,5 @@
>        do_check_eq(this.readString(), aPassword);
>      };
>  
>      ril.iccUnlockCardLock({
> -      lockType: aLock,

|iccUnlockCardLock| will check the |lockType|, so we still need it, but you can just hardcode a valid one.
Attachment #8543790 - Flags: review?(echen)
Ooops, thanks Edgar's review. will address these in next patch.
Attachment #8544640 - Flags: review?(echen)
Attachment #8544642 - Flags: review?(echen)
try seems fail on other part, not related to my patch.
(https://tbpl.mozilla.org/?tree=Try&rev=f33e6ce47917)
Comment on attachment 8544642 [details] [diff] [review]
Bug 1113476 : IDL patch - B2G RIL: support nsck/pck for SIM Lock types. v3.

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

Thank you.
Attachment #8544642 - Flags: review?(echen) → review+
Comment on attachment 8544640 [details] [diff] [review]
Bug 1113476 : RIL patch - B2G RIL: support nsck/pck for SIM Lock types. v3.

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

Perfect! Thank you.
Attachment #8544640 - Flags: review?(echen) → review+
Attachment #8538985 - Attachment is obsolete: true
Attachment #8544640 - Attachment is obsolete: true
Attachment #8545645 - Flags: review+
try is passed.

try seems fail on other part, not related to my patch.
(https://tbpl.mozilla.org/?tree=Try&rev=f33e6ce47917)
Keywords: checkin-needed
I will help to merge. :)
Keywords: checkin-needed
(In reply to Edgar Chen [:edgar][:echen] from comment #25)
> I modified the patches a bit for landing
> - fixing the conflict in idl patch
> - modifying the commit message, e.g. remove patch version
> 
> https://hg.mozilla.org/integration/b2g-inbound/rev/26cd48766d6e
> https://hg.mozilla.org/integration/b2g-inbound/rev/20c767e2b07f
> https://hg.mozilla.org/integration/b2g-inbound/rev/48e799156c69

Thanks, Edgar.
Blocks: Woodduck
blocking-b2g: 2.0M? → 2.0M+
HI li Tao,
Since you already pick the patch and fix in your side. Do you still need our patch in 2.0M?
You need to log in before you can comment on or make changes to this bug.