Diff
Modified: trunk/LayoutTests/ChangeLog (268543 => 268544)
--- trunk/LayoutTests/ChangeLog 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/LayoutTests/ChangeLog 2020-10-15 19:24:40 UTC (rev 268544)
@@ -1,3 +1,13 @@
+2020-10-15 Simon Fraser <[email protected]>
+
+ Scrolls in the passive event region only send one wheel event to the DOM
+ https://bugs.webkit.org/show_bug.cgi?id=217719
+
+ Reviewed by Tim Horton.
+
+ * fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt: Added.
+ * fast/scrolling/latching/latched-scroll-in-passive-region.html: Added.
+
2020-10-15 Andres Gonzalez <[email protected]>
Fix for multiple accessibility layout tests in isolated tree mode.
Added: trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt (0 => 268544)
--- trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region-expected.txt 2020-10-15 19:24:40 UTC (rev 268544)
@@ -0,0 +1,10 @@
+Tests that a gesture in the passive wheel event region dispatches more than one wheel event
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS wheelEventHandlerCount > 4 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region.html (0 => 268544)
--- trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/latching/latched-scroll-in-passive-region.html 2020-10-15 19:24:40 UTC (rev 268544)
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ body {
+ height: 3000px;
+ }
+ </style>
+<script src=""
+<script src=""
+<script>
+ jsTestIsAsync = true;
+
+ let wheelEventHandlerCount = 0;
+ async function scrollTest()
+ {
+ window.addEventListener('wheel', (event) => {
+ ++wheelEventHandlerCount;
+ event.preventDefault();
+ }, { passive: true });
+
+ await UIHelper.renderingUpdate();
+
+ eventSender.mouseMoveTo(100, 10);
+ eventSender.monitorWheelEvents();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+ await UIHelper.renderingUpdate();
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+
+ await UIHelper.waitForScrollCompletion();
+
+ shouldBeTrue('wheelEventHandlerCount > 4');
+
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ description("Tests that a gesture in the passive wheel event region dispatches more than one wheel event");
+ scrollTest();
+ }, false);
+</script>
+</head>
+<body>
+ <div id="wheel-handler-region"></div>
+ <div id="console"></div>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (268543 => 268544)
--- trunk/Source/WebCore/ChangeLog 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/ChangeLog 2020-10-15 19:24:40 UTC (rev 268544)
@@ -1,3 +1,37 @@
+2020-10-15 Simon Fraser <[email protected]>
+
+ Scrolls in the passive event region only send one wheel event to the DOM
+ https://bugs.webkit.org/show_bug.cgi?id=217719
+
+ Reviewed by Tim Horton.
+
+ When we hit the scrolling thread latching codepath, we'd always use { WheelEventProcessingSteps::ScrollingThread }
+ for the steps, but that caused us to not send DOM events.
+
+ Latching has to store the steps from when latching occurs (we should not re-hit-test
+ to get the region state on every event), so fix up ScrollingTreeLatchingController to
+ do that.
+
+ Test: fast/scrolling/latching/latched-scroll-in-passive-region.html
+
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::determineWheelEventProcessing):
+ (WebCore::ScrollingTree::handleWheelEvent):
+ (WebCore::ScrollingTree::handleWheelEventWithNode):
+ * page/scrolling/ScrollingTree.h:
+ * page/scrolling/ScrollingTreeLatchingController.cpp:
+ (WebCore::ScrollingTreeLatchingController::receivedWheelEvent):
+ (WebCore::ScrollingTreeLatchingController::latchingDataForEvent const):
+ (WebCore::ScrollingTreeLatchingController::latchedNodeID const):
+ (WebCore::ScrollingTreeLatchingController::latchedNodeAndSteps const):
+ (WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent):
+ (WebCore::ScrollingTreeLatchingController::nodeWasRemoved):
+ (WebCore::ScrollingTreeLatchingController::clearLatchedNode):
+ (WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const): Deleted.
+ * page/scrolling/ScrollingTreeLatchingController.h:
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread):
+
2020-10-15 Andres Gonzalez <[email protected]>
Fix for multiple accessibility layout tests in isolated tree mode.
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (268543 => 268544)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-10-15 19:24:40 UTC (rev 268544)
@@ -68,10 +68,11 @@
// This method is invoked by the event handling thread
LockHolder lock(m_treeStateMutex);
- auto latchedNode = m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching);
- LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::shouldHandleWheelEventSynchronously " << wheelEvent << " have latched node " << latchedNode);
- if (latchedNode)
- return { WheelEventProcessingSteps::ScrollingThread };
+ auto latchedNodeAndSteps = m_latchingController.latchingDataForEvent(wheelEvent, m_allowLatching);
+ if (latchedNodeAndSteps) {
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::determineWheelEventProcessing " << wheelEvent << " have latched node " << latchedNodeAndSteps->scrollingNodeID << " steps " << latchedNodeAndSteps->processingSteps);
+ return latchedNodeAndSteps->processingSteps;
+ }
m_latchingController.receivedWheelEvent(wheelEvent, m_allowLatching);
@@ -135,14 +136,14 @@
m_gestureState.receivedWheelEvent(wheelEvent);
- if (auto latchedNodeID = m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching)) {
- LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::handleWheelEvent: has latched node " << latchedNodeID);
- auto* node = nodeForID(*latchedNodeID);
+ if (auto latchedNodeAndSteps = m_latchingController.latchingDataForEvent(wheelEvent, m_allowLatching)) {
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::handleWheelEvent: has latched node " << latchedNodeAndSteps->scrollingNodeID);
+ auto* node = nodeForID(latchedNodeAndSteps->scrollingNodeID);
if (is<ScrollingTreeScrollingNode>(node)) {
auto result = downcast<ScrollingTreeScrollingNode>(*node).handleWheelEvent(wheelEvent);
if (result.wasHandled) {
- m_latchingController.nodeDidHandleEvent(*latchedNodeID, wheelEvent, m_allowLatching);
- m_gestureState.nodeDidHandleEvent(*latchedNodeID, wheelEvent);
+ m_latchingController.nodeDidHandleEvent(latchedNodeAndSteps->scrollingNodeID, processingSteps, wheelEvent, m_allowLatching);
+ m_gestureState.nodeDidHandleEvent(latchedNodeAndSteps->scrollingNodeID, wheelEvent);
}
return result;
}
@@ -154,7 +155,7 @@
LOG_WITH_STREAM(Scrolling, stream << "ScrollingTree::handleWheelEvent found node " << (node ? node->scrollingNodeID() : 0) << " for point " << position);
- return handleWheelEventWithNode(wheelEvent, node.get());
+ return handleWheelEventWithNode(wheelEvent, processingSteps, node.get());
}();
result.steps.add(processingSteps & WheelEventProcessingSteps::MainThreadForDOMEventDispatch);
@@ -161,7 +162,7 @@
return result;
}
-WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, ScrollingTreeNode* node)
+WheelEventHandlingResult ScrollingTree::handleWheelEventWithNode(const PlatformWheelEvent& wheelEvent, OptionSet<WheelEventProcessingSteps> processingSteps, ScrollingTreeNode* node)
{
while (node) {
if (is<ScrollingTreeScrollingNode>(*node)) {
@@ -169,7 +170,7 @@
auto result = scrollingNode.handleWheelEvent(wheelEvent);
if (result.wasHandled) {
- m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent, m_allowLatching);
+ m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), processingSteps, wheelEvent, m_allowLatching);
m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
return result;
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (268543 => 268544)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-10-15 19:24:40 UTC (rev 268544)
@@ -212,7 +212,7 @@
WEBCORE_EXPORT void willProcessWheelEvent();
protected:
- WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, ScrollingTreeNode*);
+ WheelEventHandlingResult handleWheelEventWithNode(const PlatformWheelEvent&, OptionSet<WheelEventProcessingSteps>, ScrollingTreeNode*);
FloatPoint mainFrameScrollPosition() const;
void setMainFrameScrollPosition(FloatPoint);
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp (268543 => 268544)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp 2020-10-15 19:24:40 UTC (rev 268544)
@@ -47,13 +47,13 @@
return;
LockHolder locker(m_latchedNodeMutex);
- if (wheelEvent.isGestureStart() && m_latchedNodeID && !latchedNodeIsRelevant()) {
+ if (wheelEvent.isGestureStart() && m_latchedNodeAndSteps && !latchedNodeIsRelevant()) {
LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " receivedWheelEvent - " << (MonotonicTime::now() - m_lastLatchedNodeInterationTime).milliseconds() << "ms since last event, clearing latched node");
- m_latchedNodeID.reset();
+ m_latchedNodeAndSteps.reset();
}
}
-Optional<ScrollingNodeID> ScrollingTreeLatchingController::latchedNodeForEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching) const
+Optional<ScrollingTreeLatchingController::ScrollingNodeAndProcessingSteps> ScrollingTreeLatchingController::latchingDataForEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching) const
{
if (!allowLatching)
return WTF::nullopt;
@@ -61,9 +61,9 @@
LockHolder locker(m_latchedNodeMutex);
// If we have a latched node, use it.
- if (wheelEvent.useLatchedEventElement() && m_latchedNodeID && latchedNodeIsRelevant()) {
- LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " latchedNodeForEvent: returning " << m_latchedNodeID);
- return m_latchedNodeID.value();
+ if (wheelEvent.useLatchedEventElement() && m_latchedNodeAndSteps && latchedNodeIsRelevant()) {
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " latchedNodeForEvent: returning " << m_latchedNodeAndSteps->scrollingNodeID);
+ return m_latchedNodeAndSteps;
}
return WTF::nullopt;
@@ -72,17 +72,26 @@
Optional<ScrollingNodeID> ScrollingTreeLatchingController::latchedNodeID() const
{
LockHolder locker(m_latchedNodeMutex);
- return m_latchedNodeID;
+ if (m_latchedNodeAndSteps)
+ return m_latchedNodeAndSteps->scrollingNodeID;
+
+ return WTF::nullopt;
}
-void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, const PlatformWheelEvent& wheelEvent, bool allowLatching)
+Optional<ScrollingTreeLatchingController::ScrollingNodeAndProcessingSteps> ScrollingTreeLatchingController::latchedNodeAndSteps() const
{
+ LockHolder locker(m_latchedNodeMutex);
+ return m_latchedNodeAndSteps;
+}
+
+void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, OptionSet<WheelEventProcessingSteps> processingSteps, const PlatformWheelEvent& wheelEvent, bool allowLatching)
+{
if (!allowLatching)
return;
LockHolder locker(m_latchedNodeMutex);
- if (wheelEvent.useLatchedEventElement() && m_latchedNodeID == scrollingNodeID) {
+ if (wheelEvent.useLatchedEventElement() && m_latchedNodeAndSteps && m_latchedNodeAndSteps->scrollingNodeID == scrollingNodeID) {
m_lastLatchedNodeInterationTime = MonotonicTime::now();
return;
}
@@ -91,7 +100,7 @@
return;
LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " nodeDidHandleEvent: latching to " << scrollingNodeID);
- m_latchedNodeID = scrollingNodeID;
+ m_latchedNodeAndSteps = ScrollingNodeAndProcessingSteps { scrollingNodeID, processingSteps };
m_lastLatchedNodeInterationTime = MonotonicTime::now();
}
@@ -98,15 +107,15 @@
void ScrollingTreeLatchingController::nodeWasRemoved(ScrollingNodeID nodeID)
{
LockHolder locker(m_latchedNodeMutex);
- if (nodeID == m_latchedNodeID)
- m_latchedNodeID.reset();
+ if (m_latchedNodeAndSteps && m_latchedNodeAndSteps->scrollingNodeID == nodeID)
+ m_latchedNodeAndSteps.reset();
}
void ScrollingTreeLatchingController::clearLatchedNode()
{
LockHolder locker(m_latchedNodeMutex);
- LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " clearLatchedNode (was " << m_latchedNodeID << ")");
- m_latchedNodeID.reset();
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " clearLatchedNode");
+ m_latchedNodeAndSteps.reset();
}
bool ScrollingTreeLatchingController::latchedNodeIsRelevant() const
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h (268543 => 268544)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h 2020-10-15 19:24:40 UTC (rev 268544)
@@ -39,13 +39,20 @@
class ScrollingTreeLatchingController {
public:
+ struct ScrollingNodeAndProcessingSteps {
+ ScrollingNodeID scrollingNodeID;
+ OptionSet<WheelEventProcessingSteps> processingSteps;
+ };
+
ScrollingTreeLatchingController();
void receivedWheelEvent(const PlatformWheelEvent&, bool allowLatching);
- Optional<ScrollingNodeID> latchedNodeForEvent(const PlatformWheelEvent&, bool allowLatching) const;
- void nodeDidHandleEvent(ScrollingNodeID, const PlatformWheelEvent&, bool allowLatching);
+ Optional<ScrollingNodeAndProcessingSteps> latchingDataForEvent(const PlatformWheelEvent&, bool allowLatching) const;
+ void nodeDidHandleEvent(ScrollingNodeID, OptionSet<WheelEventProcessingSteps>, const PlatformWheelEvent&, bool allowLatching);
+
Optional<ScrollingNodeID> latchedNodeID() const;
+ Optional<ScrollingNodeAndProcessingSteps> latchedNodeAndSteps() const;
void nodeWasRemoved(ScrollingNodeID);
void clearLatchedNode();
@@ -54,7 +61,7 @@
bool latchedNodeIsRelevant() const;
mutable Lock m_latchedNodeMutex;
- Markable<ScrollingNodeID, IntegralMarkableTraits<ScrollingNodeID, 0>> m_latchedNodeID;
+ Optional<ScrollingNodeAndProcessingSteps> m_latchedNodeAndSteps;
MonotonicTime m_lastLatchedNodeInterationTime;
};
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (268543 => 268544)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-10-15 19:12:02 UTC (rev 268543)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-10-15 19:24:40 UTC (rev 268544)
@@ -70,7 +70,7 @@
SetForScope<bool> disallowLatchingScope(m_allowLatching, false);
RefPtr<ScrollingTreeNode> targetNode = nodeForID(targetNodeID);
- auto result = handleWheelEventWithNode(wheelEvent, targetNode.get());
+ auto result = handleWheelEventWithNode(wheelEvent, { }, targetNode.get());
return result.wasHandled;
}
