Bug 107165

Summary: Traverse the layer tree instead of the render tree in closestScrollableNodeInDocumentIfPossible()
Product: WebKit Reporter: Terry Anderson <tdanderson>
Component: UI EventsAssignee: Terry Anderson <tdanderson>
Status: RESOLVED INVALID    
Severity: Normal CC: rjkroege, simon.fraser, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   

Description Terry Anderson 2013-01-17 13:08:08 PST
Consider Antonio's comment from https://bugs.webkit.org/show_bug.cgi?id=103952#c59:

> Source/WebCore/page/EventHandler.cpp:2613
> +    for (Node* scrollableNode = node; scrollableNode; scrollableNode = scrollableNode->parentNode()) {
> +        if (scrollableNode->isDocumentNode())
> +            break;
> +        RenderObject* renderer = scrollableNode->renderer();
> +        if (renderer && renderer->isBox() && toRenderBox(renderer)->canBeScrolledAndHasScrollableArea())
> +            return scrollableNode;
> +    }

It would be faster (sometimes measurably faster) if instead of traversing the Render tree upwards, you traversed the Layer tree, given that any scrollable renderer will have an associated  RenderLayer.
Comment 1 Terry Anderson 2013-01-17 13:15:49 PST
I should also re-consider Antonio's comment regarding a possibly-redundant boolean member variable:

> Source/WebCore/page/EventHandler.cpp:2633
> +    m_lastHitTestResultOverWidget = result.isOverWidget(); 

I do not think you need this m_lastHitTestResultOverWidget boolean. The Renderer can tell you this as well no? RenderObject::isWidget()
Comment 2 Terry Anderson 2013-03-01 10:39:53 PST
The function closestScrollableNodeCandidate() in EventHandler.cpp is being removed in https://bugs.webkit.org/show_bug.cgi?id=109769 since we're no longer going to be using it, so this issue is no longer valid.