Closed Bug 1088157 Opened 5 years ago Closed 5 years ago

[Window Management] The user cannot return home in card view

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S9 (21Nov)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: KTucker, Assigned: sfoster)

References

()

Details

(Whiteboard: [2.1-exploratory-3][systemsfe], ux-most-wanted-nov2014)

Attachments

(5 files, 1 obsolete file)

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
This issue was 100% on my device but then stopped reproducing. Working on solid repro. Other testers have hit this issue on occasion today.
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.
Youtube: http://youtu.be/7ePdIycIAug
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: steps-wanted
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
[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
blocking-b2g: --- → 2.0?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Hi Peipei,
Can you verify this on Woodduck 2.0M and Flame 2.0/2.1? Thanks!
Flags: needinfo?(pcheng)
Attached file adb log for V2.1
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
Flags: needinfo?(pcheng)
[Blocking Requested - why for this release]:

[Traige]per comment#2 propose to nom. to 2.1, as only seen in corner case.
blocking-b2g: 2.0? → 2.1?
Seems like a regression.
blocking-b2g: 2.1? → 2.1+
Assignee: nobody → sfoster
Target Milestone: --- → 2.1 S8 (7Nov)
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?
Flags: needinfo?(gweng)
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.
Flags: needinfo?(gweng)
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
Flags: needinfo?(sfoster)
(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?
Flags: needinfo?(sfoster) → needinfo?(gweng)
If you want to control the LockScreen flag, the modification should be put in LockScreen.
Flags: needinfo?(gweng)
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.
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
Target Milestone: 2.1 S8 (7Nov) → 2.1 S9 (21Nov)
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.
Attachment #8520978 - Flags: review?(timdream)
Attachment #8520978 - Attachment description: github PR: Unlock lockScreen when FTU starts → master/2.2 PR: Unlock lockScreen when FTU starts
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.
Attachment #8520978 - Flags: review?(timdream)
> 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
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!
Attachment #8520978 - Flags: review?(timdream)
Attachment #8520978 - Flags: feedback?(gweng)
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.
(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?
Flags: needinfo?(gweng)
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).
Flags: needinfo?(gweng)
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.
Attached file WIP patch on master
Need to discuss with Tim first.
Blocks: 1098041
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.
Flags: needinfo?(timdream)
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?
Attachment #8522538 - Flags: review?(timdream)
Attachment #8522538 - Flags: feedback?(gweng)
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!
Flags: needinfo?(timdream)
Attachment #8522538 - Flags: review?(timdream) → review+
Comment on attachment 8520978 [details] [review]
master/2.2 PR: Unlock lockScreen when FTU starts

Greg will file a bug for v2.2 patch.
Attachment #8520978 - Flags: review?(timdream)
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
Attachment #8522538 - Flags: feedback?(gweng) → approval-gaia-v2.1?(fabrice)
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.
Attachment #8520978 - Flags: feedback?(gweng)
ping on this 2.1 approval request for Attachment #8522538 [details]
Flags: needinfo?(fabrice)
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
Attachment #8520978 - Attachment is obsolete: true
Can QA test a build with that patch? Thanks!
Flags: needinfo?(fabrice)
Keywords: qawanted
Blocks: 994991
Whiteboard: [2.1-exploratory-3][systemsfe] → [2.1-exploratory-3][systemsfe], ux-most-wanted-nov2014
QA Contact: ddixon
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Contact: ddixon
Duplicate of this bug: 1101137
> 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.
Flags: needinfo?(fabrice)
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?
(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.
Attachment #8522538 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Flags: needinfo?(fabrice)
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)
Keywords: checkin-needed
: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!
Flags: needinfo?(kwierso)
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
(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.
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.
Flags: needinfo?(kwierso)
(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 ?
Flags: needinfo?(kwierso)
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
Flags: needinfo?(sfoster)
Keywords: checkin-needed
(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.
Flags: needinfo?(sfoster)
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.
Flags: needinfo?(zcampbell)
Attached video VIDEO0059[1].mp4
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
(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.
(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.
Flags: needinfo?(zcampbell)
> 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?
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.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.