[webkit-reviews] review denied: [Bug 89216] RenderView layer is marked as fixed position container in the scrolling tree if page scale != 1 : [Attachment 148530] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Sami Kyöstilä
<skyostil at chromium.org>'s request for review:
Bug 89216: RenderView layer is marked as fixed position container in the
scrolling tree if page scale != 1
https://bugs.webkit.org/show_bug.cgi?id=89216

Attachment 148530: Patch
https://bugs.webkit.org/attachment.cgi?id=148530&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=148530&action=review


> Source/WebCore/page/scrolling/chromium/ScrollingCoordinatorChromium.cpp:295
> +String ScrollingCoordinator::scrollingTreeAsText() const
> +{
> +    ASSERT(isMainThread());
> +#if USE(ACCELERATED_COMPOSITING)
> +    if (!m_page || !m_page->mainFrame())
> +	   return String();
> +
> +    RenderView* renderView = m_page->mainFrame()->contentRenderer();
> +    if (!renderView || !renderView->compositor() ||
!renderView->compositor()->rootGraphicsLayer())
> +	   return String();
> +
> +    TextStream textStream;
> +    dumpLayer(textStream, renderView->compositor()->rootGraphicsLayer(), 0);

> +    return textStream.release();
> +#else
> +    return String();
> +#endif
> +}

I'd expect this to walk the ScrollingTree nodes, not to just go to
GraphicsLayers. Not all platforms will necessarily store scrolling tree info
under GraphicsLayers.

I think you should split outs the scrollingTreeAsText changes into a separate
bug.