1080205 - Iframe appears to checkerboard all the time - see Marketplace App

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Botond Ballo [:botond]]

STR:
  1. Load a page with a scrollable <iframe> in it, such as 
     http://people.mozilla.org/~kgupta/tmp/iframe.html
  2. Scroll the <iframe> contents in any direction.

Expected results:
  The <iframe> has a display port, and as you scroll it
  there is no checkerboarding unless you scroll very fast.

Actual results:
  There is checkerboarding even when you scroll slowly.
  Areas brought into view asynchronously remain white
  until a repaint.

I'm pretty sure this is a regression, as it's pretty noticeable.

Comment 1

[Botond Ballo [:botond]]

The behaviour seems to be specific to iframes. A comparable page with a scrollable div instead of an iframe (e.g. http://people.mozilla.org/~bballo/notransdiv.html) does not exhibit the constant checkerboarding behaviour.

Comment 2

[Botond Ballo [:botond]]

It would be nice to have some branch checks done to verify that this is a regression, and if so, get a regression window.

Comment 3

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

Branch checks and blocker status needed before we can request a regression window (as per https://wiki.mozilla.org/B2G/QA/Triage#Summary_of_Keywords_and_Flags)

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

We also see this on non-OOP mozbrowser instances too.  (Specifically, the email app's gmail support for OAuth2 uses a mozbrowser.)

Comment 5

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

Not sure what 'checkerboarding' behavior looks like exactly so I made a video demonstrating what I *think* is reproducing the issue. When I scroll on the provided iframe page, the screen needs a little bit of time to load before displaying the content, hence showing white spaces for a very short while before the new contents get displayed.

Video URL:
http://youtu.be/vxVZqpIvbZ8

Please confirm whether this is reproducing the bug and then I can branch check before attempting a window.

Tested on:
Device: Flame 2.1 (shallow flash, 319MB mem)
BuildID: 20141013062451
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: 47cc92f8ab46
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 6

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

Yeah, the blank white space is the checkerboarding.

Comment 7

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

Attachment: Test page with iframe

Please use the attached test page when reproducing this issue. The page at the URL in comment 0 is subject to change at any time (in fact I'm going to delete it so people stop assuming it's going to be there permanently).

Comment 8

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

Thanks for answering, setting tracking flag so this bug can appear on our QA-Wanted query.

Comment 9

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

[Blocking Requested - why for this release]:
This appears to be a regression, and makes subframe scrolling much worse.

Comment 10

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

This issue occurs on Flame 2.2 master.

Observed behavior: White spaces appear for a very short while when scrolling on the test iFrame page on comment 7. The issue seems much more noticeable when scrolling horizontally (left-right scrolling) as opposed to scrolling vertically (up-down scrolling).

Device: Flame 2.2 Master (shallow flash, 319MB mem)
BuildID: 20141014064254
Gaia: de254419f3553f48187d003ee8e38034b429f069
Gecko: eb1b8ecbefde
Version: 36.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

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

On Flame 2.0, all white spaces are replaced with blurry texts when scrolling. I assume this is the expected behavior? If not please correct my tracking flags.

Device: Flame 2.0 (shallow flash, 319MB mem)
BuildID: 20141013043753
Gaia: 21fc294d6c9b78997142153fc32c3175b4835a89
Gecko: 93530962cca3
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 11

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

(In reply to Pi Wei Cheng [:piwei] from comment #10)
> On Flame 2.0, all white spaces are replaced with blurry texts when
> scrolling. I assume this is the expected behavior? If not please correct my
> tracking flags.

That's more or less expected, yes. You still shouldn't run into the blurry text right when you start scrolling; it should only appear if you scroll fast or suddenly change direction or something. But yes, on 2.1 and 2.2 going seeing blank white spaces instead of blurry text is part of the problem.

Comment 12

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

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> You still shouldn't run into the blurry text right when you start scrolling;
> it should only appear if you scroll fast or suddenly change direction or something.

I recall seeing the blurry text as frequent/noticeable as the white space problem in 2.1 onwards; I didn't have to scroll fast to see the issue. I could get a video of it if you'd like. Assigning myself as QA Contact to find the regression window.

Comment 13

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

mozilla-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20140923202630
Gaia: ff6dbb006e4e60edba697639aa2a2a199b07069b
Gecko: 15082adbb409
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20140923210052
Gaia: ff6dbb006e4e60edba697639aa2a2a199b07069b
Gecko: 1bdf7f5d1a6d
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Gaia's are the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=15082adbb409&tochange=1bdf7f5d1a6d

Caused by Bug 1062100.

Comment 14

[Timothy Nikkel (:tnikkel)]

I haven't looked into this at all but it's possible the issue could be the one described in bug 1068961, comment 8 (which was only partially fixed for color layers in that bug).

Comment 15

[Joshua Mitchell (Inactive)]

Possibly broken by Bug 1062100 - can you take a look Robert?

Comment 16

[Andrew Overholt [:overholt]]

Quite user-visible regression = 2.1+.

Comment 17

[Jet Villegas (inactive)]

Assigning to :roc per comment 13. Please send this back to me if I should reassign.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The display lists and layer tree look OK. The relevant part of the generated layer tree is

I/Gecko   ( 5240):     ClientContainerLayer (0xb2d63840) [clip=(x=15, y=276, w=456, h=1500)] [visible=< (x=15, y=276, w=456, h=1500); >] [componentAlpha] [metrics0={ cb=(x=15.000000, y=276.000000, w=456.000000, h=1500.000000) sr=(x=0.000000, y=0.000000, w=1994.000000, h=1994.000000) s=(136.667,26.6667) dp=(x=-136.666672, y=-26.666666, w=1706.666626, h=1994.000000) cdp=(x=-136.666672, y=-26.666666, w=682.666687, h=1365.333374) color=rgba(0, 0, 0, 0) scrollId=4 scrollParent=3 z=1.500 }]
I/Gecko   ( 5240):       ClientTiledPaintedLayer (0xb2d7bc00) [clip=(x=15, y=276, w=456, h=1500)] [transform=[ 1 0; 0 1; -190 236; ]] [bounds=(x=0, y=0, w=2991, h=2991)] [visible=< (x=0, y=0, w=2560, h=2991); >] [componentAlpha] [valid=< (x=0, y=0, w=1024, h=2048); >]

The relevant part of the display list is

I/Gecko   ( 5240): SubDocument p=0xb395b168 f=0xb10e12b8(Viewport(-1)) bounds(600,11040,18240,60000) visible(0,0,19200,72800) componentAlpha(-7600,9440,106492,114580) clip(600,11040,18240,60000)  layer=
0xb2d63840
I/Gecko   ( 5240):   CanvasBackgroundColor p=0xb12770b8 f=0xb10e19c0(Canvas(html)(-1)) bounds(-7600,9440,119640,119640) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  uniform (rgba 0,0,0,0) layer=0xb2d7bc00
I/Gecko   ( 5240):   TableBorderBackground p=0xb12770f0 f=0xb10ef6a8(Table(table)(1)) bounds(-7600,9440,119640,119640) visible(23120,81120,10240,10240) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277120 f=0xb0d4d1d0(TableCell(td)(1)) bounds(-7600,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277190 f=0xb0d4d9a0(TableCell(td)(3)) bounds(-1620,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277200 f=0xb0d4db28(TableCell(td)(5)) bounds(4360,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277270 f=0xb0d4dcb0(TableCell(td)(7)) bounds(10340,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb12772e0 f=0xb0d4de38(TableCell(td)(9)) bounds(16320,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277350 f=0xb0d4dfc0(TableCell(td)(11)) bounds(22300,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb12773c0 f=0xb0d4e148(TableCell(td)(13)) bounds(28280,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
I/Gecko   ( 5240):   Border p=0xb1277450 f=0xb0d4e2d0(TableCell(td)(15)) bounds(34260,9440,5980,5980) visible(0,0,0,0) componentAlpha(0,0,0,0) clip()  layer=0xb2d7bc00
... etc ...

Note that the clip()s for all the items are empty so we should be drawing them to the layer OK and it's got nice big visible and valid regions (I'm not sure why valid is smaller than visible though).

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

A sequence of composites while async scrolling:

I/Gecko   ( 6455):             ContainerLayerComposite (0xb04c1800) [shadow-clip=(x=15, y=276, w=456, h=1500)] [shadow-transform=[ 1 0; 0 1; -245.586 0; ]] [shadow-visible=< (x=15, y=276, w=456, h=1500); >] [clip=(x=15, y=276, w=456, h=1500)] [visible=< (x=15, y=276, w=456, h=1500); >] [componentAlpha] [metrics0={ cb=(x=15.000000, y=276.000000, w=456.000000, h=1500.000000) sr=(x=0.000000, y=0.000000, w=1994.000000, h=1994.000000) s=(318.667,2.66667) dp=(x=-318.666656, y=-2.666667, w=1877.333374, h=1994.000000) cdp=(x=-148.000000, y=-2.666667, w=682.666687, h=1365.333374) color=rgba(0, 0, 0, 0) scrollId=4 scrollParent=3 z=1.500 }]
I/Gecko   ( 6455):               PaintedLayerComposite (0xb04c1c00) [shadow-clip=(x=15, y=276, w=456, h=1500)] [shadow-transform=[ 1 0; 0 1; -463 272; ]] [shadow-visible=< (x=0, y=0, w=2816, h=2991); >] [clip=(x=15, y=276, w=456, h=1500)] [transform=[ 1 0; 0 1; -463 272; ]] [bounds=(x=0, y=0, w=2991, h=2991)] [visible=< (x=0, y=0, w=2816, h=2991); >] [componentAlpha] [valid=< (x=256, y=0, w=1024, h=2048); >]

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

I/Gecko   ( 6455):             ContainerLayerComposite (0xb04c1800) [shadow-clip=(x=15, y=276, w=456, h=1500)] [shadow-transform=[ 1 0; 0 1; -250.586 0; ]] [shadow-visible=< (x=15, y=276, w=456, h=1500); >] [clip=(x=15, y=276, w=456, h=1500)] [visible=< (x=15, y=276, w=456, h=1500); >] [componentAlpha] [metrics0={ cb=(x=15.000000, y=276.000000, w=456.000000, h=1500.000000) sr=(x=0.000000, y=0.000000, w=1994.000000, h=1994.000000) s=(318.667,2.66667) dp=(x=-318.666656, y=-2.666667, w=1877.333374, h=1994.000000) cdp=(x=-148.000000, y=-2.666667, w=682.666687, h=1365.333374) color=rgba(0, 0, 0, 0) scrollId=4 scrollParent=3 z=1.500 }]
I/Gecko   ( 6455):               PaintedLayerComposite (0xb04c1c00) [shadow-clip=(x=15, y=276, w=456, h=1500)] [shadow-transform=[ 1 0; 0 1; -463 272; ]] [shadow-visible=< (x=0, y=0, w=2816, h=2991); >] [clip=(x=15, y=276, w=456, h=1500)] [transform=[ 1 0; 0 1; -463 272; ]] [bounds=(x=0, y=0, w=2991, h=2991)] [visible=< (x=0, y=0, w=2816, h=2991); >] [componentAlpha] [valid=< (x=256, y=0, w=1024, h=2048); >]

So I guess the PaintedLayerComposite and the ClientTiledPaintedLayer shouldn't have a clip at all, since the scroll transform is being applied to the parent.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The best fix here is to stop putting framemetrics on the subdocument container layer.

Comment 21

[Botond Ballo [:botond]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #20)
> The best fix here is to stop putting framemetrics on the subdocument
> container layer.

We're looking to do just that in bug 1076192.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That looks like too much work for a regression fix. I'll try for a simpler fix here.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1: Don't add clip rects to async-scrolled layers for root scrollframes, since the FrameMetrics to scroll those layers will be set on a ContainerLayer parent

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 2: Add test and test infrastructure

Comment 25

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8510800 [details] [diff] [review]
Part 1: Don't add clip rects to async-scrolled layers for root scrollframes, since the FrameMetrics to scroll those layers will be set on a ContainerLayer parent

We should check if we are root and ignoring view port scrolling. Also make sure to get rid of the debug stuff before landing.

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1 v2

Comment 27

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8512406 [details] [diff] [review]
Part 1 v2

Even if we don't hit the "aBuilder->GetIgnoreScrollFrame() == mOuter || IsIgnoringViewportClipping()" case we still only clip "if (!(mIsRoot && mOuter->PresContext()->PresShell()->GetIsViewportOverridden()))", so we should only set mAddClipRectToLayer to true if that is true.

Do we need to initialize mAddClipRectToLayer in the constructor?

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If !mIsRoot then aBuilder->GetIgnoreScrollFrame() == mOuter will be false since we only ever ignore root scroll frames.

Can you tell me why you think the patch is actually wrong? I know it changes behavior.

Comment 29

[Timothy Nikkel (:tnikkel)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #28)
> If !mIsRoot then aBuilder->GetIgnoreScrollFrame() == mOuter will be false
> since we only ever ignore root scroll frames.
> 
> Can you tell me why you think the patch is actually wrong? I know it changes
> behavior.

If we move to containerless scrolling for root scroll frames then we would start clipping wrongly for root scroll frames that have their viewport overridden. We don't want to clip those to the scrollport because that does not correspond to what's drawn on screen.

So this will work now, but we do want to move to containerless scrolling for root scroll frames.

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1 v3

Fair enough!

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1 v4

Comment 32

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8512427 [details] [diff] [review]
Part 1 v4

Thanks.

Initialize mAddClipRectToLayer in the constructor?

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Timothy Nikkel (:tn) from comment #32)
> Initialize mAddClipRectToLayer in the constructor?

It shouldn't be necessary and I think if we leave it alone then valgrind might find more bugs.

Unfortunately the try build with these patches has failures, on B2G only:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=042c3f4a340d
Intermittent failures in layout/reftests/backgrounds, and a consistent failure in http://10.0.2.2:8888/tests/layout/reftests/image/image-orientation-background.html?90&flip. In all the failures we simply fail to paint an image. My efforts to run B2G tests locally have failed so far.

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We're green with patches from bug 1083635: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c45a80b4dd60

Comment 35

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6051b6c18f23
https://hg.mozilla.org/integration/mozilla-inbound/rev/03416040f8b4

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6051b6c18f23
https://hg.mozilla.org/mozilla-central/rev/03416040f8b4

Comment 38

[sarsenyev]

Verified the issue is fixed on 2.2 Flame
No white space appears when scrolling the attached page

Device: Flame 2.2 Master KK
BuildID: 20141105160209
Gaia: 7918024c737c4570cacd784f267e28737ae05dea
Gecko: 2114ef80f6ae
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Adding "verifyme" for 2.1

Comment 39

[Asa Dotzler [:asa]]

Updating summary so that recent reports of Marketplace checkerboard are more likely to find this.

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Please request b2g34 approval on this patch when you get a chance.

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8512427 [details] [diff] [review]
Part 1 v4

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 1062100
User impact if declined: nasty checkerboarding while scrolling iframes
Testing completed: automated tests run, been on trunk for a bit
Risk to taking this patch (and alternatives if risky): fairly low risk
String or UUID changes made by this patch: none

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/50df5cf8dbf7
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/64f37dc542ae

Comment 43

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1.

Result: Checkerboarding effect is not observed while scrolling iframes.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141112001201
Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko: b9849b3c6aaa
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0