- Revision
- 162985
- Author
- bfulg...@apple.com
- Date
- 2014-01-28 16:59:52 -0800 (Tue, 28 Jan 2014)
Log Message
Improve latching behavior for wheel events
https://bugs.webkit.org/show_bug.cgi?id=127386
<rdar://problem/12176858>
Reviewed by Simon Fraser.
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::clearLatchedNode): Added
(WebCore::ScrollingTree::latchedNode): Added
(WebCore::ScrollingTree::removeDestroyedNodes): Clear latched node if it's being removed.
(WebCore::ScrollingTree::ScrollingTree): Initialize new value used for tracking
scroll latching state.
(WebCore::ScrollingTree::setLatchedNode): Added
(WebCore::ScrollingTree::setOrClearLatchedNode): Added
Set latched node when beginning a swipe event, or clear latched node when scroll/momentum ends.
(WebCore::ScrollingTree::shouldHandleWheelEventSynchronously): Check for an existing
latched node and stay in fast scrolling mode if possible. If the current event should
start a swipe event, clear the current latched node so we can correctly find and assign
the new latch node.
* page/scrolling/ScrollingTree.h:
(WebCore::ScrollingTree::hasLatchedNode): Added
* page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
(WebCore::ScrollingTreeScrollingNodeMac::handleWheelEvent): Determine latching state
based on wheel event state and position of mouse pointer in the document.
* platform/ScrollAnimator.cpp:
(ScrollAnimator::handleWheelEvent): Always treat PlatformWheelEventPhaseMayBegin
as successfully handled so that it does not "bubble back up" to the root of
the scrolling tree.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (162984 => 162985)
--- trunk/Source/WebCore/ChangeLog 2014-01-29 00:50:56 UTC (rev 162984)
+++ trunk/Source/WebCore/ChangeLog 2014-01-29 00:59:52 UTC (rev 162985)
@@ -1,3 +1,34 @@
+2014-01-28 Brent Fulgham <bfulg...@apple.com>
+
+ Improve latching behavior for wheel events
+ https://bugs.webkit.org/show_bug.cgi?id=127386
+ <rdar://problem/12176858>
+
+ Reviewed by Simon Fraser.
+
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::clearLatchedNode): Added
+ (WebCore::ScrollingTree::latchedNode): Added
+ (WebCore::ScrollingTree::removeDestroyedNodes): Clear latched node if it's being removed.
+ (WebCore::ScrollingTree::ScrollingTree): Initialize new value used for tracking
+ scroll latching state.
+ (WebCore::ScrollingTree::setLatchedNode): Added
+ (WebCore::ScrollingTree::setOrClearLatchedNode): Added
+ Set latched node when beginning a swipe event, or clear latched node when scroll/momentum ends.
+ (WebCore::ScrollingTree::shouldHandleWheelEventSynchronously): Check for an existing
+ latched node and stay in fast scrolling mode if possible. If the current event should
+ start a swipe event, clear the current latched node so we can correctly find and assign
+ the new latch node.
+ * page/scrolling/ScrollingTree.h:
+ (WebCore::ScrollingTree::hasLatchedNode): Added
+ * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeScrollingNodeMac::handleWheelEvent): Determine latching state
+ based on wheel event state and position of mouse pointer in the document.
+ * platform/ScrollAnimator.cpp:
+ (ScrollAnimator::handleWheelEvent): Always treat PlatformWheelEventPhaseMayBegin
+ as successfully handled so that it does not "bubble back up" to the root of
+ the scrolling tree.
+
2014-01-23 Myles C. Maxfield <mmaxfi...@apple.com>
ASSERT_WITH_SECURITY_IMPLICATION in WebCore::InlineTextBox::paint
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (162984 => 162985)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2014-01-29 00:50:56 UTC (rev 162984)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2014-01-29 00:59:52 UTC (rev 162985)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -48,6 +48,7 @@
, m_mainFramePinnedToTheBottom(false)
, m_mainFrameIsRubberBanding(false)
, m_scrollPinningBehavior(DoNotPin)
+ , m_latchedNode(0)
, m_scrollingPerformanceLoggingEnabled(false)
, m_isHandlingProgrammaticScroll(false)
{
@@ -57,13 +58,39 @@
{
}
+static bool shouldConsiderLatching(const PlatformWheelEvent& wheelEvent)
+{
+ return wheelEvent.phase() == PlatformWheelEventPhaseBegan
+ || wheelEvent.phase() == PlatformWheelEventPhaseMayBegin;
+}
+
+static bool eventShouldClearLatchedNode(const PlatformWheelEvent& wheelEvent)
+{
+ if (wheelEvent.phase() == PlatformWheelEventPhaseCancelled)
+ return true;
+
+ if (wheelEvent.phase() == PlatformWheelEventPhaseNone && wheelEvent.momentumPhase() == PlatformWheelEventPhaseEnded)
+ return true;
+
+ return false;
+}
+
bool ScrollingTree::shouldHandleWheelEventSynchronously(const PlatformWheelEvent& wheelEvent)
{
+ // This method is invoked by the event handling thread
MutexLocker lock(m_mutex);
if (m_hasWheelEventHandlers)
return true;
+ bool shouldSetLatch = shouldConsiderLatching(wheelEvent);
+
+ if (hasLatchedNode() && !shouldSetLatch)
+ return false;
+
+ if (shouldSetLatch)
+ m_latchedNode = 0;
+
if (!m_nonFastScrollableRegion.isEmpty()) {
// FIXME: This is not correct for non-default scroll origins.
IntPoint position = wheelEvent.position();
@@ -74,6 +101,14 @@
return false;
}
+void ScrollingTree::setOrClearLatchedNode(const PlatformWheelEvent& wheelEvent, ScrollingNodeID nodeID)
+{
+ if (shouldConsiderLatching(wheelEvent))
+ setLatchedNode(nodeID);
+ else if (eventShouldClearLatchedNode(wheelEvent))
+ clearLatchedNode();
+}
+
void ScrollingTree::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
{
if (m_rootNode)
@@ -174,14 +209,18 @@
void ScrollingTree::removeDestroyedNodes(const ScrollingStateTree& stateTree)
{
- const Vector<ScrollingNodeID>& removedNodes = stateTree.removedNodes();
- size_t size = removedNodes.size();
- for (size_t i = 0; i < size; ++i) {
- ScrollingTreeNode* node = m_nodeMap.take(removedNodes[i]);
+ for (const auto& removedNode : stateTree.removedNodes()) {
+ ScrollingTreeNode* node = m_nodeMap.take(removedNode);
+ if (!node)
+ continue;
+
// Never destroy the root node. There will be a new root node in the state tree, and we will
// associate it with our existing root node in updateTreeFromStateNode().
- if (node && node->parent())
+ if (node->parent())
m_rootNode->removeChild(node);
+
+ if (node->scrollingNodeID() == m_latchedNode)
+ clearLatchedNode();
}
}
@@ -315,6 +354,24 @@
return m_scrollingPerformanceLoggingEnabled;
}
+ScrollingNodeID ScrollingTree::latchedNode()
+{
+ MutexLocker locker(m_mutex);
+ return m_latchedNode;
+}
+
+void ScrollingTree::setLatchedNode(ScrollingNodeID node)
+{
+ MutexLocker locker(m_mutex);
+ m_latchedNode = node;
+}
+
+void ScrollingTree::clearLatchedNode()
+{
+ MutexLocker locker(m_mutex);
+ m_latchedNode = 0;
+}
+
} // namespace WebCore
#endif // ENABLE(ASYNC_SCROLLING)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (162984 => 162985)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2014-01-29 00:50:56 UTC (rev 162984)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2014-01-29 00:59:52 UTC (rev 162985)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -99,6 +99,13 @@
ScrollingTreeScrollingNode* rootNode() const { return m_rootNode.get(); }
+ ScrollingNodeID latchedNode();
+ void setLatchedNode(ScrollingNodeID);
+ void clearLatchedNode();
+
+ bool hasLatchedNode() const { return m_latchedNode; }
+ void setOrClearLatchedNode(const PlatformWheelEvent&, ScrollingNodeID);
+
protected:
void setMainFrameScrollPosition(IntPoint);
virtual void handleWheelEvent(const PlatformWheelEvent&);
@@ -132,6 +139,7 @@
bool m_mainFramePinnedToTheBottom;
bool m_mainFrameIsRubberBanding;
ScrollPinningBehavior m_scrollPinningBehavior;
+ ScrollingNodeID m_latchedNode;
bool m_scrollingPerformanceLoggingEnabled;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm (162984 => 162985)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2014-01-29 00:50:56 UTC (rev 162984)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2014-01-29 00:59:52 UTC (rev 162985)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -134,6 +134,7 @@
return;
m_scrollElasticityController.handleWheelEvent(wheelEvent);
+ scrollingTree().setOrClearLatchedNode(wheelEvent, scrollingNodeID());
scrollingTree().handleWheelEventPhase(wheelEvent.phase());
}
Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (162984 => 162985)
--- trunk/Source/WebCore/platform/ScrollAnimator.cpp 2014-01-29 00:50:56 UTC (rev 162984)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp 2014-01-29 00:59:52 UTC (rev 162985)
@@ -81,6 +81,16 @@
bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
{
+#if PLATFORM(MAC)
+ // Events in the PlatformWheelEventPhaseMayBegin phase have no deltas, and therefore never passes through the scroll handling logic below.
+ // This causes us to return with an 'unhandled' return state, even though this event was successfully processed.
+ //
+ // We receive at least one PlatformWheelEventPhaseMayBegin when starting main-thread scrolling (see FrameView::wheelEvent), which can
+ // fool the scrolling thread into attempting to handle the scroll, unless we treat the event as handled here.
+ if (e.phase() == PlatformWheelEventPhaseMayBegin)
+ return true;
+#endif
+
Scrollbar* horizontalScrollbar = m_scrollableArea->horizontalScrollbar();
Scrollbar* verticalScrollbar = m_scrollableArea->verticalScrollbar();
