Bug 78739

Created attachment 127229 [details] Patch

This depends on https://bugs.webkit.org/show_bug.cgi?id=78401

Comment on attachment 127229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127229&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:3050 > + page()->settings()->setScrollingCoordinatorEnabled(true); > + page()->scrollingCoordinator()->setClient(scrollingCoordinatorClientImpl()); This is just a nit, but this function is getting to be a mess, and it's sad that you have to repeat these two sets of calls in two places. Can you refactor it any to unify what needs to happen when compositing is successfully turned on or off?

(In reply to comment #3) > (From update of attachment 127229 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127229&action=review > > > Source/WebKit/chromium/src/WebViewImpl.cpp:3050 > > + page()->settings()->setScrollingCoordinatorEnabled(true); > > + page()->scrollingCoordinator()->setClient(scrollingCoordinatorClientImpl()); > > This is just a nit, but this function is getting to be a mess, and it's sad that you have to repeat these two sets of calls in two places. Can you refactor it any to unify what needs to happen when compositing is successfully turned on or off? Yes it's ugly. I'm not completely in love with having to reach down to the ScrollingCoordinator from up here in WebKit, but don't see a better way to interface with what's there today. If we go this route I'll add some helper functions to tighten this up.

Created attachment 127278 [details] Patch

This moves wheel tracking down to be per layer and cleans up a few other things (including setting/clear the client). I think this is looking fairly plausible to land, once 78401 or something like it lands.

Comment on attachment 127278 [details] Patch This looks good to me. I dig the wheel handlers on layers change. Thanks also for cleaning up WebViewImpl a little.

I think I'll do a bit more chopping - ScrollingTreeState isn't really a useful intermediate for us since we buffer the tree via the commit flow, so I think I'll just route the setters on ScrollingCoordinator straight through to the LayerChromium tree. That should get rid of a bit of gobble-dee-gook.

Created attachment 127510 [details] Patch

Even svelter. This pretty much completely ignored the ScrollingTreeState buffering, since we buffer changes through commit. Doesn't fully work yet (esp. transitioning out of compositing mode mode) but getting closer.

Created attachment 127642 [details] Patch

I'm pretty sure everything works here except that when we transition out of compositing mode we need to convince WebCore that the ScrollingCoordinator isn't in charge of that view any more.

Created attachment 127883 [details] Patch

There are some bugs when switching pages but I think this is in pretty good shape. I'll break it up into a few pieces to land (at the very least separating the cross-platform code from the Chromium-specific bits, and possibly separating the properties into their own patches).

Created attachment 127885 [details] Patch

Created attachment 127888 [details] Patch

Comment on attachment 127888 [details] Patch Attachment 127888 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11556051 New failing tests: compositing/checkerboard.html compositing/geometry/limit-layer-bounds-clipping-ancestor.html compositing/geometry/foreground-layer.html compositing/layer-creation/overlap-transformed-layer.html compositing/images/clip-on-directly-composited-image.html compositing/geometry/clip.html compositing/iframes/connect-compositing-iframe2.html compositing/geometry/fixed-position-transform-composited-page-scale.html compositing/layer-creation/scroll-partial-update.html accessibility/aria-describedby-on-input.html compositing/geometry/fixed-position-composited-switch.html compositing/iframes/become-overlapped-iframe.html compositing/geometry/layer-due-to-layer-children-switch.html compositing/geometry/fixed-position-iframe-composited-page-scale.html compositing/iframes/connect-compositing-iframe3.html compositing/iframes/enter-compositing-iframe.html compositing/geometry/limit-layer-bounds-fixed-positioned.html compositing/geometry/layer-due-to-layer-children-deep-switch.html compositing/geometry/ancestor-overflow-change.html compositing/layer-creation/overlap-clipping.html compositing/geometry/fixed-position-composited-page-scale.html compositing/iframes/connect-compositing-iframe.html compositing/iframes/become-composited-nested-iframes.html compositing/layer-creation/overflow-scroll-overlap.html compositing/iframes/connect-compositing-iframe-delayed.html compositing/layer-creation/rotate3d-overlap.html compositing/layer-creation/overlap-child-layer.html compositing/geometry/fixed-position-iframe-composited-page-scale-down.html compositing/iframes/composited-parent-iframe.html compositing/geometry/fixed-position-transform-composited-page-scale-down.html

Cross-platform changes here: https://bugs.webkit.org/show_bug.cgi?id=79126 Wheel event handler here: https://bugs.webkit.org/show_bug.cgi?id=79133 I'll do the remaining properties as separate patches, so closing this bug.