1155785 - [Windows Management][Browser] Edge gesturing between browser pages will show black box flicker and rendering issues

Status: VERIFIED FIXED

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

Description

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

Attachment: Edge gesure Flickering

Description:
If the user transitions between webpages that are open using edge gestures, the browser app will show graphical issues while rendering. This includes flickering, low resolution images, and the appearence of black boxes

Repro Steps:
1) Update a Flame to 20150417010203
2) Open Browser app> navigate to any webpage
3) Open a second browser window
4) Edge gesture between the two pages


Actual:
The webpage will flicker, and black boxes will appear on half of the screen


Expected:
User is able to smoothly transition between browser pages

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150417010203
Gaia: 3cd0a9facce26c2acc7be3755a17131a6358e33f
Gecko: 51e3cb11a258
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Notes: There seems to be performance issues accross edge gestures in multiple areas, but browser was the only area I was able to get certain graphical issues like black boxes while loading and jittery transitions


Repro frequency: 5/10
See attached: Logcat, Video - https://youtu.be/rM7HXMHdGaA

Comment 1

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

This issue DOES occur on Flame 2.2

The webpage will flicker, and black boxes will appear on half of the screen

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150417002504
Gaia: d50b8a3919a7b4d8d289f150d3b9bed704ebafa9
Gecko: 5ebf32030512
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

=========================================================================================

This issue does NOT occur on Flame 2.1

The transition between pages is slow, but the page does not show any graphical flicker issues

Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat)(Full Flash)
Build ID: 20150415001211
Gaia: bbe983b4e8bebfec26b3726b79568a22d667223c
Gecko: 3e3cbe35bce3
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

[Peter Bylenga [:PBylenga]]

Visual performance regression of a core feature.

Requesting a window.

Comment 3

[Gregor Wagner [:gwagner]]

Putting on Milans radar

Comment 4

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

Bug 1111142 may have been the cuase for this issue.  Not sure though.  I cannot narrow this down further as I do not have access to the fx team inbound.

Central Regression Window:

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150103032041
Gaia: 698e6e8a098cc060b26cd6f25171633c4c7e739d
Gecko: 48e2bab81030
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150103172440
Gaia: 698e6e8a098cc060b26cd6f25171633c4c7e739d
Gecko: b478b0c48bcc
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 698e6e8a098cc060b26cd6f25171633c4c7e739d
Gecko: b478b0c48bcc

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 698e6e8a098cc060b26cd6f25171633c4c7e739d
Gecko: 48e2bab81030

Gaia Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=48e2bab81030&tochange=b478b0c48bcc

Comment 5

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

Margaret, can you take a look at this please? This could have possibly been caused by the work done for bug 1111142 but we are really not sure.

Comment 6

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

Removing needinfo on me, but I'll watch the comments in the bug, and CC-d Kats.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

It seems unlikely to be bug 1111142 considering the regression window in comment 4 has both the landing and backout of that bug. None of the other changes in the window seem like likely culprits either. The most likely I would say is bug 1115802 but even that is a stretch. Are the STR 100% reliable? I see a 50% repro rate, but when it happens is it unambiguous, or might be it be confused with other similar issues (like black boxes vs just slow perf)?

Comment 8

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

This bug seemed to be more of a black boxes bug. There is slow performance across the board for edge gesturing right now, but the black boxes only seem to appear in the browser app.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Just by playing with this a little bit and looking at logcat it looks like the window with browser content doesn't OMTA on the edge gesture transition, whereas the window with about:home does get OMTA. So during the edge gesture transition the OMTA'd window moves more smoothly and the content window lags behind, providing the visual effect of black "boxes" (which is really just a wide gap between the two windows).

Comment 10

[Kartikaya Gupta (email:[e-mail])]

The above is supported by the following logcat output:

Performance warning: Async animation disabled because frame size (320, 1029) is bigger than the viewport (360, 641) or the visual rectangle (320, 1029) is larger than the max allowable value (17895698) [div with id 'AppWindow_5']

In this case AppWindow_5 is the browser content window I have open. Not sure why the frame size is 1029 pixels tall though.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

So it looks like the frame's scr-overflow is 52734 app units tall, which is 1029 CSS pixels. Later on it even grows further to 68320 app units, or around 1139 CSS pixels. This is too large to do OMTA on. I tried poking around in WebIDE to see why the scrollable overflow is so big but I don't know enough about how the scrollable overflow is computed to actually answer that question.

It's probably faster at this point to redo the regression window and hope it points to the culprit. The other alternative is to get somebody from layout to look into this and they're all swamped. Another option is to have gaia folks look at why the appWindow for browser content somehow seems to be different (taller) than non-browser-content appWindows, that might also point to the explanation.

Flagging regressionwindow-wanted to get a new regression window because it definitely doesn't look like anything in the window from comment 4 would have caused this.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Frame dumps

