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

Will provide m-c patch for review first, and up lift to 2.0m then.

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.

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.

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.

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

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.

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.

Ooops, thanks Edgar's review. will address these in next patch.

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.

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.

try is passed. try seems fail on other part, not related to my patch. (https://tbpl.mozilla.org/?tree=Try&rev=f33e6ce47917)

I will help to merge. :)

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.

https://hg.mozilla.org/mozilla-central/rev/26cd48766d6e https://hg.mozilla.org/mozilla-central/rev/20c767e2b07f https://hg.mozilla.org/mozilla-central/rev/48e799156c69

HI li Tao, Since you already pick the patch and fix in your side. Do you still need our patch in 2.0M?