B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

RESOLVED FIXED in mozilla28

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sku, Assigned: sku)

Tracking

unspecified
mozilla28
ARM
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 9 obsolete attachments)

1.22 KB, patch
Details | Diff | Splinter Review
2.34 KB, patch
Details | Diff | Splinter Review
1.98 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
Per current design, when there is no more PUK retry, ril_worker.js will map cardState to pukRequired due to "app_state":3. This is in-correct. We should refer to universalPINState/pin1_replaced/pin1 to determine if permanent blocked.


10-24 02:50:01.679   739   756 I Gecko   : RIL Worker[0]: iccStatus: {"cardState":1,"universalPINState":0,"gsmUmtsSubscriptionAppIndex":0,"cdmaSubscriptionAppIndex":-1,"imsSubscriptionAppIndex":-1,"apps":[{"app_type":2,"app_state":3,"perso_substate":0,"aid":"a0000000871002f886ff9289050b00ff","app_label":"434854","pin1_replaced":0,"pin1":5,"pin2":1}]}
(Assignee)

Updated

4 years ago
Summary: B2G RIL: correct cardState to permanentBlocked when no more PUK retry. → B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
(Assignee)

Comment 1

4 years ago
Created attachment 821500 [details] [diff] [review]
Bug 930394 - Part 1: RIL patch. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
(Assignee)

Comment 2

4 years ago
Created attachment 821501 [details] [diff] [review]
Bug 930394 - Part 2: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
(Assignee)

Comment 3

4 years ago
Created attachment 821502 [details] [diff] [review]
Bug 930394 - Part 3: Add test cases. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
(Assignee)

Updated

4 years ago
Attachment #821500 - Flags: review?(allstars.chh)
(Assignee)

Updated

4 years ago
Attachment #821501 - Flags: review?(allstars.chh)
(Assignee)

Updated

4 years ago
Attachment #821502 - Flags: review?(allstars.chh)
(Assignee)

Comment 4

4 years ago
Add related bug link - 
Bug 921815 - [B2G][Helix][SIM][wangchao]No limit for the times of enter sim puk when no puk time left.
Comment on attachment 821501 [details] [diff] [review]
Bug 930394 - Part 2: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

I am okay with this patch.
However IDL change should be in Part 1.
Please reorder you patches.
Attachment #821501 - Flags: review?(allstars.chh)
(Assignee)

Comment 6

4 years ago
Created attachment 823119 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
Attachment #821501 - Attachment is obsolete: true
(Assignee)

Comment 7

4 years ago
Created attachment 823120 [details] [diff] [review]
Bug 930394 - Part 2: RIL patch. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
Attachment #821500 - Attachment is obsolete: true
Attachment #821500 - Flags: review?(allstars.chh)
(Assignee)

Comment 8

4 years ago
Created attachment 823121 [details] [diff] [review]
Bug 930394 - Part 3: Add test cases. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.
Attachment #821502 - Attachment is obsolete: true
Attachment #821502 - Flags: review?(allstars.chh)
(Assignee)

Comment 9

4 years ago
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #5)
> Comment on attachment 821501 [details] [diff] [review]
> Bug 930394 - Part 2: add comment in IDL. B2G RIL: Correct cardState to
> permanentBlocked when no more PUK retry.
> 
> Review of attachment 821501 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I am okay with this patch.
> However IDL change should be in Part 1.
> Please reorder you patches.

Thanks Yoshi for reminding me this part.
(Assignee)

Updated

4 years ago
Attachment #823119 - Flags: review?(allstars.chh)
(Assignee)

Updated

4 years ago
Attachment #823120 - Flags: review?(allstars.chh)
(Assignee)

Updated

4 years ago
Attachment #823121 - Flags: review?(allstars.chh)
Comment on attachment 823119 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

I am okay with this patch.
However I'd like to request some comments from Hsinyi.

Currently cardState in IccManager is actually combining states from AppState, PersoState.
Now this patch will add PinState here.

Not sure if we need to file another bug to discuss this.
Attachment #823119 - Flags: review?(allstars.chh) → review+
Comment on attachment 823119 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

Press enter too fast,
see Comment 10.
Attachment #823119 - Flags: review?(htsai)

Updated

4 years ago
Attachment #823119 - Flags: review?(htsai) → review+
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #10)
> Comment on attachment 823119 [details] [diff] [review]
> Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to
> permanentBlocked when no more PUK retry.
> 
> Review of attachment 823119 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I am okay with this patch.
> However I'd like to request some comments from Hsinyi.
> 
> Currently cardState in IccManager is actually combining states from
> AppState, PersoState.
> Now this patch will add PinState here.
> 
> Not sure if we need to file another bug to discuss this.

Hi Yoshi,
 
May I know more about your concern here? Have you foreseen any drawbacks if we combine states from AppState, PersoState, PinState into 'cardState' attribute? 

What if we don't combine them? So in API we will have 'cardState' 'AppState' 'persoState' and 'pinState.' Then how would gaia use these different states? Maybe you could shed some light here? Thanks.
Comment on attachment 823119 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

