920890 - App occasionally becomes white after several open/close action quickly.

Status: RESOLVED FIXED

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

Description

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

STR:
1. Open any app
2. Press home button

Repeat several times.

Actual:
The app becomes all white.

Still don't know what happens but doesn't look like a visibility issue.

Comment 1

[Jason Smith [:jsmith]]

isn't this related to the problem you mentioned in bug 919048? Same root cause?

Comment 2

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

No, it's the whole app becomes white, but maybe the same root cause. Not a visibility issue because touch event is not passed to the app.

Comment 3

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

I think visibility is correctly set from gaia side. Trying to build gecko to see if frameLoader.visible is correctly set, too.

Comment 4

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

I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: true; src=app://homescreen.gaiamobile.org/index.html#root
I/Gecko   ( 1229): BrowserElementParent.jsm - _childVisibilityChange(true)
I/Gecko   ( 1229): BrowserElementParent.jsm - fireEventFromMsg: visibilitychange, {"visible":true,"msg_name":"visibilitychange"}
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: true; src=app://homescreen.gaiamobile.org/index.html#root
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: true; src=app://communications.gaiamobile.org/dialer/index.html#keyboard-view
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: false; src=app://homescreen.gaiamobile.org/index.html#root
I/Gecko   ( 1229): BrowserElementParent.jsm - _childVisibilityChange(false)
I/Gecko   ( 1229): BrowserElementParent.jsm - fireEventFromMsg: visibilitychange, {"visible":false,"msg_name":"visibilitychange"}
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: false; src=app://homescreen.gaiamobile.org/index.html#root
I/Gecko   ( 1229): BrowserElementParent.jsm - _childVisibilityChange(true)
I/Gecko   ( 1229): BrowserElementParent.jsm - fireEventFromMsg: visibilitychange, {"visible":true,"msg_name":"visibilitychange"}
I/Gecko   ( 1229): BrowserElementParent.jsm - Successful gotDOMRequestResult.
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: false; src=app://communications.gaiamobile.org/dialer/index.html#keyboard-view
I/Gecko   ( 1229): BrowserElementParent.jsm - _childVisibilityChange(false)
I/Gecko   ( 1229): BrowserElementParent.jsm - fireEventFromMsg: visibilitychange, {"visible":false,"msg_name":"visibilitychange"}
I/Gecko   ( 1229): BrowserElementParent.jsm - Successful gotDOMRequestResult.
I/Gecko   ( 1229): BrowserElementParent.jsm - frameLoader visible: false; src=app://communications.gaiamobile.org/dialer/index.html#keyboard-view

Looks like app://communications.gaiamobile.org/dialer/index.html#keyboard-view is set visible = false two times :(

Comment 5

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

OK, windowClosed is executed after openWindow so visibility is set to false.
I think this might be caused by latency of getScreenshot callback :(

Comment 7

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/12488

I remove the getScreenshot call because of race condition and it seems we don't need to catch screenshot anymore. Card view would capture screenshot on demand.

Comment 8

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

TRIAGE: this should be koi+ without doubt. Nearly 100% reproduce rate.
Also this patch seems resolve bug 919048 as well.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Alive Kuo [:alive] from comment #7)
> Created attachment 810945 [details]
> https://github.com/mozilla-b2g/gaia/pull/12488
> 
> I remove the getScreenshot call because of race condition and it seems we
> don't need to catch screenshot anymore. Card view would capture screenshot
> on demand.

This code was not to capture a screenshot but to avoid a flicker effect when the application is transitioned to the card view. The current patch will likely regress that. I wonder if we can't simply check the state of the application in this case before calling setVisible(false)  instead?

Comment 11

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

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9)
> (In reply to Alive Kuo [:alive] from comment #7)
> > Created attachment 810945 [details]
> > https://github.com/mozilla-b2g/gaia/pull/12488
> > 
> > I remove the getScreenshot call because of race condition and it seems we
> > don't need to catch screenshot anymore. Card view would capture screenshot
> > on demand.
> 
> This code was not to capture a screenshot but to avoid a flicker effect when
> the application is transitioned to the card view. The current patch will
> likely regress that. I wonder if we can't simply check the state of the
> application in this case before calling setVisible(false)  instead?

I propose that capture the screenshot on appwillclose event when homebutton is long pressed. Thought?
I don't want to introduce another buggy state check here in window manager...

Comment 12

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

Proposed change:
In cardview's holdhome event listener, request a screenshot from current appWindow by appWindow.getScreent()
and do the following in the callback function.
I know this may slow down the holdhome action but maybe we could reduce the hold timing to avoid that.
I think in the original design we're taking screenshot even we don't need it if we never use cardview and screenshot takes time.

Comment 13

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

Uh, the screenshotting only happens on current displayed app in card view :/
But it's still strange that we capture screenshot each time we close an app.

Trying to capture upon app closing..

Comment 14

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

WIP
https://github.com/alivedise/gaia/compare/bugzilla;920890_master_v2;getscreenshot-delay?expand=1

Vivien, I do think current logic in master couldn't prevent flicking...
Reason:

Holdhome -> Display Homescreen -> CloseWindow -> getScreenshot ->  traverse runningApps -> put cahced screenshot ->  getScreenshot for lastDiaplayedApp --async--> having screenshotURL
                                                       |--async---> having screenshotURL


I think we cannot guarantee first callback is executed before getting the cached screenshot in cardview.

Comment 15

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

Attachment: 920890_v2.patch

V2: Catching screenshot before holdhome handler is executed.

Please note the app may be flicking when openning from cardview but this is not a regression as it also happens in master now. I will try to check this issue later.

I introduce two new public methods to WindowManager but I may remove them later.

Comment 16

[Milan Sreckovic [:milan] (needinfo for best results)]

Is this a duplicate of bug 912961?

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 811087 [details]
920890_v2.patch

As you say fixing flicking when the app comes into view is a different bug but that is a must have imo to make the OS feels nice. Can you file a followup?

Comment 18

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

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #17)
> Comment on attachment 811087 [details]
> 920890_v2.patch
> 
> As you say fixing flicking when the app comes into view is a different bug
> but that is a must have imo to make the OS feels nice. Can you file a
> followup?

Sure, but that seems like a graphic issue now to me.

Comment 19

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

filed bug 922546

re-running travis
https://travis-ci.org/mozilla-b2g/gaia/builds/12000403

Comment 20

[Julien Wajsberg [:julienw]]

from bug 918671 comment 23

Comment 23

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

Fixing travis
https://travis-ci.org/mozilla-b2g/gaia/builds/12034927

Comment 28

[Jason Smith [:jsmith]]

This isn't a smoketest blocker - it doesn't block any of the target smoketests defined right now. This is however, still a bad bug that we should get fixed.

Comment 29

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

Travis is green
https://travis-ci.org/mozilla-b2g/gaia/builds/12034927

Megred during summit!
https://github.com/mozilla-b2g/gaia/commit/c9090021f7d642bae1db73a1093ab3dbb5078642

Comment 30

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 c9090021f7d642bae1db73a1093ab3dbb5078642
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 31

[sarsenyev]

The issue reproduces on the latest Buri 1.2 Aurora MOZ RIL,
During the test run, a white screen appears when opening an email from "Contact" and then opening the "email" app

BuildID 20131010004001
Gaia   51f3c79ea93bb91d3b12e50b49d203a049a94a9b
SourceStamp 3f16dc100b1f
Version 26.0a2US_
Firmware revision: 20130912

Comment 32

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

v1.2
https://github.com/mozilla-b2g/gaia/commit/79abf09f2b5b6440f43cb5ae44ef6c85c0437e8d