Closed Bug 1113476 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

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.
Blocks: 1111411
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: 1115448
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
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
(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, 1080481
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?
Depends on: 1157018
You need to log in before you can comment on or make changes to this bug.