Bug 48988 – Delegate scrolling via a separate method

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kenneth Rohde Christiansen]

Instead of piggy-backing on the HostWindow::scroll method, we use a special delegateScrollRequested method. This means that we avoid calling additional scrollbar related code before the call to HostWindow::scroll, and that we do not need to do special casing in ChromeClient::scroll to figure out if it was a delegated scroll or not, thus imposing overhead on the normal case. This will also make the webkit2 impl more clear.

[Kenneth Rohde Christiansen]

Created attachment 72926 [details]
Patch

[Early Warning System Bot]

Attachment 72926 [details] did not build on qt:
Build output: http://queues.webkit.org/results/5099010

[Kenneth Rohde Christiansen]

Created attachment 72933 [details]
Patch2

[Adam Roben (:aroben)]

Comment on attachment 72933 [details]
Patch2

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

> WebCore/platform/ScrollView.cpp:340
> +    IntPoint newScrollPosition = scrollPoint.shrunkTo(maximumScrollPosition());
> +    newScrollPosition.clampNegativeToZero();
> +
> +#if ENABLE(TILED_BACKING_STORE)
>      if (delegatesScrolling()) {
> -        scrollContents(IntSize(scrollPoint.x(), scrollPoint.y()));
> +        hostWindow()->delegatedScrollRequested(IntSize(scrollPoint.x(), scrollPoint.y()));
>          return;
>      }
> -
> -    IntPoint newScrollPosition = scrollPoint.shrunkTo(maximumScrollPosition());
> -    newScrollPosition.clampNegativeToZero();
> +#endif

Looks like you removed the call to scrollContents entirely, even in the non-tiled case.

[Kenneth Rohde Christiansen]

(In reply to comment #4)
> (From update of attachment 72933 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72933&action=review

> Looks like you removed the call to scrollContents entirely, even in the non-tiled case.

No, that is called later indirectly bye setting the scrollbars.

[Dave Hyatt]

Comment on attachment 72933 [details]
Patch2

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

Looks good.

> WebCore/ChangeLog:9
> +        scrolling to the view. This is only used in conjunging with tiling,

"conjunction" is what you meant to say here I think.

[Andreas Kling]

Committed r71352: <http://trac.webkit.org/changeset/71352>