Closed Bug 1138799 Opened 5 years ago Closed 2 years ago

[LockScreen] Prevent create LockScreenWindow when there is FTU

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: gweng, Unassigned)

References

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1136156 +++

This is the follow-up bug of Bug 1136156. I would keep this bug until the performance issue got solved.

The patch would sequentialize the path of launching LockScreen to make it more reasonable.
Assignee: nobody → gweng
Whiteboard: [fromAutomation]
Summary: Lockscreen shows incorrect time format after it's modified for the first time → [LockScreen] Prevent racing of LockScreen & FTU in LockScreenWindowManager
Summary: [LockScreen] Prevent racing of LockScreen & FTU in LockScreenWindowManager → [LockScreen] Prevent create LockScreenWindow when there is FTU
Depends on: 1138802
No longer depends on: 1136156
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Tim: from my device it works well with following paths:

1. FTU -> FTU closed -> powerbutton *2 -> Lock screen
2. FTU -> FTU closed -> Settings app to set it -> powerbutton *2 -> Lock screen
3. No FTU -> Lock screen

The position of connection info text also works well. So I set the flag.
Attachment #8571771 - Flags: review?(timdream)
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Discussed offline. Please fix the code pattern quirk and resolve the start-up state control issues.
Attachment #8571771 - Flags: review?(timdream)
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Tim: I've updated the patch. Please review it again, thanks!
Attachment #8571771 - Flags: review?(timdream)
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Maybe it's better to have another discussion before reviewing.
Attachment #8571771 - Flags: review?(timdream)
Tim: I've found System would new the wallpaper manager in bootstrap.js:

199   window.wallpaperManager = new window.WallpaperManager();

Since there is a 'window' prefix so we grepped nothing. And thus my patch is work well even I removed the same line in lockscreen_bootstrap.js.
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Tim: I've re-checked the WallpaperManager issue and replied as above. The patch passed all tests except one notification Gij task that I'm debugging and it looks like a minor issue only need to modify the test itself, so I set review.
Attachment #8571771 - Flags: review?(timdream)
Hmm... it passed the test perfectly on my local console. I would rebase the patch and push it again.
Comment on attachment 8571771 [details] [review]
[gaia] snowmantw:bug1138799 > mozilla-b2g:master

Sorry for the really late r+ ...
Attachment #8571771 - Flags: review?(timdream) → review+
http://docs.taskcluster.net/tools/task-graph-inspector/#2P6HGMRzTkqj9n9D7HmXbw

The pull request failed to pass integration tests. It could not be landed, please try again.
http://docs.taskcluster.net/tools/task-graph-inspector/#Txo7UpFXT0Won5KuyZGSbA

The pull request failed to pass integration tests. It could not be landed, please try again.
Since on Gaia-Try it passed all test but autolander still detected it's broken, I would land it manually after the next try, if it continues as that.
http://docs.taskcluster.net/tools/task-graph-inspector/#4M5MDJMeSXGAlY81T3MJgg

The pull request failed to pass integration tests. It could not be landed, please try again.
Hmm it passed on Treeherder again:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=a8ad792c6147d37499d62e8b9a0441763ef3bc04

And it also failed on TaskCluster as before (with new failures):

http://docs.taskcluster.net/tools/task-graph-inspector/#4M5MDJMeSXGAlY81T3MJgg

So I would land it manually, and to see if they're really blocking failures (if so, I would back out it).
master: https://github.com/mozilla-b2g/gaia/commit/8b899668a5ef6effcd8b2bd84eea5c1450a3f207

Let's see if it's really broken.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
It looks like lockscreen do not gets enabled at bootime for me, with current Gaia ... Johan confirmed that it was good on a Flame build from yesterday
Flags: needinfo?(gweng)
Okay thanks for the info. I would check it and back out the patch and fix it again. By the way, I wonder why tests still seem good on TreeHerder, even it's with such issue...
Flags: needinfo?(gweng)
And of course I don't reproduce on my Flame :(
OK, I've checked it. It would only appear if I press the power button to make it lock, but not appear after rebooting without FTU. I would re-patch this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8593174 [details] [review]
[gaia] snowmantw:bug1138799-rev2 > mozilla-b2g:master

Tim: I've found it looks that some rebasing mistakes hit the patch so it lacks one checking function to determine if the FTU was opened once, which in my recollection it was in the patch. I now add it back and set review again.

By the way, our CI reported it works well (from TreeHerder) even without such part... More tests should be added, of course. However I prefer to do that after LockScreen part of System 2 is done, or we would both be choked by the continuous test failures.
Attachment #8593174 - Flags: review?(timdream)
Comment on attachment 8593174 [details] [review]
[gaia] snowmantw:bug1138799-rev2 > mozilla-b2g:master

Well since Alive is working on system launcher, so we would need a completely different patch to solve the issue.
Attachment #8593174 - Flags: review?(timdream)
Assignee: gweng → nobody
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 5 years ago2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.