KTucker [:KTucker][Inactive 3/4/2016] Reporter
	Description • 10 years ago

Description:
The user cannot return to the homescreen from card view. The user will press the home button but nothing will happen.

Repro Steps:
1)  Updated Flame to Build ID: 20141023001201
2)  Open up any app.
3)  Press and hold the home button on the device to enter card view.
4)  While in card view, tap the home button on the dut to go to the homescreen.

Actual:
The user is not taken to the homescreen when tapping the home button in card view.

Expected:
The user is returned to the homescreen when they tap the home button on the dut.

Environmental Variables
Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141023001201
Gaia: 1e48e3e40e0780c0cd07a3457e5fe2efeeb542d1
Gecko: 09fb60a37850
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Notes:
Repro frequency: 100%
See attached: Logcat, Video

	KTucker [:KTucker][Inactive 3/4/2016] Reporter
	Comment 1 • 10 years ago

This issue was 100% on my device but then stopped reproducing. Working on solid repro. Other testers have hit this issue on occasion today.

	KTucker [:KTucker][Inactive 3/4/2016] Reporter
	Comment 2 • 10 years ago

100% repro by following these steps:

1. Factory reset the phone.
2. Open the settings app.
3. Hold down the home button to enter card view.
4. Tap the home button while in card view.

Actual:
The user is not returned to the homescreen.

This only happens on a fresh flash or factory reset.

	KTucker [:KTucker][Inactive 3/4/2016] Reporter
	Comment 3 • 10 years ago

Youtube: http://youtu.be/7ePdIycIAug

	KTucker [:KTucker][Inactive 3/4/2016] Reporter
	Comment 4 • 10 years ago

I cannot get a logcat for this issue since it only happens after a factory reset or fresh flash. The issue does not reproduce if i go into the developer menu and enable adb and devtools.

----------------------------------------------------------------------------------------------

This issue also reproduces on the Flame 2.2(319mb)(KK)(Full Flash) and the Flame 2.0(319mb)(KK)(Full Flash)

Nothing will happen when the user taps the home button while in card view. The user should be returned to the most recent app in 2.2.

On 2.0 the user is not returned home when they tap the home button in card view.

Flame 2.2 

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141023040204
Gaia: 27a1d1baaa8e375b70e043efee67d5f2206c330b
Gecko: 88adcf8fef83
Version: 36.0a1 (Master)
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141023001201
Gaia: 1e48e3e40e0780c0cd07a3457e5fe2efeeb542d1
Gecko: 09fb60a37850
Version: 34.0 (2.1)
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15
	Comment 5 • 10 years ago

[Blocking Requested - why for this release]:

The user should always be able to return home with the home button. Nominating this to block 2.0

	Josh Cheng [:josh]
	Comment 6 • 10 years ago

Hi Peipei,
Can you verify this on Woodduck 2.0M and Flame 2.0/2.1? Thanks!

	Peipei Cheng (needinfo if you need my action)
	Comment 7 • 10 years ago

With the same device and the same steps as comment 2, I can reproduce
this issue on Flame V2.2 and V2.1. But on Flame V2.0, I didn't reproduce.

Also I didn't reproduce this on Woodduck 2.0M.


Attached is the adb log for V2.1. Test time starts: 10-28 05:19:27.051


Woodduck build
----------------------------------------------------------------------------
Gaia-Rev        fdb8236d7d99061ef6bedc021fd6b482e1af3f5a
Gecko-Rev       717b111a9f7102a3fe76cece8f21a5c794e3bddf
Build-ID        20141027050313
Version         32.0
Device-Name     soul35
FW-Release      4.4.2
FW-Incremental  1414357512
FW-Date         Mon Oct 27 05:05:35 CST 2014

Flame V2.0
---------------------------------------------------------------------------------
Gaia-Rev        5e532a84e762b1bb6231756182cf1465671a061e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/124f0bed1700
Build-ID        20141027160201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141027.194042
FW-Date         Mon Oct 27 19:40:52 EDT 2014
Bootloader      L1TC00011880

