Summary: | add a general ScrollableArea::scroll function | ||
---|---|---|---|
Product: | WebKit | Reporter: | Robert Kroeger <rjkroege> |
Component: | UI Events | Assignee: | Robert Kroeger <rjkroege> |
Status: | NEW --- | ||
Severity: | Normal | CC: | allan.jensen, andersca, ap, bdakin, jamesr, ljaehun.lim, rjkroege, sam, simon.fraser |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
Description
Robert Kroeger
2012-03-08 07:55:45 PST
Possibly nascent idea: Scrolling a FrameView is different from scrolling a div. Does it need to be? wheel event flow, FrameView. === 5 0x135641f1 WebCore::ScrollableArea::scrollPositionChanged(WebCore::IntPoint const&) 6 0x13564581 WebCore::ScrollableArea::setScrollOffsetFromAnimation(WebCore::IntPoint const&) -- D: common -- 7 0x13556e1a WebCore::ScrollAnimator::notifyPositionChanged() 8 0x1381ced9 WebCore::ScrollAnimatorMac::notifyPositionChanged() 9 0x13556546 WebCore::ScrollAnimator::scroll(WebCore::ScrollbarOrientation, WebCore::ScrollGranularity, float, float) 10 0x1381c795 WebCore::ScrollAnimatorMac::scroll(WebCore::ScrollbarOrientation, WebCore::ScrollGranularity, float, float) 11 0x13556ad6 WebCore::ScrollAnimator::handleWheelEvent(WebCore::PlatformWheelEvent const&) 12 0x1381e299 WebCore::ScrollAnimatorMac::handleWheelEvent(WebCore::PlatformWheelEvent const&) 13 0x13564490 WebCore::ScrollableArea::handleWheelEvent(WebCore::PlatformWheelEvent const&) -- C: found the scrollable area -- 14 0x126b966e WebCore::FrameView::wheelEvent(WebCore::PlatformWheelEvent const&) -- A: start divergent path -- 15 0x12673fbe WebCore::EventHandler::handleWheelEvent(WebCore::PlatformWheelEvent const&) wheel event flow, scrollable div === 1 0x6741e17e WebCore::ScrollableArea::scrollPositionChanged(WebCore::IntPoint const&) 2 0x6741e541 WebCore::ScrollableArea::setScrollOffsetFromAnimation(WebCore::IntPoint const&) -- D: common -- 3 0x67410dca WebCore::ScrollAnimator::notifyPositionChanged() 4 0x676d6e99 WebCore::ScrollAnimatorMac::notifyPositionChanged() 5 0x674104f6 WebCore::ScrollAnimator::scroll(WebCore::ScrollbarOrientation, WebCore::ScrollGranularity, float, float) 6 0x676d6755 WebCore::ScrollAnimatorMac::scroll(WebCore::ScrollbarOrientation, WebCore::ScrollGranularity, float, float) 7 0x6741de87 WebCore::ScrollableArea::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float) -- C: found the scrollable area -- 8 0x65a638cc WebCore::RenderLayer::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float) 9 0x659a6bb3 WebCore::RenderBox::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float, WebCore::Node**) 10 0x659a6c8e WebCore::RenderBox::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float, WebCore::Node**) 11 0x659a6c8e WebCore::RenderBox::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float, WebCore::Node**) 12 0x659a6c8e WebCore::RenderBox::scroll(WebCore::ScrollDirection, WebCore::ScrollGranularity, float, WebCore::Node**) 13 0x6652e36b WebCore::scrollNode(float, WebCore::ScrollGranularity, WebCore::ScrollDirection, WebCore::ScrollDirection, WebCore::Node*, WebCore::Node**) -- B: dom dispatch done -- 14 0x6652e0d0 WebCore::EventHandler::defaultWheelEventHandler(WebCore::Node*, WebCore::WheelEvent*) 15 0x67943c58 WebCore::Node::defaultEventHandler(WebCore::Event*) 16 0x67906b65 WebCore::EventDispatcher::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) 17 0x67904bef WebCore::EventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const 18 0x679fa74b WebCore::WheelEventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const 19 0x67905a35 WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>) 20 0x67943452 WebCore::Node::dispatchWheelEvent(WebCore::PlatformWheelEvent const&) -- A: start divergent path -- 21 0x6652df11 WebCore::EventHandler::handleWheelEvent(WebCore::PlatformWheelEvent const&) Two refactorings seem possible that might improve the code. They can be done in either order and be completed independently. refactoring 1 ---- unify code-path potion C-D by introducing a new single kind of scroll. Something like: ScrollableArea::scrollByVector(FloatPoint, ScrollByVectorFlags) (Strawman name -- this one seems descriptive.) ScrollByVectorFlags would subsume phase, momentum phase, granularity. The caller would be responsible for setting up the sign of the FloatPoint to specify direction. refactoring 2 --- To let scrollable divs get scrolled the same way as FrameViews whether it happens by calling ScrollableArea::handleWheelEvent or by calling ScrollableArea::scrollByVector: divide up the process of A-C in EventHandler::handleWheelEvent into three phases: * preserve the target finding code * perform A-B: dispatch the wheel event into the DOM without the callback into EventHandler. * perform B-C: find the ScrollableArea that's going to handle the scroll by a call from EventHandler. * finally, dispatch a ScrollableArea::scrollByVector (or ::handleWheelEvent) Approach --- * on both paths we pass through ScrollAnimator:: to perform scrolls so we can verify the correctness of the changes by validating the calls to ScrollAnimator::notifyPositionChanged. I would think that this could be exposed to layout tests via window.internals. * modify existing wheel (and gesture) event tests to baseline their current position change handling * in either order perform refactoring 1 (incrementally) and refactoring 2. Comments? In particular: how should this be recast to help with the scrolling thread? Or is this all a bad idea? |