[Webkit-unassigned] [Bug 73345] [chromium] Split scrollRootLayer() into scroll{Begin, By, End}()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73345


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #117183|review?                     |review-
               Flag|                            |




--- Comment #6 from James Robinson <jamesr at chromium.org>  2011-11-30 16:40:17 PST ---
(From update of attachment 117183)
View in context: https://bugs.webkit.org/attachment.cgi?id=117183&action=review

Comments inline. I like the shape of this, just have some questions about the details.

> Source/WebCore/platform/graphics/chromium/cc/CCInputHandler.h:55
> +    // Scroll the layer selected with scrollBegin().
> +    virtual void scrollBy(const IntSize&) = 0;

If the layer being scrolled runs out of scrollable area, then in at least some UIs we start scrolling the nearest ancestor that can accept the scrolls (even if it isn't the same scroll). I believe this API is compatible with this interaction but this comment seems to indicate that all scrolls will have to be on the same layer. Can you update the comment for this case?

What happens if we run out of scrollable area on the initially targeted layer and we need to start scrolling some ancestor that we can't scroll on the thread? It seems like in that case we need to start punting the scrolls to the main thread - or is that considered a separate scrollBegin/scrollBy/scrollEnd interaction at that point?

> Source/WebKit/chromium/src/WebCompositorImpl.cpp:122
> +        if (m_inputHandlerClient->scrollBegin(IntPoint(wheelEvent.x, wheelEvent.y))) {

I think this logic is subtly wrong in the case where the main thread has no wheel event handlers and the root content is not scrollable - in that case we should drop the wheel on the floor, not send it to the main thread.

I think you need to distinguish between the "input handler cannot handle the scroll" and "input handler knows that nobody cares about this scroll" cases to handle this, either by having separate APIs to query whether the handler can do a scroll vs whether there's anything to scroll at all or by having multiple return values from this call.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.