Flame V2.2
-------------------------------------------------------------------------------
Gaia-Rev        dc496d04907dd314f9736ff78bab3bd27156f79a
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/f2d7d694aae5
Build-ID        20141020160202
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141020.191321
FW-Date         Mon Oct 20 19:13:32 EDT 2014
Bootloader      L1TC00011880


Flame V2.1
---------------------------------------------------------------------------------
Gaia-Rev        3d9cc667f4e929861a9a77c41096bbf5a9c1bde0
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/928b18f7d8ff
Build-ID        20141021161205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141021.193023
FW-Date         Tue Oct 21 19:30:32 EDT 2014
Bootloader      L1TC10011800

	Wesly Huang (EPM)
	Comment 8 • 10 years ago

[Blocking Requested - why for this release]:

[Traige]per comment#2 propose to nom. to 2.1, as only seen in corner case.

	Gregor Wagner [:gwagner]
	Comment 9 • 10 years ago

Seems like a regression.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 10 • 10 years ago

I tracked this down in 2.1 to https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/system/js/lockscreen.js#L261 - the lockscreen listens for 'home' events and stops them if it thinks it is locked. When this bug repros, this.locked is true so the task manager never sees the event. You can confirm this also by reproducing with the steps in comment #2, then let the screen lock, unlock and attempt again - this time it works. 

This sounds like a dupe then of bug 1089028, which was supposed to have been fixed in bug 1086175. That too affected 2.2 and 2.1. Greg can you take this?

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 11 • 10 years ago

While I'm bisecting I've found a interesting things is if I checkout out the version 2b4d4027d56c89ea0bba1239278bfdf1e03beda9, to press homekey it would cancel the card view and return to the app, but not the Homescreen.

Still bisecting since at v2.1 merge day it works. So I think I can find the actual regression patch and to see what happened.

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 12 • 10 years ago

I've bisected that a93410a4ef8ff11ff042e2ccbb26001eddd03285 is the first broken commit, and after I checkout & reverted it, the cardview works fine. However, I've not catch the possible root cause of this (yet). Sam, can you take a look?

----

commit a93410a4ef8ff11ff042e2ccbb26001eddd03285
Merge: c1fef90 2a17f89
Author: Sam Foster <sam@sam-i-am.com>
Date:   Tue Oct 14 11:47:58 2014 -0700

    Merge pull request #25100 from sfoster/not-shown-tm-bug-1081528
    
    v2.1 Bug 1081528 - Only handle attentionscreenopened events while TM is showing. r=alive a=fabrice

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 13 • 10 years ago

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #12)
> I've bisected that a93410a4ef8ff11ff042e2ccbb26001eddd03285 is the first
> broken commit, and after I checkout & reverted it, the cardview works fine.
> However, I've not catch the possible root cause of this (yet). Sam, can you
> take a look?

The v2.1 Bug 1081528 patch was fixing a rather bad merge/oversight in which Task Manager was wrongly listening and responding to some events even while hidden. So, I'm not sure it makes sense for this to be the cause, rather it revealed an earlier bug. I'm also not sure when that was first introduced, I passed this along to you as as far as I could tell the behavior described in this bug was down to lockscreen.locked being true after stepping through the FTU and going to use the task manager for the first time. That agrees with the observation that the bug does not reproduce when you flash gaia with DEVICE_DEBUG=1 - as that flag sets NO_LOCK_SCREEN. This bug seems to be another dupe of bug 1089028 - when we enter the FTU, that locked flag should be toggled off. I'm just not sure of the best way or place to do that? In FTULauncher, or in lockscreen itself?

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 14 • 10 years ago

If you want to control the LockScreen flag, the modification should be put in LockScreen.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 15 • 10 years ago

From the looks of https://github.com/mozilla-b2g/gaia/blob/master/apps/system/lockscreen/js/lockscreen.js#L652 (and, in v2.1 https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/system/js/lockscreen.js#L728 ) it seems the intention is that lockscreen gets unlocked during FTU, but this happens intermittently in master, and not at all in v2.1

