[Webkit-unassigned] [Bug 73350] [chromium] Allow scrolling non-root layers in the compositor thread

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73350





--- Comment #26 from James Robinson <jamesr at chromium.org>  2011-12-19 11:37:57 PST ---
(From update of attachment 119683)
View in context: https://bugs.webkit.org/attachment.cgi?id=119683&action=review

High level design looks good to me. Still going through the details, but here's some initial easy-to-resolve feedback.

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:699
> +        m_scrollableArea->scroll(scrollDelta.width() > 0 ? ScrollRight : ScrollLeft, ScrollByPixel, abs(scrollDelta.width()));
> +    if (scrollDelta.height())
> +        m_scrollableArea->scroll(scrollDelta.height() > 0 ? ScrollDown : ScrollUp, ScrollByPixel, abs(scrollDelta.height()));

I'm pretty sure you need to use the ..WithoutAnimation() variants here or WebKit will attempt to apply animation here, which doesn't make sense for scrolls initiated from the impl thread (which we've presumably already animated in the appropriate way)

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:32
> +#include "RenderBox.h"

you don't actually need this include any more, do you? it's a layering violation

> Source/WebKit/chromium/src/ChromeClientImpl.h:36
> +#include "GraphicsLayer.h"

can GraphicsLayer be forward declared?

> Source/WebKit/chromium/src/ChromeClientImpl.h:44
> +class Node;

don't think you need this

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.