1059837 - [DSDS] Skip SIM pin 1 but SIM pin 2 is also skipped

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Eric Chang [:ericcc] [:echang]]

### STR
1. SIM pin on for both SIM
2. Reboot
3. Skip SIM1 pin

### Expected
SIM2 pin request page shown if SIM1 is skipped

### Actual
SIM2 pin request page not shown if SIM1 is skipped
http://youtu.be/yPPO56-yiBM

### Version
Gaia      3a838afca295c9db32e1a3ec76d49fb7fe7fd2d2
Gecko     https://hg.mozilla.org/mozilla-central/rev/5f0b5cc8f78d
BuildID   20140827160207
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

Comment 1

[Eric Chang [:ericcc] [:echang]]

[Blocking Requested - why for this release]: regression.

Comment 2

[Eric Chang [:ericcc] [:echang]]

20140531160201 oK
20140623160208 ok - last good build
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-central-flame-eng/2014/06/2014-06-23-16-02-08/

20140624040203 fail
20140624160202 fail
20140626040205 fail
20140628160201 fail
20140704160202 fail
20140713160201 fail

Comment 3

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20140619080214
Gaia: 135a13ea0ee333314b6a12cb19f18617308a3b46
Gecko: ea703db56bcf
Platform Version: 33.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

First broken
BuildID: 20140619161246
Gaia: ccd70903544486bea04e85d8a4aacf63f1de2a72
Gecko: 79e69d064957
Platform Version: 33.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Working Gaia / Broken Gecko = No repro
Gaia: 135a13ea0ee333314b6a12cb19f18617308a3b46
Gecko: 79e69d064957
Broken Gaia / Working Gecko = Repro
Gaia: ccd70903544486bea04e85d8a4aacf63f1de2a72
Gecko: ea703db56bcf
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/135a13ea0ee333314b6a12cb19f18617308a3b46...ccd70903544486bea04e85d8a4aacf63f1de2a72


B2G inbound
Last working
BuildID: 20140619074315
Gaia: c2c92e7be95dd0b65ee44113e1d17592dd0e20c6
Gecko: 08a6760dbed5
Platform Version: 33.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

First broken
BuildID: 20140619085213
Gaia: 8ccac0744848bd3ceaa6aa1c972ae0eb1e9ddc1e
Gecko: 07b8810f7448
Platform Version: 33.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Working Gaia / Broken Gecko = No repro
Gaia: c2c92e7be95dd0b65ee44113e1d17592dd0e20c6
Gecko: 07b8810f7448
Broken Gaia / Working Gecko = Repro
Gaia: 8ccac0744848bd3ceaa6aa1c972ae0eb1e9ddc1e
Gecko: 08a6760dbed5
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/c2c92e7be95dd0b65ee44113e1d17592dd0e20c6...8ccac0744848bd3ceaa6aa1c972ae0eb1e9ddc1e

Comment 4

[Joshua Mitchell (Inactive)]

possibly broken by Bug 1007948 - can you take a look Guillaume ?

Comment 5

[Alberto Pastor [:albertopq]]

Not sure how to approach this... I have been trying several things, but still don't understand all the paths in which showIfLocked (in sim_lock.js) is called. Is definetely a regression from Bug 1007948, but didn't find a way of fixing one without breaking the other...

Any ideas?

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

Unfortunately, I am also not familiar with this code. Guillaume, you are ni?'d on this bug. Can you take this off of Alberto's plate since you fixed bug 1007948 and are probably the most familiar with this code?

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alberto Pastor [:albertopq] from comment #5)
> Not sure how to approach this... I have been trying several things, but
> still don't understand all the paths in which showIfLocked (in sim_lock.js)
> is called. Is definetely a regression from Bug 1007948, but didn't find a
> way of fixing one without breaking the other...
> 
> Any ideas?

I guess I am the right person to ask questions.

It might be a design failure because we show the lock state of two SIM cards in the same UI. Once any of them is dispatching some state change event we will have timing issue because we have only one copy of SIM lock UI.

Not sure if we have a better way in a blocker..

Comment 8

[Guillaume Marty [:gmarty]]