In master, it looks like we might have a race between the lockscreen-frame-bootstrap event, and the homescreen-ready event which is used in the main system bootstrap to kick off the FTU. I'm seeing something similar in v2.1, I captured this with some extra logging in the bootstrap.js and lockscreen.js files: 

...
"lockScreen.lockIfEnabled: false" lockscreen.js:736
"Boostrap, safelyLaunchFTU, calling FtuLauncher.retrieve" bootstrap.js:74
...
"lockScreen, got event: ftuopen" lockscreen.js:353
"lockScreen, got event: ftudone"

I can add a this.unlock() in a ftudone/ftuskip handler. But as it looks like the intention is that we are unlocked when the FTU is running, I may need to adjust the bootstrap sequence. Or perhaps a call to unlock in a ftuopen handler will work just as well. 

I'll put a patch together with using the ftuopen approach and probably a marionette test to cover it.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 16 • 10 years ago

Hrm, actually with the refactoring in 2.2/master, I'm not sure how/where to fix this in that branch. I see LockScreenManager watches ftuopen already but I think that closeApp(true) call will do nothing in this case as the LockScreenWindow instance is not active, leaving window.lockScreen.locked true. Meantime I'll work on 2.1

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 17 • 10 years ago

We have a race condition in which lockScreen checks FtuLauncher.isFtuRunning() before the FTU has started. This patch adds a handler for the ftuopen event to catch this case. This also fixed the edge-case where FTU is launched from the Settings app. 

Note that the race plays out differently in b2g-desktop, so although I had written a marionette test for this, it passes both with and without the fix so seems a bit pointless. We can re-visit when we get on-device Gij test runs. 

The lockscreen code was refactored since 2.1 so we'll need a different patch there. I'm proposing the exact same patch the file just in its previous location. I'll put that together when this lands.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 18 • 10 years ago

Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

Thanks for finding the root cause.

I am a bit confused about this fix. So event listener of |ftuopen| is replaced with listener for |ftu| event, but in handleEvent you added a |ftuopen| case?

You might want to ask for :snowmantw for feedback as well.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 19 • 10 years ago

> I am a bit confused about this fix. So event listener of |ftuopen| is
> replaced with listener for |ftu| event, but in handleEvent you added a
> |ftuopen| case?

Yeah that's not right. And my push didn't trigger a treeherder job. I'll rebase, fix and push again with r? and f? for :snowmantw

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 20 • 10 years ago

Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

Re-pushed to fix the 'ftu' vs. 'ftuopen' event. I confirmed that patch didn't fix the issue as it was, and now does!

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 21 • 10 years ago

I think the reason that LockScreen doesn't handle 'ftuopen' is because we want to delegate the decision of window opening and closing to LockScreenWindowManager, rather than the legacy LockScreen. From your diagnosing and patch, I think we can read the current broken flow as:

After re-flashing: even no opened LockScreenWindow, LockScreen still add event listeners of 'ftuopen' and 'home'. This is because we created it once at LWM's starting stage at

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/lockscreen_window_manager.js#L387

And we close the window later after we receive 'ftuopen' in LWM:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/lockscreen_window_manager.js#L125

But since we now unfortunately have no iframe for LockScreenWindow, the code of LockScreen has been evaluated and thus the event handlers have been registered, even after the window has been closed or even killed. This become worse since the code in LockScreen check no whether it's active (according to the window status), but rely on a legacy flag 'this.locked' which now may not so accurate because window opening/closing has been delegated to LWM. So when the 'home' event comes, it could be incorrectly blocked according to 'this.locked === true' while it is should be false, because the window has been closed by LWM at the handler of 'ftuopen'.

I think the proper solution is to fix the 'this.locked', to let LWM update it when window has been closed or opened, and deprive the right the legacy LockScreen manage it to prevent unsync issue like this. However, I don't know whether this would result in unknown regressions. I would do some test for it.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 22 • 10 years ago

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #21)
> I think the reason that LockScreen doesn't handle 'ftuopen' is because we
> want to delegate the decision of window opening and closing to
> LockScreenWindowManager, rather than the legacy LockScreen. 

I did look through the LockscreenWindowManager and an earlier version of this patch attempted to make the change there - but I went back to the current approach with a mind to making uplift easier. It sounds like we're going to need 2 very different patches for 2.0/2.1 and 2.2. 

