1118854 - [Window Management][Homescreen] If the user taps home while opening task manager, the homescreen will appear blank

Status: VERIFIED FIXED

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

Description

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: No Icons Homescreen Logcat

Description:
If the user opens task manager, and while its opening double taps on the home button, the homescreen will appear blank. The user can still interact with any buttons that are active on the phone, but cannot view any of the buttons. This can be recovered by going back into card view, unless the phone is locked first


Repro Steps:
1) Update a Flame to 20150107010216
2) Open any app
3) Long press home button
4) As soon as the task manager starts to open, double tap home button


Actual:
Homescreen appears blank


Expected:
User is taken to the top of the homescreen without issue

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150107010216
Gaia: 69ac77cfa938fae2763ac426a80ca6e5feb6ad25
Gecko: 33781a3a5201
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Notes:This bug appears to be a timing issue.

Repro frequency: 10/10 100%
See attached: Logcat, Video - https://www.youtube.com/watch?v=K1yfxG_384k&edit=vd

Comment 1

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This issue does NOT reproduce on Flame 2.1

The user is brought back to the homescreen without issue.

Device: Flame 2.1 (319mb)(Kitkat)(Full Flash)
BuildID: 20150107001244
Gaia: b04a8cb7b2482e0a44e6702b48c42283a00b5b1e
Gecko: 99cea2c818f6
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 34.0 (2.1)
Firmware: V18D
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

[Peter Bylenga [:PBylenga]]

severe functional issue.

Requesting a window.

Comment 3

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

A quick idea to fix this issue is ignore home button event in appWindowManager when taskManager is transitioning. (It seems strange for now but if we move task manager into appWindowManager as sub module later then it makes more sense.)

Comment 4

[Pi Wei Cheng [:piwei] (inactive)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20140910055313
Gaia: 2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a
Gecko: 15ae5d04c3de
Version: 35.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20140910061313
Gaia: b3698e7e93b5945b5aa5f857dd01932894f57a85
Gecko: b80b5054f126
Version: 35.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: b3698e7e93b5945b5aa5f857dd01932894f57a85
Gecko: 15ae5d04c3de

First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: 2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a
Gecko: b80b5054f126

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/2f71bd60ff9e50c9821ef96d6a67fbd51f721f2a...b3698e7e93b5945b5aa5f857dd01932894f57a85

Caused by Bug 1061324.

Comment 5

[KTucker [:KTucker][Inactive 3/4/2016]]

Looks like this was caused by the uplifts for bug 1061324. Etienne or Vivien, can you please take a look at this issue?

Comment 6

[Gregor Wagner [:gwagner]]

Broken feature.

Comment 7

[Sam Foster [:sfoster] (he/him)]

The bug title and steps disagree - is this triggered when long-tapping home button while an app opens, or tapping home while the task manager is transitioning to show. I'm not able to reproduce either on my Flame/master

Comment 8

[Sam Foster [:sfoster] (he/him)]

Ah I just got it to reproduce. Yeah it looks like overlapping events - we're still getting into the showing task manager state while simultaneously hiding it and opening the homescreen. Alive's suggestion from Comment #3 looks worth a shot. Cancelling need-infos for etienne and vivien, I don't we need anything further from them. I can only reproduce this very sporadically - 1/10 at best, if there are better STR that would be a big help

Comment 9

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

It seems that you were able to see this issue per comment 8. Clearing my need info for now. If more information is needed, NI me again.

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: task_manager_test_repo_bug.js

js-marionette test case that can reproduce the issue;  run the test twice in a row )

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Turns out that even the automation has a low rate of repro.  I think if we remove some of the waits, we might get closer to hitting the bug more frequently.... this would mean to modify some of the other supporting files for the automation...

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

ie ./lib/task_manager

Comment 13

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

Alive, I'm not sure if the TaskManager has already an event to notice the system that the card view is in a transition state.

Besides, I have a doubt here. This approach, using |transitionend| event, waits until the transition is finished, then, the user can use the home button. However, I notice that this happens when all the transitions ends, which means that the user has to wait until that moment which, in my opinion is not good for the user experience. So, please tell me if we can reach our goal but using another approach.

Thanks!

Comment 14

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

Comment on attachment 8551147 [details] [review]
Proposed patch

LGTM but Unit test needed. Also I don't think we need to maintain taskManager state in appWindowManager so using taskManager.isTransitioning is not bad, but better if use Service.query('TaskManager.isTransitioning'), see service.js for more info.

I won't worry about user experience because holdhome then quick press home is not a normal user behavior to me. A better way might be "catch" the home event during the task manager is transitioning but implement this might take too much work so let's just ignore now.

Flag sam to take a look as well.

