APZ Triggers layers when scrolling a non-scrollable subframe

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: BenWa, Assigned: kats)

Tracking

({perf})

48 Branch
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 wontfix, firefox49 wontfix, firefox50 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(4 attachments, 1 obsolete attachment)

Posted image bug.gif
STR:
1) Load https://www.topcoder.com/community/data-science/data-science-tutorials/power-up-c-with-the-standard-template-library-part-1/
2) Scroll some of the code samples subframes that have disabled scrollbars.
I can repro this.

What's also interesting is that both a ContainerLayer and a PaintedLayer get created, and after a while the PaintedLayer goes away (I'm guessing due to displayport expiry), but the ContainerLayer sticks around until you next scroll the page, at which point the ContainerLayer goes away.
Blocks: apz-desktop
Keywords: perf
Whiteboard: [gfx-noted]
Version: unspecified → 48 Branch
Attachment #8766430 - Attachment mime type: text/plain → text/html
This appears to be happening because those elements are overflow:scroll. That makes the scrollbars visible (even if the element can't scroll), and that makes GetPerceivedScrollingDirections() return both horizontal and vertical directions [1]. This makes the element WantAsyncScroll() and so it gets a displayport and layerized. We should probably stop using GetPerceivedScrollingDirections() and just check if the scroll range is nonzero. We used to do that but it was changed in bug 1126090 [2], incorrectly I think.

[1] http://searchfox.org/mozilla-central/rev/261fe13dcd88cfd2e99e65755e7ca4b7a2e583df/layout/generic/nsGfxScrollFrame.cpp#5804
[2] https://hg.mozilla.org/mozilla-central/rev/5921ef66656c
Blocks: 1126090
I have a patch, pretty simple but I don't think we need to uplift this. I would like to write a test as well.
Assignee: nobody → bugmail.mozilla
https://reviewboard.mozilla.org/r/61336/#review58210

::: gfx/layers/apz/test/mochitest/test_bug1253683.html:32
(Diff revision 1)
> +
> +  // Check initial state
> +  is(container.scrollTop, 0, "Initial scrollY should be 0");
> +  ok(!isLayerized('no_layer'), "initially 'no_layer' should not be layerized");
> +
> +  // Scrolling over outer1 should layerize outer1, but not inner1.

Whoops - this comment is copypasta, I'll update it.
Comment on attachment 8766469 [details]
Bug 1253683 - Don't layerize scrollframes which are overflow:scroll but not actually scrollable.

https://reviewboard.mozilla.org/r/61334/#review58218
Attachment #8766469 - Flags: review?(tnikkel) → review+
Try push (triggered from MozReview) seems green enough, landing.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/36f3e97c79d1
Don't layerize scrollframes which are overflow:scroll but not actually scrollable. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/a5516d8f4ec8
Add a test. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/36f3e97c79d1
https://hg.mozilla.org/mozilla-central/rev/a5516d8f4ec8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.