> I think the proper solution is to fix the 'this.locked', to let LWM update
> it when window has been closed or opened, and deprive the right the legacy
> LockScreen manage it to prevent unsync issue like this. However, I don't
> know whether this would result in unknown regressions. I would do some test
> for it.

To be clear, are you looking into this? I don't presently really understand the relationship between the new window and manager and the window.lockScreen / LockScreen. I can figure it out, but we have some urgency to get a patch ready for requesting approval for 2.1. We have a little more time to work this out for 2.2, and the 2.2 patch will not at all resemble the 2.1, so do you think it makes sense to review and request approval for a version of the current patch for 2.1 while we are figuring out an approach for 2.2?

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 23 • 10 years ago

Yes, I'm doing some test of it. But I think we should land your patch because it looks the simple flag change breaks entire LockScreen, at least that's what I now encountered. So my suggestion is using your patch to solve this bug, and I keep investigating to see what's the real proper solution of this (of course the ultimate solution is kill lockscreen.js and move toward as-an-app, but a proper middle term solution is necessary).

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 24 • 10 years ago

Update: I solved the issue. I would discuss with Tim to see how to solve this bug.

Sam: Thanks for your helpful diagnosing. Without that, I can't catch the architecture issue so quickly.

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 25 • 10 years ago

Need to discuss with Tim first.

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 26 • 10 years ago

Tim: In the WIP I sync the flag between window management and the legacy LockScreen component as I said. Please let me know if you have any concern about that.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 27 • 10 years ago

This is the same patch backported to 2.1. Should we move the master/2.2 work to another bug to let this 2.1 blocker move forward?

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 28 • 10 years ago

Comment on attachment 8522538 [details] [review]
2.1 PR Unlock lockScreen when FTU starts

Let's land this for v2.1 and clone another bug for v2.2. Thanks everyone!

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 29 • 10 years ago

Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

Greg will file a bug for v2.2 patch.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 30 • 10 years ago

Comment on attachment 8522538 [details] [review]
2.1 PR Unlock lockScreen when FTU starts

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Race condition between lockscreen and FTU launcher
[User impact] if declined: When first using the card view after the First Time Experience, the user cannot return to the home screen. If they have no apps open, the only option is to restart the device
[Testing completed]: Manual testing on Flame/2.1, new unit test (b2g-desktop environment doesnt have this race condition so we cant test usefully there)
[Risk to taking this patch] (and alternatives if risky): low risk - small change to ensure an FTU event we thought was being handled actually gets handled. 
[String changes made]: None

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 31 • 10 years ago

Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

So I would submit a patch for master. Clear the feedback.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 32 • 10 years ago

ping on this 2.1 approval request for Attachment #8522538 [details]

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 33 • 10 years ago

Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

Work in a fix for this issue in 2.2/master is taking place in bug 1098997

	[:fabrice] Fabrice Desré
	Comment 34 • 10 years ago

Can QA test a build with that patch? Thanks!

	Duane Dixon [:ddixon]
	Comment 35 • 10 years ago

Tested patch: "checkout unlock-in-ftu-2_1-bug-1088157" in Flame 2.1 build (shallow flash, eng. build, 319 MB). 

Issue appears to be fixed with the patch applied. 

Actual Results:  User can go to homescreen when in card view after flashing device and opening Settings app. 

Device: Flame 2.1
BuildID: 20141117201226
Gaia: 8be826ce66946f04611810b552759d06de3fdf1e
Gecko: 95fbd7635152
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 37 • 10 years ago

> Issue appears to be fixed with the patch applied. 
> 
> Actual Results:  User can go to homescreen when in card view after flashing
> device and opening Settings app. 
> 
> Device: Flame 2.1
..
Is this what you were looking for Fabrice? Let me know if there's anything more I can do to move this forward.

	bhavana bajaj [:bajaj]
	Comment 38 • 10 years ago

Sam, i think this is good to land now, what is the 2.2 bug here that needs to tracked so it does not fall through the cracks?

	bhavana bajaj [:bajaj]
	Comment 39 • 10 years ago

