[webkit-reviews] review denied: [Bug 73350] [chromium] Allow scrolling non-root layers in the compositor thread : [Attachment 119285] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 14 15:34:51 PST 2011

James Robinson <jamesr at chromium.org> has denied Sami Kyostila
<skyostil at google.com>'s request for review:
Bug 73350: [chromium] Allow scrolling non-root layers in the compositor thread

Attachment 119285: Patch

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119285&action=review

R- for several layering violations. I think a good place to start for this is
to think about where the interface boundaries lie between the compositor and
the thing that's scrolling, then go from there to figure out the proper
interface boundaries.

> Source/WebCore/platform/graphics/chromium/LayerChromium.h:39
> +#include "Node.h"

no no no, you can't depend on Node in here. This is in WebCore/platform so it
can't depend on anything in WebCore outside of WebCore/platform.

are you sure you want to be talking about Nodes here anyway? there are things
that aren't Nodes that need to scroll. are you perhaps looking for
ScrollableArea? the fact that you are taking an actual reference here really
scares me

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

you can't use code from WebCore/rendering/ here (nor do i think you want to)

More information about the webkit-reviews mailing list