STR: - Enable SIM PIN and reboot Expected: SimPinDialog should show up prompting for PIN. Observed: No SimPinDialog shows up even though the settings apps shows that the SIM card is locked. This issue seems like a regression caused by bug 928295.

The root cause of this issue is that iccId is not yet available by the time simslot_manager creates a new SIMSlot at http://mxr.mozilla.org/gaia/source/apps/system/js/simslot_manager.js#25. RIL eventually sends IccId to gaia by calling RIL:IccInfoChanged message which triggers the iccdetected event at http://mxr.mozilla.org/gaia/source/apps/system/js/simslot_manager.js#111 but sim_lock.js doesn't register for this event and therefore doesn't call showIfLocked method again.

I think this is not caused by Bug 928295, it seems that it is a new design in System app. I can take a look at this bug first and ask @Alive about more details. Thanks .

(In reply to Anshul from comment #1) > The root cause of this issue is that iccId is not yet available by the time > simslot_manager creates a new SIMSlot at > http://mxr.mozilla.org/gaia/source/apps/system/js/simslot_manager.js#25. > > RIL eventually sends IccId to gaia by calling RIL:IccInfoChanged message > which triggers the iccdetected event at > http://mxr.mozilla.org/gaia/source/apps/system/js/simslot_manager.js#111 but > sim_lock.js doesn't register for this event and therefore doesn't call > showIfLocked method again. Hi Anshul, I traced the code and yes, there is a situation when there is no iccId at the first time and we will not inject iccObject into SIMSlot.simCard. But after checking more details, you can find there is a `iccdetected` event registered on SIMSlotManager (http://mxr.mozilla.org/gaia/source/apps/system/js/simslot_manager.js#29) and will try to update slot with newest iccObject and register necessary events on the slot. Tested more than five times here, but current code works well because `oniccdetected` will definitely be triggered before `will-lock` and the pin dialog shows up on bootup. Any further details ? Thanks Anshul !

@eragonj could you please provide a patch with debug messages that would help find the root cause of the issue? When is will-lock event generated?

Sorry for the typo, it is `will-unlock` instead.

Ej Chen, I added some debug messages and I don't set the will-unlock event happening after the icc-detected event. Please see the logs below. 1270 40:12-19 12:14:06.776 I/GeckoDump( 307): -*- simslot_manager conn length 1 1271 40:12-19 12:14:06.776 I/GeckoDump( 307): -*- simslot_manager conn iccid null 1272 40:12-19 12:14:06.786 I/GeckoDump( 307): -*- simslot sim card null 1490 40:12-19 12:14:11.646 I/GeckoDump( 307): -*- sim_lock handle event will-unlock 1491 40:12-19 12:14:11.646 I/GeckoDump( 307): -*- will-unlock handle event 1492 40:12-19 12:14:11.646 I/GeckoDump( 307): -*- sim_lock in showIfLocked 1493 40:12-19 12:14:11.646 I/GeckoDump( 307): -*- sim_lock getslots currentslotindex undefined index 0 1494 40:12-19 12:14:11.646 I/GeckoDump( 307): -*- sim_lock no sim card 1682 40:12-19 12:14:12.746 I/GeckoDump( 307): -*- simslot_manager iccdetected 89014104232824538333

Ej Chen, I think I found the issue. Following are my observations: 1. The event will-unlock happens if the screen lock is enabled. In my case the screen lock was disabled so will-unlock never happens and so I never see the sim pin dialog on bootup. If I enable the screen lock from the settings app then this seems to work fine. However the PIN dialog needs to be shown on bootup with or without the screen lock. 2. Once I am in a PIN dialog and I begin entering PIN, If I press Skip button then all it does is dismisses the keyboard but PIN dialog stays until I hit Skip again.

1. even my phone without screen lock, it will still show up on bootup. Not sure what's wrong here .. 2. I have never encountered this situation before. @Alive, can you give me some hints about this bug ? thanks :)

(In reply to EJ Chen [:eragonj](PTO: 12/25 ~ 1/5) from comment #8) > 1. even my phone without screen lock, it will still show up on bootup. Not > sure what's wrong here .. Ej Chen, that is because there is a difference in behavior between Moz RIL and QC RIL in terms of when iccid is notified when phone is booting up. > 2. I have never encountered this situation before. > > @Alive, can you give me some hints about this bug ? thanks :)

I suggest to register 'screenchange' event handler in sim_lock to catch the case the lockscreen.enabled is false.

(In reply to Anshul from comment #9) > Ej Chen, that is because there is a difference in behavior between Moz RIL > and QC RIL in terms of when iccid is notified when phone is booting up. I got confused, Anshul. Because you said that QC ril handles differently compared with Moz ril so there might be a loading priority issue makes sim_lock not able to get defined icc information to show up simpin dialog. You can see the code here : http://mxr.mozilla.org/gaia/source/apps/system/js/sim_lock.js#151 (In reply to Anshul from comment #7) > 1. The event will-unlock happens if the screen lock is enabled. In my case > the screen lock was disabled so will-unlock never happens and so I never see > the sim pin dialog on bootup. If I enable the screen lock from the settings > app then this seems to work fine. However the PIN dialog needs to be shown > on bootup with or without the screen lock. And aslo, you left this comment. It means that if we turns on `lockscreen.enabled` in mozSettings, the simpin dialog will show up on bootup even if we didn't change the way QC ril how to handle simcards ! And I don't think a mozSetting key will change ril behavior like this. Maybe we can clarify few things together below : 1. Why QC ril works differently after setting `lockscreen.enabled` ? If we set this key `true` or `false` will make QC ril handle simcards differently ? 2. Does `will-unlock` event will be received in sim_lock.js no matter `lockscreen.enabled` is `true` or `false` ? From my side, no matter `lockscreen.enabled` is `true` or `false`, sim_lock.js can still receive `will-unlock` event and dialog will show up ! Hope this information helps ! Thanks for your help, Anshul and happy Xmas first :)

(In reply to Alive Kuo [:alive][NEEDINFO][OOO:12/24-1/4] from comment #10) > I suggest to register 'screenchange' event handler in sim_lock to catch the > case the lockscreen.enabled is false. Hi Alive, this event works but after testing, if we use this event, then simpin dialog shows up every time when users wake his phone up. I am not sure whether this is consistent with UX spec, or is it possible to handle on another event that will be triggered on bootup at the first time (? thanks :)

Hi Anshul, is there any update about this bug (? Thanks !

(In reply to EJ Chen [:eragonj] from comment #11) > (In reply to Anshul from comment #9) > > Ej Chen, that is because there is a difference in behavior between Moz RIL > > and QC RIL in terms of when iccid is notified when phone is booting up. > > I got confused, Anshul. Because you said that QC ril handles differently > compared with Moz ril so there might be a loading priority issue makes > sim_lock not able to get defined icc information to show up simpin dialog. > > You can see the code here : > http://mxr.mozilla.org/gaia/source/apps/system/js/sim_lock.js#151 > > (In reply to Anshul from comment #7) > > 1. The event will-unlock happens if the screen lock is enabled. In my case > > the screen lock was disabled so will-unlock never happens and so I never see > > the sim pin dialog on bootup. If I enable the screen lock from the settings > > app then this seems to work fine. However the PIN dialog needs to be shown > > on bootup with or without the screen lock. > > And aslo, you left this comment. It means that if we turns on > `lockscreen.enabled` in mozSettings, the simpin dialog will show up on > bootup even if we didn't change the way QC ril how to handle simcards ! And > I don't think a mozSetting key will change ril behavior like this. > > Maybe we can clarify few things together below : > > 1. Why QC ril works differently after setting `lockscreen.enabled` ? If we > set this key `true` or `false` will make QC ril handle simcards differently > ? QC RIL doesn't work differently based on this setting. All I was saying that QC reports the iccid later than Moz RIL and due to this bug in the gaia code the PIN unlock screen doesn't show up on bootup if the screen is not locked. > > 2. Does `will-unlock` event will be received in sim_lock.js no matter > `lockscreen.enabled` is `true` or `false` ? > I don't see will-unlock getting generated if the screen is not locked. Please share a patch with me with logs that you want and I can run and share the logs with you to help you debug. > > From my side, no matter `lockscreen.enabled` is `true` or `false`, > sim_lock.js can still receive `will-unlock` event and dialog will show up ! > Hope this information helps ! > > Thanks for your help, Anshul and happy Xmas first :)

Hi Anshul, this is a quick patch to see whether screenchanage event can solve the problem or not based on Alive's comment.

Hi Anshul, can you help me test your device with this debug patch ? And by the way, is it possible for me to get a working device that can let me debug on my side here ? THanks !

ni Anshul for patch testing.

(In reply to EJ Chen [:eragonj] from comment #16) > Created attachment 8356447 [details] > debug patch > > Hi Anshul, can you help me test your device with this debug patch ? > > And by the way, is it possible for me to get a working device that can let > me debug on my side here ? > > THanks ! Ej Chen, your patch didn't apply on the latest 1.3 so I had to manually make the changes. With this patch I am still not seeing the PIN Lock UI on bootup but I do when I tried to open the dialer app. Please find attached the log with no PIN UI on bootup.

@Anshul, I will take a look at your debug log and plz test with this patch https://bug951337.bugzilla.mozilla.org/attachment.cgi?id=8356443 which is based on Alive's comment. Thanks !

Please provide test results when you get a chance

EJ, Please provide next steps here. Is there patch that can be attached for code review?

(In reply to EJ Chen [:eragonj] from comment #20) > @Anshul, > > I will take a look at your debug log and plz test with this patch > https://bug951337.bugzilla.mozilla.org/attachment.cgi?id=8356443 which is > based on Alive's comment. Displaying PIN UI everytime the phone screen turns on is not a desired UX behavior. Could you please check with UX team if that is an acceptable behavior. I will test the patch tomorrow. In the meanwhile could you please debug why the PIN UI doesn't show up even though will-unlock event is generated? > > Thanks !

No Preeti, there is no ready-to-review patch now for this bug. Anshul and I are still working on this bug to find the problem.

(In reply to Anshul from comment #23) > (In reply to EJ Chen [:eragonj] from comment #20) > > @Anshul, > > > > I will take a look at your debug log and plz test with this patch > > https://bug951337.bugzilla.mozilla.org/attachment.cgi?id=8356443 which is > > based on Alive's comment. > Displaying PIN UI everytime the phone screen turns on is not a desired UX > behavior. Could you please check with UX team if that is an acceptable > behavior. I will test the patch tomorrow. In the meanwhile could you please > debug why the PIN UI doesn't show up even though will-unlock event is > generated? > > > > > Thanks ! Sorry Anshul, there are too many bugs on my side now, at least we made sure `will-unlock` event was emitted and I will try to find deeper about problem and leave comments here. Thanks Anshul !

Hi Anshul, can you try with debug patch again ? thanks ! I think the problem may be that when we received `will-unlock`, all simcards are not available at that time so that we can't show the simpin dialog. Can you try this patch twice, one with "lock screen enabled" and the other one with "lock screen diabled" and attach all logs here ? Thanks :) -- E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/simslot.js:30 in onIccChange: [EJ] iccchange >> 89886920030022106702 E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/simslot.js:30 in onIccChange: [EJ] iccchange >> 89886920031031452269 E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/sim_lock.js:84 in sl_handleEvent: [EJ] got will-unlock event E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/sim_lock.js:158 in sl_showIfLocked: [EJ] how many simcards > 2 E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/sim_lock.js:172 in iterator: [EJ] card 0, cardState ready E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/sim_lock.js:172 in iterator: [EJ] card 1, cardState ready E/GeckoConsole( 379): Content JS WARN at app://system.gaiamobile.org/js/homescreen_launcher.js:104 in hl_getHomescreen: HomescreenLauncher: not ready right now. E/GeckoConsole( 379): Content JS LOG at app://system.gaiamobile.org/js/lockscreen.js:626 in ls_unlock: [EJ] emit will-unlock event

BTW, I just updated the debug patch on Github with more debug log. You can just pull the branch to test. Thanks !

(In reply to EJ Chen [:eragonj] from comment #27) > BTW, I just updated the debug patch on Github with more debug log. You can > just pull the branch to test. Thanks ! Hey EJ, please note that I am testing the scenario on a single SIM device and not multisim.

(In reply to Anshul from comment #30) > (In reply to EJ Chen [:eragonj] from comment #27) > > BTW, I just updated the debug patch on Github with more debug log. You can > > just pull the branch to test. Thanks ! > Hey EJ, please note that I am testing the scenario on a single SIM device > and not multisim. ok ! thanks. I will focus on SIM device to see what's going on.

Anshul, this is another experimental patch, please try it in your local. Thanks.

Some notes : After discussion with Edgar, the root cause may be the timing issue based on comment #26. Moz ril may work quickly before we receive `will-unlock` event so that we can fetch iccObject from IccManager and show related simpin dialog to users. But in Qualcomm ril, it may not work so quickly before receiving the event, that's why we won't show any simpin dialog. And the reason why when we set 'lockscreen.enabled' to true will make simpin dialog show up is because lockscreen is enabled, there is a UI blocking users action and will send `will-unlock` event after he/she unlocks lockscreen. In this scenario, there is no timing issue because `iccdetected` event must be emitted before unlocking lockscreen. Thanks.

(In reply to EJ Chen [:eragonj] from comment #33) > Some notes : > > After discussion with Edgar, the root cause may be the timing issue based on > comment #26. > > Moz ril may work quickly before we receive `will-unlock` event so that we > can fetch iccObject from IccManager and show related simpin dialog to users. > But in Qualcomm ril, it may not work so quickly before receiving the event, > that's why we won't show any simpin dialog. > > And the reason why when we set 'lockscreen.enabled' to true will make simpin > dialog show up is because lockscreen is enabled, there is a UI blocking > users action and will send `will-unlock` event after he/she unlocks > lockscreen. In this scenario, there is no timing issue because `iccdetected` > event must be emitted before unlocking lockscreen. > > Thanks. Yes, we encounter this issue with below two conditions: 1). In lockScreen off case, there is still 'will-unlock' sended but much faster than lockScreen on case. (In lockScreen on case, 'will-unlock' won't be sended until user unlocks lockscreen). 2). And as mention in comment #14, QC RIL reports iccdetected slower than MOZ ril. (That is why we did not encounter same issue with MOZ ril). So system app will receive "will-unlock" first then "iccdetected" when lockscreen off. Finally system app will not show pin page.

Comment on attachment 8361001 [details] patch on master Hi Alive, based on my comment #33, can you give me some feedbacks about this problem and my experimental patch ? Just want to make sure this experimental patch is on the right direction and wont break anything. Thanks !

PM triaged this bug and it be a 1.3 blocker.

(In reply to EJ Chen [:eragonj] from comment #32) > Created attachment 8361001 [details] > experimental patch > > Anshul, this is another experimental patch, please try it in your local. > > Thanks. EJ Chen, this patch seems to have fixed the problem. Thanks a lot for your support.

(In reply to Anshul from comment #37) > (In reply to EJ Chen [:eragonj] from comment #32) > > Created attachment 8361001 [details] > > experimental patch > > > > Anshul, this is another experimental patch, please try it in your local. > > > > Thanks. > EJ Chen, this patch seems to have fixed the problem. Thanks a lot for your > support. Good to hear that, Anshul. We are almost there ! I'll start to ask for review of this bug. Thanks !

Comment on attachment 8361001 [details] patch on master Hi Alive, Anshul said this experimental patch fixed the problem. I need your review about this ! Thanks :)

Comment on attachment 8361001 [details] patch on master This seems a little strange :/ If the lockscreen is unlocked and the iccdetected event is too late while we are already in some other app, do we need to prompt SIM PIN unlock as well?

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #40) > Comment on attachment 8361001 [details] > experimental patch > > This seems a little strange :/ > > If the lockscreen is unlocked and the iccdetected event is too late while we > are already in some other app, do we need to prompt SIM PIN unlock as well? Yeah I know this bug is weird, that's why I focus on the bootup case (with removeEventListener). Your concern is right and I can't give you the answer to your question. There is no any flow talking about this case because this problem won't happen in our ril, that's why I call this a hack. If this really happens, we may have to fix this in different way ? 1. can gecko throw this event quicker when communicating with ril ? (or maybe someone can give ril an update about this case ?) 2. if we can't do anything about fixing ril & gecko, should we involve UX to discuss more in this case and think about any possible scenario like what you mentioned above ?

(In reply to EJ Chen [:eragonj] from comment #41) > (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #40) > > Comment on attachment 8361001 [details] > > experimental patch > > > > This seems a little strange :/ > > > > If the lockscreen is unlocked and the iccdetected event is too late while we > > are already in some other app, do we need to prompt SIM PIN unlock as well? > > Yeah I know this bug is weird, that's why I focus on the bootup case (with > removeEventListener). > > Your concern is right and I can't give you the answer to your question. > There is no any flow talking about this case because this problem won't > happen in our ril, that's why I call this a hack. > > If this really happens, we may have to fix this in different way ? > > 1. can gecko throw this event quicker when communicating with ril ? (or > maybe someone can give ril an update about this case ?) > 2. if we can't do anything about fixing ril & gecko, should we involve UX to > discuss more in this case and think about any possible scenario like what > you mentioned above ? I still don't understand what's going to be fixed. 1. If iccdetected -> will-unlock: Listen to will-unlock should work. 2. If will-unlock -> iccdeteced: This is what I wonder. So why this is a bug? What's really happening? Is (2) happening now? Or (1) and (2) happens intermittently?

Comment on attachment 8361001 [details] patch on master Please try alternative to implement SIMSlotManager.ready() Thanks.

Comment on attachment 8361001 [details] patch on master Hi Alive, how do you think about this updated patch :) ?

Comment on attachment 8361001 [details] patch on master See github. Please also add unit test for SIMSlotManager.

Comment on attachment 8361001 [details] patch on master Hi Alive, just added missing tests and I think we are almost there :D Please help me review again when you have time ! Big thanks !!

Comment on attachment 8361001 [details] patch on master r+ with nits

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #48) > Created attachment 8362818 [details] > patch on v1.3 Just got r+ from Alive and all patches are green now, so we are ready to go.

Special thanks to @anshul, @alive and @edgar for your great help. Just landed on, Gaia/master : 76ffeeef7a0b119a9f145a04319803dcd33d3ed1 Gaia/v1.3 : 47049555282a9a01fb60d1e1421b57e2810c96f5 Thanks :)