Press too quickly... I need some answers to the previous comment. :)
Attachment #823119 - Flags: review+ → review?
Comment on attachment 823120 [details] [diff] [review]
Bug 930394 - Part 2: RIL patch. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

::: dom/system/gonk/ril_worker.js
@@ +3007,5 @@
> +      if (app.pin1 === CARD_PINSTATE_ENABLED_PERM_BLOCKED) {
> +        newCardState = GECKO_CARDSTATE_PERMANENT_BLOCKED;
> +      }
> +    }
> +

let pinState = app.pin1_replaced ?
               iccStatus.universalPINState :
               app.pin1;
if (pinState === CARD_PINSTATE_ENABLED_PERM_BLOCKED) {
  ...
}
Attachment #823120 - Flags: review?(allstars.chh) → review+
Comment on attachment 823121 [details] [diff] [review]
Bug 930394 - Part 3: Add test cases. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

Add r=me
Attachment #823121 - Flags: review?(allstars.chh) → review+
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #12)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #10)
> > Comment on attachment 823119 [details] [diff] [review]
> > Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to
> > permanentBlocked when no more PUK retry.
> > 
> > Review of attachment 823119 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I am okay with this patch.
> > However I'd like to request some comments from Hsinyi.
> > 
> > Currently cardState in IccManager is actually combining states from
> > AppState, PersoState.
> > Now this patch will add PinState here.
> > 
> > Not sure if we need to file another bug to discuss this.
> 
> Hi Yoshi,
>  
> May I know more about your concern here? Have you foreseen any drawbacks if
> we combine states from AppState, PersoState, PinState into 'cardState'
> attribute? 
> 
> What if we don't combine them? So in API we will have 'cardState' 'AppState'
> 'persoState' and 'pinState.' Then how would gaia use these different states?
> Maybe you could shed some light here? Thanks.

I don't have any concern, I already gave r+ on this.
Just let you know the cardState will include PinState now.
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #16)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #12)
> > (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #10)
> > > Comment on attachment 823119 [details] [diff] [review]
> > > Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to
> > > permanentBlocked when no more PUK retry.
> > > 
> > > Review of attachment 823119 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > I am okay with this patch.
> > > However I'd like to request some comments from Hsinyi.
> > > 
> > > Currently cardState in IccManager is actually combining states from
> > > AppState, PersoState.
> > > Now this patch will add PinState here.
> > > 
> > > Not sure if we need to file another bug to discuss this.
> > 
> > Hi Yoshi,
> >  
> > May I know more about your concern here? Have you foreseen any drawbacks if
> > we combine states from AppState, PersoState, PinState into 'cardState'
> > attribute? 
> > 
> > What if we don't combine them? So in API we will have 'cardState' 'AppState'
> > 'persoState' and 'pinState.' Then how would gaia use these different states?
> > Maybe you could shed some light here? Thanks.
> 
> I don't have any concern, I already gave r+ on this.
> Just let you know the cardState will include PinState now.

Thanks, Yoshi. I think having combining state is easy to use if every state itself indeed presents clearly.
Comment on attachment 823119 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry.

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

r+ per comment 16.
Attachment #823119 - Flags: review? → review+
(Assignee)

Comment 19

4 years ago
Created attachment 828393 [details] [diff] [review]
[Final] Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi, r=hsinyi.
Attachment #823119 - Attachment is obsolete: true
(Assignee)

Comment 20

4 years ago
Created attachment 828394 [details] [diff] [review]
[Final] Bug 930394 - Part 2: RIL patch. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi.
Attachment #823120 - Attachment is obsolete: true
(Assignee)

Comment 21

4 years ago
Created attachment 828395 [details] [diff] [review]
[Final] Bug 930394 - Part 3: Add test cases. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi.
Attachment #823121 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Hi, Shawn
Please remove the '[Final]' words from your patches.
Keywords: checkin-needed
(Assignee)

Comment 24

4 years ago
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #23)
> Hi, Shawn
> Please remove the '[Final]' words from your patches.

Thanks, Yoshi, Do it right now.
(Assignee)

Comment 25

4 years ago
Created attachment 828552 [details] [diff] [review]
Bug 930394 - Part 1: add comment in IDL. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi, r=hsinyi.
Attachment #828393 - Attachment is obsolete: true
(Assignee)

Comment 26

4 years ago
Created attachment 828553 [details] [diff] [review]
Bug 930394 - Part 2: RIL patch. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi.
Attachment #828394 - Attachment is obsolete: true
(Assignee)

Comment 27

4 years ago
Created attachment 828554 [details] [diff] [review]
Bug 930394 - Part 3: Add test cases. B2G RIL: Correct cardState to permanentBlocked when no more PUK retry. r=yoshi.
Attachment #828395 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/e42cb0c56ca2
https://hg.mozilla.org/mozilla-central/rev/cbca363f09ee
https://hg.mozilla.org/mozilla-central/rev/844f6bf9bc17
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 years ago
Depends on: 936471

Updated

4 years ago
No longer depends on: 936471
You need to log in before you can comment on or make changes to this bug.