A fix sounds difficult and I'm not very familiar with the code.
When looking into it I found that in sim_lock.js, l. 73 and 82, this.length should probably refer to SIMSlotManager.length (the way the code is working won't change much though).

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> (In reply to Alberto Pastor [:albertopq] from comment #5)
> > Not sure how to approach this... I have been trying several things, but
> > still don't understand all the paths in which showIfLocked (in sim_lock.js)
> > is called. Is definetely a regression from Bug 1007948, but didn't find a
> > way of fixing one without breaking the other...
> > 
> > Any ideas?
> 
> I guess I am the right person to ask questions.
> 
> It might be a design failure because we show the lock state of two SIM cards
> in the same UI. Once any of them is dispatching some state change event we
> will have timing issue because we have only one copy of SIM lock UI.
> 
> Not sure if we have a better way in a blocker..

If I am understanding correctly, without a major re-work we can either choose between this bug or bug 1007948?

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Michael Henretty [:mhenretty] from comment #9)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> > (In reply to Alberto Pastor [:albertopq] from comment #5)
> > > Not sure how to approach this... I have been trying several things, but
> > > still don't understand all the paths in which showIfLocked (in sim_lock.js)
> > > is called. Is definetely a regression from Bug 1007948, but didn't find a
> > > way of fixing one without breaking the other...
> > > 
> > > Any ideas?
> > 
> > I guess I am the right person to ask questions.
> > 
> > It might be a design failure because we show the lock state of two SIM cards
> > in the same UI. Once any of them is dispatching some state change event we
> > will have timing issue because we have only one copy of SIM lock UI.
> > 
> > Not sure if we have a better way in a blocker..
> 
> If I am understanding correctly, without a major re-work we can either
> choose between this bug or bug 1007948?

Hmmm...maybe we should backout bug 1007948 and re-consider it's a blocker to work on and find out what's causing the regression? or it is not a regression...

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #10)
> (In reply to Michael Henretty [:mhenretty] from comment #9)
> > If I am understanding correctly, without a major re-work we can either
> > choose between this bug or bug 1007948?
> 
> Hmmm...maybe we should backout bug 1007948 and re-consider it's a blocker to
> work on and find out what's causing the regression? or it is not a
> regression...

Yup, I agree with this course of action. This bug is worse than bug 1007948, and the solution in bug 1007948 does not handle the DSDS case. Let's backout bug 1007948, and close this one as WFM.

Comment 12

[Alberto Pastor [:albertopq]]

You need to take into account the dulplicate of bug 1007948, bug 998918 as well for taking this decision

Comment 13

[Michael Henretty [:mikehenrty][:mhenretty]]

Fixed by backing out bug 1007948.

master: https://github.com/mozilla-b2g/gaia/commit/c9f346c66434b7256593d986cff7fc2f6bc3a49c
v2.1: https://github.com/mozilla-b2g/gaia/commit/ad374f4b2a07cb5711679322a815f9feadb5cf64

Comment 14

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alberto Pastor [:albertopq] from comment #12)
> You need to take into account the dulplicate of bug 1007948, bug 998918 as
> well for taking this decision

How do you mean, take this into account?

Comment 15

[Alberto Pastor [:albertopq]]

I meant that bug 1007948 was also fixing bug 998918, and the use case in the second bug is also quite important. Just tried with current master:

1.- Restart the phone
2.- Enter a wrong PIN on SIM 1 (you'll see an error message)
3.- Press the power button (screen goes black)
4.- Press again for waking up the screen
5.- Unlock, if necessary

Expected:

The error message is still there

Actual:

The error message is gone.

Not sure if the bug I just described (that was fixed by the patch we just backed out) is not as important as the one about skipping both sim cards.

Comment 16

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Alberto Pastor [:albertopq] from comment #15)
> I meant that bug 1007948 was also fixing bug 998918, and the use case in the
> second bug is also quite important. Just tried with current master:
> 
> 1.- Restart the phone
> 2.- Enter a wrong PIN on SIM 1 (you'll see an error message)
> 3.- Press the power button (screen goes black)
> 4.- Press again for waking up the screen
> 5.- Unlock, if necessary
> 
> Expected:
> 
> The error message is still there
> 
> Actual:
> 
> The error message is gone.
> 
> Not sure if the bug I just described (that was fixed by the patch we just
> backed out) is not as important as the one about skipping both sim cards.

Yeah, bug 998918 and bug 1007948 are indeed the same bug. I agree with you that they are both pretty important. However, this one was marked as a blocker while those two weren't. And in any case, nobody could figure out how to solve this one with the solution implemented in bug 1007948 (not even Alive). It seems that in order to solve this bug we would would need to rework the solution in bug 1007948, so I figured we just backout that bug and resolve it there.

Comment 17

[Eric Chang [:ericcc] [:echang]]

Gaia-Rev        778ebac47554e1c4b7e9a952d73e850f58123914
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/aa98eb8873a2
Build-ID        20141005160206
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141005.194124
FW-Date         Sun Oct  5 19:41:35 EDT 2014
Bootloader      L1TC00011840

Comment 18

[Lancy(Leave from Mozilla)]

Attachment: Verify_Video_Flame v2.1.MP4

This issue has been verified successfully on Flame 2.1.
See attachment: Verify_Video_Flame v2.1.MP4
Reproducing rate: 0/10

Flame v2.1 version:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0