Also for future reference here are the frame dumps. I modified the perf warning output to include the frame address, so you can find it in the frame trees. The tree at the top of the file is the one that triggers the "performance warning" output, as it's the first frame tree that has frame aa52d198 with a transform set on it, but at that point it's CSS 1014 pixels tall. The second frame tree is taken from later, after stuff has stabilized - in that tree you can see frame aa52d198 has an even larger scr-overflow. (Note that this dump is from a different run than the above comments so the numbers are a bit different, but it illustrates the same thing).

Comment 13

[Gregor Wagner [:gwagner]]

Jet, can someone from layout help out here to fix the root issue?
Etienne, can you try a workaround as suggested in comment 11?

Comment 14

[Kartikaya Gupta (email:[e-mail])]

FWIW I believe the root issue is actually in Gaia, not in layout. I just don't understand this particular bit of layout code but I wouldn't assume it's wrong.

Comment 15

[Gregor Wagner [:gwagner]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #14)
> FWIW I believe the root issue is actually in Gaia, not in layout. I just
> don't understand this particular bit of layout code but I wouldn't assume
> it's wrong.

Thanks Kats. Assigning to Etienne for debugging then.

Comment 16

[GitHub Autolander Bot]

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

Comment 17

[Etienne Segonzac (:etienne)]

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

Kevin, Alive: I just can't remember why the AppWindows weren't already |overflow:hidden|.

Maybe something to do with orientation change support that's not the case anymore? Or from before the app_statusbar?

Anyway, tested orientation, scrollgrab, inline activities, lockscreen and everything looks good.

Please let me know if you remember/see anything that'll break.

Comment 18

[Kevin Grandon :kgrandon]

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

I did a quick check, and it looks like we used to have this, but it was removed here: https://github.com/mozilla-b2g/gaia/commit/bc6eb8d37373d2a877630c3337f10efa398004ed#diff-722a99771d790c2ec314f2033b232fe9L51

It seems that you reviewed it, so I think I'm going to just forward my review to Vivien if that's ok and let you guys figure out what is best :)

Comment 19

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

I found this regression window based upon the reporter's video. The large black gap that happens when edge gesturing between windows in browser. 

B2G Inbound - Jellybean

Last Working
Environmental Variables:
Device: Flame 2.1(Shallow Flash)(JB)(319mb)
BuildID: 20140829145759
Gaia: f530a13b8b2e17a9cdd6e44e453122ef44ef77fc
Gecko: 56c602487233
Version: 34.0a1 (2.1)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken
Device: Flame 2.1(Shallow Flash)(JB)(319mb)
BuildID: 20140829154813
Gaia: b40c2ce979735faa17adcdd98c6e9b721b8fd948
Gecko: 02eef143aaa9
Version: 34.0a1 (2.1)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Last Working Gaia First Broken Gecko: Issue does NOT reproduce
Gaia: f530a13b8b2e17a9cdd6e44e453122ef44ef77fc
Gecko: 02eef143aaa9

First Broken Gaia Last Working Gecko: Issue DOES reproduce
Gaia: b40c2ce979735faa17adcdd98c6e9b721b8fd948
Gecko: 56c602487233

https://github.com/mozilla-b2g/gaia/compare/f530a13b8b2e17a9cdd6e44e453122ef44ef77fc...b40c2ce979735faa17adcdd98c6e9b721b8fd948

This looks to have been caused by Bug 1054466 

------------------------------------------------

Dale, can you take a look at this please? This looks to have been caused by the work done for Bug 1054466.

Comment 20

[Dale Harvey (:daleharvey)]

I would be very surprised if this was caused by https://github.com/mozilla-b2g/gaia/commit/838213d40dcba2e37b9e397c162aa5e44e9e64e7, but will take a look. 

Ettiene do you want me to take this bug or happy with fixing it, looks like there is a patch in so seems everything is ok here?

Comment 21

[Etienne Segonzac (:etienne)]

