Bug 178396 - Composited frames incorrectly get requestAnimationFrame throttled
Summary: Composited frames incorrectly get requestAnimationFrame throttled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-17 10:27 PDT by Mihai Parparita
Modified: 2017-12-05 15:30 PST (History)
6 users (show)

See Also:


Attachments
Patch (6.73 KB, patch)
2017-12-05 14:15 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2017-10-17 10:27:14 PDT
I'm observing iframes having their requestAnimationFrame callbacks getting incorrectly throttled because WebKit thinks that they are outside of the viewport, when they are in fact visible. To reproduce (in either Safari 11.0 or more recent WebKit builds - I tried r223536):

1. Go to http://persistent.info/webkit/test-cases/frame-raf-throttling/container.html. There is an "overflow: auto" scrollable div (gray background, 300 pixels wide) in which there is a composited iframe (yellow, 2000 pixels wide). The iframe uses requestAnimationFrame (for how it just reports the interval between successive calls, to make throttling more apparent)
2. Scroll the container sideways
3. Once the scroll offset goes past the window width (there's a "Scroll to Window Width" button to do this) then the requestAnimationFrame cadence drops down every 10 seconds, corresponding to the aggressiveThrottlingAnimationInterval value in ScriptedAnimationController (which is triggered by ThrottlingReason::OutsideViewport)

What I've traced is that when the area gets scrolled, we end up in FrameView::viewportContentsChanged, which uses applyRecursivelyWithVisibleRect() to get the visible rect to determine if the iframe is visible. That in turn relies on windowClipRect(), and that returns a 0x0 sized rectangle.

Digging into windowClipRect, we end up in windowClipRectForFrameOwner, which since the frame has an enclosing layer uses that layer's childrenClipRect. The return value of that is (-<scroll offset>, y - <window width> x <window height>). Thus if the scroll offset is greater than the window width, when we intersect the rectangle with the window's clip rectangle.

The reason why childrenClipRect's return value appears to refleect the window's dimensions is because it passes renderer().view().unscaledDocumentRect() as the paintDirtyRect parameter to calculateRects (which forms the basis of the return value).

I'm out of my depth here as far as my WebKit knowledge, so someone with more familiarity will need to attempt a fix. Though the manifestation of this bug is that requestAnimationFrame gets throttled incorrectly, since the problem is at a lower level (as far as determining its visible rectangle) there may be other side effects too.

Given that unscaledDocumentRect is used as the initial value, a workaround is to add an element to the main frame document that is as wide as the iframe. This is what the "Add Filler" button does, and once that element is present (even if the main frame document uses "overflow: hidden") then throttling does not occur.
Comment 1 Radar WebKit Bug Importer 2017-10-17 11:12:10 PDT
<rdar://problem/35033205>
Comment 2 Simon Fraser (smfr) 2017-12-04 21:05:27 PST
I think the issue is that FrameView::windowClipRectForFrameOwner() uses enclosingLayer->childrenClipRect() which is sensitive to compositing.
Comment 3 Simon Fraser (smfr) 2017-12-04 21:25:01 PST
RenderLayer::childrenClipRect() takes the document rect (renderer().view().unscaledDocumentRect()) and calls localToAbsoluteQuad() on it, which maps it through the scroll offset, so it can end up with some large negative offset.

This seems wrong; we should probably map LayoutRect::infiniteRect() through and then clip to the document rect.
Comment 4 Simon Fraser (smfr) 2017-12-04 21:26:28 PST
I also note that RenderWidget::setWidgetGeometry() uses childrenClipRect() and may have a similar bug.
Comment 5 Simon Fraser (smfr) 2017-12-05 14:15:18 PST
Created attachment 328497 [details]
Patch
Comment 6 WebKit Commit Bot 2017-12-05 15:30:51 PST
Comment on attachment 328497 [details]
Patch

Clearing flags on attachment: 328497

Committed r225554: <https://trac.webkit.org/changeset/225554>
Comment 7 WebKit Commit Bot 2017-12-05 15:30:53 PST
All reviewed patches have been landed.  Closing bug.