Comment 15

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8551147 [details] [review]
Proposed patch

Added a note in the PR about the listener removal, cancelling r? for now. Are we sure transitionend is the right moment? It seems to me that in-between cardviewbeforeshow and cardviewshown we are in an indeterminate state and that may be where the issue lies, not so much the animation itself?

Comment 16

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

(In reply to Sam Foster [:sfoster] from comment #15)
> Comment on attachment 8551147 [details] [review]
> Proposed patch
> 
> Added a note in the PR about the listener removal, cancelling r? for now.
> Are we sure transitionend is the right moment? It seems to me that
> in-between cardviewbeforeshow and cardviewshown we are in an indeterminate
> state and that may be where the issue lies, not so much the animation itself?

I don't really look into what happens between cardviewbeforeshow and cardviewshown. But if you think transition is not enough, raise your hand.

Comment 17

[Manuel Casas Barrado [:mancas]]

After a deep research, I've noticed that the issue occurs when the card view is inactive, and the current app is closed. This cause that the screen element has a wrong class, that hides all the content in the homescreen.

This is the explanation:

When the card view is opened, and the user press home button, the designed behaviour is that the last app is brought to the foreground. Pressing again home button, the app is closed and the homescreen appears. In that moment, the |appclosed| event is caught by the taskmanager and then is handled in [1] which set the |cards-view| class, to the screen element, causing the issue. As you can see, this is only reproducible when you perform the steps quickly because the normal behaviour when the card view is opened is that the current app sends the |app closed| event which will be handle in [1] and then the handler is removed.

Please take a look at the new patch and let me know what you think about this new approach

Thanks! =)

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/task_manager.js#L162-L165

Comment 18

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

Nice catch! I think Sam will review this properly.
Again good work!

Comment 19

[Sam Foster [:sfoster] (he/him)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #18)
> Nice catch! I think Sam will review this properly.
> Again good work!

Yes, thanks for the deep dive and getting to the bottom of this. I am looking at the patch. Some parts of task_manager.js are relatively new to me and I'm trying to understand the flow. I'll do a detailed review but at first glance it looks like we may still have a race condition from that setTimeout in exitToApp that calls hide()?

More soon.

Comment 20

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8551147 [details] [review]
Proposed patch

Patch looks good. A couple of notes on the PR. I think the unit test isn't adding much value - its too contrived. I'm also trying to get a failing marionette test that we can demonstrate the fix on but I've got nothing solid yet. I'll attach the WIP. If it does what we expect and passes, that's a win. Would be better if it failed without the patch though! I've also been trying to reproduce again on device with gaia master (without the patch) to verify and am unable(!) I think we should land regardless as the patch fixes a known logic error.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Attachment: marionette test: task_manager_bug_1118854.js

Doesn't repro the bug for me currently. The timing between the 'home' events is tricksy but too little and the event is ignored and its times out having opened the app. Or it opens the app then opens the homescreen as expected. 

I guess its useful for catching regressions, but I can't repro manually on device either so at least its consistent :/

Comment 22

[Sam Foster [:sfoster] (he/him)]

:mancas can you address the review comments and land this (or leave checkin-needed on it) If you can make a failing test that repros the issue and passes with your patch that would be great. If the bug no longer repros lets put qawanted on it and figure out what fixed it.

Comment 23

[Manuel Casas Barrado [:mancas]]

Hey Sam, I've been trying to make the test but I can't reproduce the issue. As you said it's really difficult to reproduce it but we should land the patch in order to solve the logic issue.

Thanks

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/eaaab511cc231ce39df95a521498e12c14d21041

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Please request Gaia v2.2 approval on this when you get a chance.

Comment 26

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8551147 [details] [review]
Proposed patch

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Task Manager
[User impact] if declined: Its possible to end up on an inactive (empty) homescreen
[Testing completed]: On device, Gaia-Trry
[Risk to taking this patch] (and alternatives if risky): Low risk, the patch closes up an infrequent race condition
[String changes made]: None

Comment 27

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/c287765ae8be3d86271db969034083cecdf95e74

Comment 28

[Yeojin Chung [:YeojinC]]

This issue still reproduces on Flame 2.2 and Master.

Result: A blank screen appears when fast double-tapping home button during transition to task manager.

Device: Flame 2.2 (319mb, full flash)
Build ID: 20150202002507
Gaia: d6141fa3208f224393269e17c39d1fe53b7e6a05
Gecko: be206fa2fb60
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame Master (319mb, full flash)
Build ID: 20150202010229
Gaia: 740c7c2330d08eb9298597e0455f53d4619bbc1a
Gecko: 940118b1adcd
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 29

[Sam Foster [:sfoster] (he/him)]

(In reply to Yeojin Chung [:YeojinC] from comment #28)
> This issue still reproduces on Flame 2.2 and Master.

Would it be possible to attach your STR and a video? I'm testing with a Flame @ 319MB and I'm unable to reproduce this. I *do* see the homescreen background briefly before its grid gets populated with icons, but that is expected.

Comment 30

[Yeojin Chung [:YeojinC]]

My repro step was same as comment 0. The key point to reproduce this bug is double-tapping before going in to the task manager. 

STR:
1) Open any app.
2) Long-press home button to enter task manager.
3) Before the transition starts, double-tap the home button.

