Closed Bug 1080205 Opened 5 years ago Closed 5 years ago

Iframe appears to checkerboard all the time - see Marketplace App

Categories

(Core :: Layout, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla36
blocking-b2g 2.1+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: botond, Assigned: roc)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

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.
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.
It would be nice to have some branch checks done to verify that this is a regression, and if so, get a regression window.
Keywords: qawanted
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)
We also see this on non-OOP mozbrowser instances too.  (Specifically, the email app's gmail support for OAuth2 uses a mozbrowser.)
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
Flags: needinfo?(botond)
Yeah, the blank white space is the checkerboarding.
Flags: needinfo?(botond)
Attached file 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).
Thanks for answering, setting tracking flag so this bug can appear on our QA-Wanted query.
[Blocking Requested - why for this release]:
This appears to be a regression, and makes subframe scrolling much worse.
blocking-b2g: --- → 2.1?
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
(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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
QA Contact: pcheng
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.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
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).
Possibly broken by Bug 1062100 - can you take a look Robert?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(roc)
Component: Panning and Zooming → Layout
Quite user-visible regression = 2.1+.
blocking-b2g: 2.1? → 2.1+
Assigning to :roc per comment 13. Please send this back to me if I should reassign.
Assignee: nobody → roc
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).
Flags: needinfo?(roc)
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.
The best fix here is to stop putting framemetrics on the subdocument container layer.
(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.
That looks like too much work for a regression fix. I'll try for a simpler fix here.
Attachment #8510806 - Flags: review?(tnikkel) → review+
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.
Attachment #8510800 - Flags: review?(tnikkel)
Attached patch Part 1 v2 (obsolete) — Splinter Review
Attachment #8510800 - Attachment is obsolete: true
Attachment #8512406 - Flags: review?(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?
Attachment #8512406 - Flags: review?(tnikkel)
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.
Flags: needinfo?(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.
Flags: needinfo?(tnikkel)
Attached patch Part 1 v3 (obsolete) — Splinter Review
Fair enough!
Attachment #8512422 - Flags: review?(tnikkel)
Attachment #8512422 - Attachment is patch: true
Attachment #8512422 - Attachment mime type: message/rfc822 → text/plain
Attached patch Part 1 v4Splinter Review
Attachment #8512406 - Attachment is obsolete: true
Attachment #8512422 - Attachment is obsolete: true
Attachment #8512422 - Flags: review?(tnikkel)
Attachment #8512427 - Flags: review?(tnikkel)
Comment on attachment 8512427 [details] [diff] [review]
Part 1 v4

Thanks.

Initialize mAddClipRectToLayer in the constructor?
Attachment #8512427 - Flags: review?(tnikkel) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/6051b6c18f23
https://hg.mozilla.org/mozilla-central/rev/03416040f8b4
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Updating summary so that recent reports of Marketplace checkerboard are more likely to find this.
Summary: Iframe appears to checkerboard all the time → Iframe appears to checkerboard all the time - see Marketplace App
Please request b2g34 approval on this patch when you get a chance.
Flags: needinfo?(roc)
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
Flags: needinfo?(roc)
Attachment #8512427 - Flags: approval-mozilla-b2g34?
Attachment #8512427 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.