(In reply to Dale Harvey (:daleharvey) from comment #20)
> I would be very surprised if this was caused by
> https://github.com/mozilla-b2g/gaia/commit/
> 838213d40dcba2e37b9e397c162aa5e44e9e64e7, but will take a look. 

Not that surprising.
I think the issue is that, after opening the menu, the appWindow frame is too big to be OMTA.
Probably because we're drawing the menu "below" the iframe once it's been dismissed.

> 
> Ettiene do you want me to take this bug or happy with fixing it, looks like
> there is a patch in so seems everything is ok here?

Yes I'll check again with Vivien but my patch sounds safer since it fixes the root issue. If we can't do it that way I'll ni you again to work around in the menu. cheers!

Comment 22

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

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

\O/

Comment 23

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

(In reply to Etienne Segonzac (:etienne) from comment #17)
> Comment on attachment 8597206 [details] [review]
> [gaia] etiennesegonzac:bug-1155785 > mozilla-b2g:master
> 
> Kevin, Alive: I just can't remember why the AppWindows weren't already
> |overflow:hidden|.

So do I :/ But maybe Vivien knows the magic.

Comment 24

[Etienne Segonzac (:etienne)]

(In reply to Kevin Grandon :kgrandon from comment #18)
> Comment on attachment 8597206 [details] [review]
> [gaia] etiennesegonzac:bug-1155785 > mozilla-b2g:master
> 
> I did a quick check, and it looks like we used to have this, but it was
> removed here:
> https://github.com/mozilla-b2g/gaia/commit/
> bc6eb8d37373d2a877630c3337f10efa398004ed#diff-
> 722a99771d790c2ec314f2033b232fe9L51
> 
> It seems that you reviewed it, so I think I'm going to just forward my
> review to Vivien if that's ok and let you guys figure out what is best :)

Thanks that's really helpful.
Running some tests to make sure we're not regressing on the overpaint front.

(Already checked with Vivien but we couldn't remember either.)

Comment 25

[Etienne Segonzac (:etienne)]

Overpaint is fine but we're hitting bug 1059154 which is failing an integration test. Still pondering what to do.

Comment 26

[Etienne Segonzac (:etienne)]

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

Updated the patch, here's my thinking:
* having an overflow:hidden on the appWindows is really important, otherwise we will have issues like this one every time we do polish work and add some transform:translate on any dialog/select/etc.. inside the app element
* the overflow:visible on appWindow was only here to work around a bug where setting a transform will initially cause a reflow, I tried mitigating this issue and updated the test. in real life these reflows will probably get collapsed with the statusbar reflow anyway
* I double-checked the overpaint numbers and we're stable

Since the patch changed a bit I'm flagging Alive for another round of review :)

Comment 27

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

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

Looks fine

Comment 28

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/29711

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.

Comment 29

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/29711

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.

Comment 30

[Kevin Grandon :kgrandon]

Tree is reopened now. Let's try again.

Comment 31

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/5b91f36d3ff8c62f2d2426824965c7e71cfb45e3

Comment 32

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

Kevin, could you also uplift to 2.2?

Comment 33

[Kevin Grandon :kgrandon]

(In reply to Hermes Cheng[:hermescheng] from comment #32)
> Kevin, could you also uplift to 2.2?

I guess that request was for etienne?

Comment 34

[Etienne Segonzac (:etienne)]

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

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): dialogs with nice transitions
[User impact] if declined: slugish performance
[Testing completed]: edge gestures + browser windows tests
[Risk to taking this patch] (and alternatives if risky): low-ish (we had some uncertainties during the making of this patch but it's tiny and fixes the root cause of the issue)
[String changes made]: none

Comment 35

[Ryan VanderMeulen [:RyanVM]]

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

Comment 36

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

keep unverified until bug 1160191 has been resolved.

Comment 37

[Shally Li（leave from Mozilla）]

This bug has been verified as pass on latest build of Flame v2.2&3.0 and Nexus 5 v2.2&3.0 by the STR in Comment 0.

Actual results: User smoothly transfers between browser pages after adding a new window.
See attachment: verified_v2.2&3.0.mp4
Reproduce rate: 0/10

-------------------------------------------------------------------------------
Device: Flame v2.2 build(Pass)
Build ID               20150514162500
Gaia Revision          8d478e4df36ace173b3e505b568f1a5077fed7c3
Gaia Date              2015-05-14 17:30:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/666b327287d5
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150514.201419
Firmware Date          Thu May 14 20:14:30 EDT 2015
Bootloader             L1TC000118D0

Device: Flame v3.0 build(Pass)
Build ID               20150514160201
Gaia Revision          8897e1810aa6426ca483269af76ce2bfd2029d25
Gaia Date              2015-05-14 18:49:06
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/59b6d856160c
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150514.192647
Firmware Date          Thu May 14 19:26:58 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 v2.2 build (Pass)
Build ID               20150514162500
Gaia Revision          8d478e4df36ace173b3e505b568f1a5077fed7c3
Gaia Date              2015-05-14 17:30:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/666b327287d5
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150514.214406
Firmware Date          Thu May 14 21:44:23 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 v3.0 build (Pass)
Build ID               20150514010203
Gaia Revision          338f66e6a96491d2f5854b188c6b141ceb690d97
Gaia Date              2015-05-13 14:08:45
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1fab94ad196c
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150514.044222
Firmware Date          Thu May 14 04:42:38 EDT 2015
Bootloader             HHZ12f

Comment 38

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.2&3.0.mp4

Comment 39

[Shally Li（leave from Mozilla）]

Note:
The depending on bug 1164153 is "New" status and is not fixed now.

Comment 40

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

It seems this patch results in bug 1164153. Let's wait for developer.