Bug 75028 - Inform the scrolling coordinator when scrollbar layers come and go
Summary: Inform the scrolling coordinator when scrollbar layers come and go
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-21 12:10 PST by Anders Carlsson
Modified: 2011-12-21 13:01 PST (History)
6 users (show)

See Also:

Attachments
Patch (5.42 KB, patch)
2011-12-21 12:10 PST, Anders Carlsson 		kling: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2011-12-21 12:10:38 PST 
Inform the scrolling coordinator when scrollbar layers come and go
Comment 1 Anders Carlsson 2011-12-21 12:10:57 PST 
Created attachment 120212 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-12-21 12:46:56 PST 
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?
Comment 3 Andreas Kling 2011-12-21 12:47:30 PST 
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.
Comment 4 Andreas Kling 2011-12-21 12:48:06 PST 
Comment on attachment 120212 [details]
Patch

Resetting r? as I didn't see smfr's comments.
Comment 5 WebKit Review Bot 2011-12-21 12:51:11 PST 
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
Comment 6 Anders Carlsson 2011-12-21 12:55:33 PST 
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*);
Comment 7 Anders Carlsson 2011-12-21 13:01:52 PST 
Committed r103435: <http://trac.webkit.org/changeset/103435>