Closed
Bug 1088157
Opened 10 years ago
Closed 10 years ago
[Window Management] The user cannot return home in card view
Categories
(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 verified, b2g-v2.2 verified)
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
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.
Keywords: regression,
steps-wanted
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.
Reporter | ||
Comment 3•10 years ago
|
||
Youtube: http://youtu.be/7ePdIycIAug
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.2:
--- → affected
Flags: needinfo?(jmitchell)
Keywords: steps-wanted
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
Keywords: regression
[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+]
Updated•10 years ago
|
Flags: needinfo?(jmitchell)
Comment 6•10 years ago
|
||
Hi Peipei, Can you verify this on Woodduck 2.0M and Flame 2.0/2.1? Thanks!
Flags: needinfo?(pcheng)
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
Flags: needinfo?(pcheng)
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.
blocking-b2g: 2.0? → 2.1?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sfoster
Target Milestone: --- → 2.1 S8 (7Nov)
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?
Flags: needinfo?(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.
Flags: needinfo?(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
Flags: needinfo?(sfoster)
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?
Flags: needinfo?(sfoster) → needinfo?(gweng)
Comment 14•10 years ago
|
||
If you want to control the LockScreen flag, the modification should be put in LockScreen.
Flags: needinfo?(gweng)
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.
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
Updated•10 years ago
|
Target Milestone: 2.1 S8 (7Nov) → 2.1 S9 (21Nov)
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.
Attachment #8520978 -
Flags: review?(timdream)
Assignee | ||
Updated•10 years ago
|
Attachment #8520978 -
Attachment description: github PR: Unlock lockScreen when FTU starts → master/2.2 PR: Unlock lockScreen when FTU starts
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.
Attachment #8520978 -
Flags: review?(timdream)
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
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!
Attachment #8520978 -
Flags: review?(timdream)
Attachment #8520978 -
Flags: feedback?(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.
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?
Flags: needinfo?(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).
Flags: needinfo?(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.
Comment 25•10 years ago
|
||
Need to discuss with Tim first.
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.
Flags: needinfo?(timdream)
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?
Attachment #8522538 -
Flags: review?(timdream)
Attachment #8522538 -
Flags: feedback?(gweng)
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!
Flags: needinfo?(timdream)
Attachment #8522538 -
Flags: review?(timdream) → review+
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.
Attachment #8520978 -
Flags: review?(timdream)
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
Attachment #8522538 -
Flags: feedback?(gweng) → approval-gaia-v2.1?(fabrice)
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.
Attachment #8520978 -
Flags: feedback?(gweng)
Assignee | ||
Comment 32•10 years ago
|
||
ping on this 2.1 approval request for Attachment #8522538 [details]
Flags: needinfo?(fabrice)
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
Attachment #8520978 -
Attachment is obsolete: true
Comment 34•10 years ago
|
||
Can QA test a build with that patch? Thanks!
Flags: needinfo?(fabrice)
Keywords: qawanted
Updated•10 years ago
|
Blocks: 994991
Whiteboard: [2.1-exploratory-3][systemsfe] → [2.1-exploratory-3][systemsfe], ux-most-wanted-nov2014
Updated•10 years ago
|
QA Contact: 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
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Contact: ddixon
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.
Flags: needinfo?(fabrice)
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?
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.
Updated•10 years ago
|
Attachment #8522538 -
Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Updated•10 years ago
|
Flags: needinfo?(fabrice)
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)
Keywords: checkin-needed
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!
Flags: needinfo?(kwierso)
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
(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)
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 ?
Flags: needinfo?(kwierso)
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
Flags: needinfo?(sfoster)
Keywords: checkin-needed
Updated•10 years ago
|
Flags: needinfo?(kwierso)
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.
Flags: needinfo?(sfoster)
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.
Flags: needinfo?(zcampbell)
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
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.
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.
Flags: needinfo?(zcampbell)
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?
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.
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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
Updated•9 years ago
|
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.
Description
•