And here's my video: http://youtu.be/WCDXfMGKUdA

Hope this is helpful. :)

Comment 31

[Hermes Cheng (inactive after July 27, 2015)]

Yeojin, is it able to be reproduced with current build now?

Comment 32

[Yeojin Chung [:YeojinC]]

Yes, I was still able to reproduce this issue on today's nightly Flame Master and 2.2.

Device: Flame Master (KK, 319mb, full flash)
Build ID: 20150226010233
Gaia: 7894b929f1b0394f3c997f72a6482bc7813e758d
Gecko: dd6353d61993
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.

Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150226002503
Gaia: bf24aa57fa7760260ab05d1f53242c8d8ae59e83
Gecko: 363123044e61
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 33

[Hermes Cheng (inactive after July 27, 2015)]

According to comment 32, reopen this bug.

Comment 34

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1118854 > mozilla-b2g:master

Comment 35

[Etienne Segonzac (:etienne)]

Comment on attachment 8574080 [details] [review]
[gaia] etiennesegonzac:bug-1118854 > mozilla-b2g:master

I was able to reproduce the issue, here's a follow up patch.

Comment 36

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8574080 [details] [review]
[gaia] etiennesegonzac:bug-1118854 > mozilla-b2g:master

Patch looks fine, but somewhere recently (today?) we lost the transition to show the task manager, which means it shows instantly and is impossible to reproduce the bug it fixes.

Comment 37

[Sam Foster [:sfoster] (he/him)]

> Patch looks fine, but somewhere recently (today?) we lost the transition to
> show the task manager, which means it shows instantly and is impossible to
> reproduce the bug it fixes.

..from the homescreen. Works fine from another app.

Comment 38

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 39

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#UKhNgU-6TwWbSwoNr2HGBQ

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 40

[Etienne Segonzac (:etienne)]

Looks like I'm confusing autolander. Anyway:
- green try https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=808e0a307560
- landed on master: https://github.com/mozilla-b2g/gaia/commit/fbb57e3a25d561983c58a8b26ccaaaf94c869829

Comment 41

[Hermes Cheng (inactive after July 27, 2015)]

Please also uplift to v2.2, thanks.

Comment 42

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8574080 [details] [review]
[gaia] etiennesegonzac:bug-1118854 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Task Manager (follow up to this bug - bug 1118854)
[User impact] if declined: Its possible to end up on an inactive (empty) homescreen 
[Testing completed]: On device, Gaia-Try
[Risk to taking this patch] (and alternatives if risky): Low risk, the patch closes up an infrequent race condition
[String changes made]: None

Comment 43

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed in the latest Nightly Flame 3.0 build.  

Actual Results: The homescreen correctly has icons.

Environmental Variables:
Device: Flame 3.0 KK (319 MB) (Full Flash)
BuildID: 20150310010227
Gaia: 2fb09da0cb9cefad9c6e40f57533fafda6d12557
Gecko: 6686aacf006f
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

On 2.2 this issue still occurs, but I believe that the fixed flag was from the original fix and not this new updated fix.

Environmental Variables:
Device: Flame 2.2 KK (319 MB) (Full Flash)
BuildID: 20150310002536
Gaia: 166491b92278dc9e648f8d49ab02d9ca00d74421
Gecko: 1cda026f8996
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 44

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a0ebe8118aa5200384f3ffca76825a01d2b6c1fb

For future reference, approvals are going to go unnoticed if the status is already set to fixed. I only stumbled upon this one by accident while investigating a conflict on another uplift.

Comment 45

[Hermes Cheng (inactive after July 27, 2015)]

Sorry, my fault. I did not notice that flag.
Jayme, could you please re-verify 2.2? Thanks!

Comment 46

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame KK 2.2 build.

Actual Results: The homescreen will correctly show when following the steps.

Environmental Variables:
Device: Flame 2.2 KK (319MB) (Full Flash)
BuildID: 20150401002624
Gaia: 8b3086ad3963f1707e2bee9094baccafffe161c4
Gecko: 20b67213a047
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0