905503 - B2G RIL: Add a new card state for personalization ready

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Edgar Chen [:edgar]]

Currently we change sim card state to "ready" when personalization substate becomes to CARD_PERSOSUBSTATE_READY. This may cause some timing problems as bug 869778 comment #13.

Comment 1

[shawn ku [:sku]]

(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> Currently we change sim card state to "ready" when personalization substate
> becomes to CARD_PERSOSUBSTATE_READY. This may cause some timing problems as
> bug 869778 comment #13.
Hi Edgar,
personsubstate is a temporary state. we should expect APP_STATE to ready eventually no matter how long shuld it take.

sku.

Comment 2

[Edgar Chen [:edgar]]

Attachment: Add a new card state and xpcshell tests for personalization ready, v1

Comment 3

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

Comment on attachment 790627 [details] [diff] [review]
Add a new card state and xpcshell tests for personalization ready, v1

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

You need to update the comments in nsIDOMIccManager.idl as well.

Comment 4

[Edgar Chen [:edgar]]

Attachment: Add a new card state and xpcshell tests for personalization ready, v2

Update the comments in nsIDOMIccManager.idl

Comment 5

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

Comment on attachment 790706 [details] [diff] [review]
Add a new card state and xpcshell tests for personalization ready, v2

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

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +9,5 @@
>  interface nsIDOMDOMRequest;
>  interface nsIDOMEventListener;
>  interface nsIDOMMozIccInfo;
>  
> +[scriptable, builtinclass, uuid(572afe3d-561a-46b4-b87d-9a46781c5da7)]

Updating comments doesn't need to update uuid.

@@ +296,5 @@
>     *
>     * Possible values: null, 'unknown', 'absent', 'pinRequired', 'pukRequired',
> +   * 'personalizationInProgress', 'personalizationReady' 'networkLocked',
> +   * 'corporateLocked', 'serviceProviderLocked', 'networkPukRequired',
> +   * 'corporatePukRequired', 'serviceProviderPukRequired', 'ready'.

I think we could put 'personalizationReady' after 'serviceProviderPukRequired' to help people understand the state transition in cardState.

::: dom/system/gonk/ril_consts.js
@@ +2339,5 @@
>  this.GECKO_CARDSTATE_ABSENT                        = "absent";
>  this.GECKO_CARDSTATE_PIN_REQUIRED                  = "pinRequired";
>  this.GECKO_CARDSTATE_PUK_REQUIRED                  = "pukRequired";
>  this.GECKO_CARDSTATE_PERSONALIZATION_IN_PROGRESS   = "personalizationInProgress";
> +this.GECKO_CARDSTATE_PERSONALIZATION_READY         = "personalizationReady";

in ril_consts, it's okay for where you want to put it,
but I'd suggest you put in the same order in PERSONSUBSTATE,
so either you update here or update the code below.

Comment 6

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #5)
> Comment on attachment 790706 [details] [diff] [review]
> Add a new card state and xpcshell tests for personalization ready, v2
> 
> Review of attachment 790706 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/icc/interfaces/nsIDOMIccManager.idl
> @@ +9,5 @@
> >  interface nsIDOMDOMRequest;
> >  interface nsIDOMEventListener;
> >  interface nsIDOMMozIccInfo;
> >  
> > +[scriptable, builtinclass, uuid(572afe3d-561a-46b4-b87d-9a46781c5da7)]
> 
> Updating comments doesn't need to update uuid.
> 
> @@ +296,5 @@
> >     *
> >     * Possible values: null, 'unknown', 'absent', 'pinRequired', 'pukRequired',
> > +   * 'personalizationInProgress', 'personalizationReady' 'networkLocked',
> > +   * 'corporateLocked', 'serviceProviderLocked', 'networkPukRequired',
> > +   * 'corporatePukRequired', 'serviceProviderPukRequired', 'ready'.
> 
> I think we could put 'personalizationReady' after
> 'serviceProviderPukRequired' to help people understand the state transition
> in cardState.

Okay, I will put it after 'serviceProviderPukRequired'.

> 
> ::: dom/system/gonk/ril_consts.js
> @@ +2339,5 @@
> >  this.GECKO_CARDSTATE_ABSENT                        = "absent";
> >  this.GECKO_CARDSTATE_PIN_REQUIRED                  = "pinRequired";
> >  this.GECKO_CARDSTATE_PUK_REQUIRED                  = "pukRequired";
> >  this.GECKO_CARDSTATE_PERSONALIZATION_IN_PROGRESS   = "personalizationInProgress";
> > +this.GECKO_CARDSTATE_PERSONALIZATION_READY         = "personalizationReady";
> 
> in ril_consts, it's okay for where you want to put it,
> but I'd suggest you put in the same order in PERSONSUBSTATE,
> so either you update here or update the code below.

Okay, I will update the order of PERSONSUBSTATE to have the same order in ril.h.

Thanks

Comment 7

[Edgar Chen [:edgar]]

Attachment: Add a new card state and xpcshell tests for personalization ready, v3, r=allstars.chh

Address review comment #5.

Comment 8

[Edgar Chen [:edgar]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=fcd5069f9691

Comment 9

[Edgar Chen [:edgar]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1aee5bb06429

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1aee5bb06429