Bug 75028 – Inform the scrolling coordinator when scrollbar layers come and go

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Inform the scrolling coordinator when scrollbar layers come and go

[Anders Carlsson]

Created attachment 120212 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 120212 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120212&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

OOPS

> Source/WebCore/page/ScrollingCoordinator.h:66
> +    // Should be called whenever the horizontal scrollbar layer for the given frame view changes.
> +    void setFrameViewHorizontalScrollbarLayer(FrameView*, const GraphicsLayer* horizontalScrollbarLayer);
> +
> +    // Should be called whenever the horizontal scrollbar layer for the given frame view changes.
> +    void setFrameViewVerticalScrollbarLayer(FrameView*, const GraphicsLayer* verticalScrollbarLayer);

These names are confusing. Are they really setting the FrameView's scrollbar layer, or doing something with the scrollbar layer associated with the FrameView?

[Andreas Kling]

Comment on attachment 120212 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120212&action=review

> Source/WebCore/page/ScrollingCoordinator.h:63
> +    void setFrameViewHorizontalScrollbarLayer(FrameView*, const GraphicsLayer* horizontalScrollbarLayer);

The second argument name is hardly needed here.

> Source/WebCore/page/ScrollingCoordinator.h:66
> +    void setFrameViewVerticalScrollbarLayer(FrameView*, const GraphicsLayer* verticalScrollbarLayer);

Ditto.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1725
> +            if (ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator())
> +                scrollingCoordinator->setFrameViewHorizontalScrollbarLayer(m_renderView->frameView(), 0);

Wrong indentation here.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1750
> +            if (ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator())
> +                scrollingCoordinator->setFrameViewVerticalScrollbarLayer(m_renderView->frameView(), 0);

Ditto.

[Andreas Kling]

Comment on attachment 120212 [details]
Patch

Resetting r? as I didn't see smfr's comments.

[WebKit Review Bot]

Comment on attachment 120212 [details]
Patch

Attachment 120212 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10999066

New failing tests:
compositing/iframes/scrolling-iframe.html
compositing/iframes/iframe-resize.html

[Anders Carlsson]

I renamed all three "layer-setting" functions on ScrollingCoordinator to:

void frameViewScrollLayerDidChange(FrameView*, const GraphicsLayer*);
void frameViewHorizontalScrollbarLayerDidChange(FrameView*, const GraphicsLayer*);
void frameViewVerticalScrollbarLayerDidChange(FrameView*, const GraphicsLayer*);

[Anders Carlsson]

Committed r103435: <http://trac.webkit.org/changeset/103435>