(In reply to bhavana bajaj [:bajaj] from comment #38)
> Sam, i think this is good to land now, what is the 2.2 bug here that needs
> to tracked so it does not fall through the cracks?

ignore comment , found it  https://bugzilla.mozilla.org/show_bug.cgi?id=1098997!, I'll make sure to track that.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 40 • 10 years ago

I dont have write access to the v2.1 branch it seems, leaving checkin-needed for attachment 8522538 [details] [review] (Github PR for Gaia / v2.1)

	bhavana bajaj [:bajaj]
	Comment 41 • 10 years ago

:kwierso, sorry for another adhoc request. Please switch the NI if someone else is bale to help check this in on "v2.1 gaia" alone? Thanks!

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 42 • 10 years ago

Gaia is currently closed which is probably why I wasn't able to merge. Once it opens up again I'll see if I can merge if no-one else has done so already

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 43 • 10 years ago

(In reply to bhavana bajaj [:bajaj] from comment #41)
> :kwierso, sorry for another adhoc request. Please switch the NI if someone
> else is bale to help check this in on "v2.1 gaia" alone? Thanks!
I'll grab it tonight if no one else does it first.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 44 • 10 years ago

v2.1: https://github.com/mozilla-b2g/gaia/commit/f8d3bf44029e0afc0124600a4bb34dba8fc1ad21

For future record, when Gaia isn't closed, anyone who can commit to gaia's master branch should have permissions to be able to merge things to 2.1 and other release branches.

	Carsten Book [:Tomcat]
	Comment 45 • 10 years ago

(In reply to Wes Kocher (:KWierso) from comment #44)
> v2.1:
> https://github.com/mozilla-b2g/gaia/commit/
> f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
> 
> For future record, when Gaia isn't closed, anyone who can commit to gaia's
> master branch should have permissions to be able to merge things to 2.1 and
> other release branches.

is there any other action item or can i delete the checkin-needed keyword ?

	Ryan VanderMeulen [:RyanVM]
	Comment 46 • 10 years ago

Was someone going to deal with the Gip permafail this is causing? Which was also visible on the pull request's Gaia Try runs, fwiw.
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=39251&repo=mozilla-b2g34_v2_1

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 47 • 10 years ago

(In reply to [Away 18-Nov to 23-Nov] Ryan VanderMeulen [:RyanVM UTC-5] from comment #46)
> Was someone going to deal with the Gip permafail this is causing? Which was
> also visible on the pull request's Gaia Try runs, fwiw.
> https://treeherder.mozilla.org/ui/logviewer.html#?job_id=39251&repo=mozilla-
> b2g34_v2_1

1000 apologies, I'm looking into it.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 48 • 10 years ago

I've run this test a dozen times locally without seeing a fail. Nor can I reproduce on device - flash, step through FTU, let screen lock then take screenshot to produce a lockscreen-notification. The reason I wasn't looking hard at Gip/Gij results in this PR was the bug itself doesn't reproduce there - the particular race-condition in which the lockscreen initializes and checks to see if Ftu is running - before it has launched - doesnt seem to manifest in the b2g-desktop environment, and the tests I had written passed with and without my patch. 

So, I'm pretty confident this failure is confined to the test/b2g-desktop environment. Zac, do you have any insights into why the FTU event handler in the patch would cause a failure in a test where the FTU is not even run? I can put a logging patch together to try and root it out, but that's my only idea at this point.

	Elie
	Comment 49 • 10 years ago

This issue has been successfully verified on Flame 2.1:
Gaia-Rev        1b231b87aad384842dfc79614b2a9ca68a4b4ff3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152
Build-ID        20141119001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2

	Zac C (:zac)
	Comment 50 • 10 years ago

(In reply to Sam Foster [:sfoster] from comment #48)
> I've run this test a dozen times locally without seeing a fail. Nor can I
> reproduce on device - flash, step through FTU, let screen lock then take
> screenshot to produce a lockscreen-notification. The reason I wasn't looking
> hard at Gip/Gij results in this PR was the bug itself doesn't reproduce
> there - the particular race-condition in which the lockscreen initializes
> and checks to see if Ftu is running - before it has launched - doesnt seem
> to manifest in the b2g-desktop environment, and the tests I had written
> passed with and without my patch. 
> 
> So, I'm pretty confident this failure is confined to the test/b2g-desktop
> environment. Zac, do you have any insights into why the FTU event handler in
> the patch would cause a failure in a test where the FTU is not even run? I
> can put a logging patch together to try and root it out, but that's my only
> idea at this point.

I'll run it locally today and let you know. It sounds very much like a race condition on the faster desktopb2g environment.

	Zac C (:zac)
	Comment 51 • 10 years ago

(In reply to Sam Foster [:sfoster] from comment #48)
> I've run this test a dozen times locally without seeing a fail. Nor can I
> reproduce on device - flash, step through FTU, let screen lock then take
> screenshot to produce a lockscreen-notification. The reason I wasn't looking
> hard at Gip/Gij results in this PR was the bug itself doesn't reproduce
> there - the particular race-condition in which the lockscreen initializes
> and checks to see if Ftu is running - before it has launched - doesnt seem
> to manifest in the b2g-desktop environment, and the tests I had written
> passed with and without my patch. 
> 
> So, I'm pretty confident this failure is confined to the test/b2g-desktop
> environment. Zac, do you have any insights into why the FTU event handler in
> the patch would cause a failure in a test where the FTU is not even run? I
> can put a logging patch together to try and root it out, but that's my only
> idea at this point.

I've just finished testing it using a nightly gecko and the Gaia (build just using `make`) from when your PR was merged. My OS is linux64.

I can replicate the failure 100% with your patch. The wake unlock is not working with your patch. When the notification is pushed it should wake the screen, but it is not. I can see the notification pop up in the status bar but the main screen area where the lockscreen should be is still black. The screenshot in the Treeherder output is showing exactly what is happening.

To validate the failure I also commented out your new lines in lockscreen.js, rebuilt Gaia and the test passed fine.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 52 • 10 years ago

> I've just finished testing it using a nightly gecko and the Gaia (build just
> using `make`) from when your PR was merged. My OS is linux64.
> 
> I can replicate the failure 100% with your patch. The wake unlock is not
> working with your patch. When the notification is pushed it should wake the
> screen, but it is not. I can see the notification pop up in the status bar
> but the main screen area where the lockscreen should be is still black. The
> screenshot in the Treeherder output is showing exactly what is happening.
> 
> To validate the failure I also commented out your new lines in
> lockscreen.js, rebuilt Gaia and the test passed fine.

Looking at the 2.1 treeherder jobs bug https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1, bug 1065973 seems to have cleared up that lockscreen/notification test failure. Are we happy with this as an outcome, or can I help dig into why - why it broke and why this fixed it?

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 53 • 10 years ago

The run in question: https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g34_v2_1&revision=6582202c75ca - sounds like the change to the lock method on GaiaTest more closely emulates real user/device behavior so I think we're good here.

	Shine(Leave from Mozilla)
	Comment 54 • 9 years ago

This Problem is verified as pass on latest build of Flame 2.2 and Nexus5 2.2 by the STR in comment 2.
Actual result: Device will return to the homescreen when I tap the home button on the card view.
Rate: 0/5
See attachment: Flame2.2_verify_video.3gp

Device information:
Flame v2.2 (319mb): (Pass)
Build ID               20150614002504
Gaia Revision          cfceba16e48ede3defee24be93637a0fa291c494
Gaia Date              2015-06-11 22:10:18
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/2cfd86c2ba1b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150614.040237
Firmware Date          Sun Jun 14 04:02:49 EDT 2015
Bootloader             L1TC000118D0

Nexus5 v2.2: (Pass)
Build ID               20150614002504
Gaia Revision          cfceba16e48ede3defee24be93637a0fa291c494
Gaia Date              2015-06-11 22:10:18
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/2cfd86c2ba1b
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150614.042607
Firmware Date          Sun Jun 14 04:26:25 EDT 2015
Bootloader             HHZ12d