Diff
Modified: trunk/LayoutTests/ChangeLog (261426 => 261427)
--- trunk/LayoutTests/ChangeLog 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/LayoutTests/ChangeLog 2020-05-09 00:02:45 UTC (rev 261427)
@@ -1,5 +1,16 @@
2020-05-08 Simon Fraser <[email protected]>
+ Async overflow scroll: sometimes a <select> becomes non-scrollable
+ https://bugs.webkit.org/show_bug.cgi?id=211433
+ <rdar://problem/62338474>
+
+ Reviewed by Dean Jackson.
+
+ * scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe-expected.txt: Added.
+ * scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe.html: Added.
+
+2020-05-08 Simon Fraser <[email protected]>
+
Overflow scrollers in iframes don't get mouseMovedInContentArea()
https://bugs.webkit.org/show_bug.cgi?id=211347
<rdar://problem/62784560>
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe-expected.txt (0 => 261427)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe-expected.txt 2020-05-09 00:02:45 UTC (rev 261427)
@@ -0,0 +1,15 @@
+
+Tests that scrolling over a <select> doesn't latch on the document
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS select.scrollTop is 90
+PASS window.pageYOffset is 0
+Scrolling up then down in the select
+PASS select.scrollTop > 0 is true
+PASS window.pageYOffset is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe.html (0 => 261427)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe.html 2020-05-09 00:02:45 UTC (rev 261427)
@@ -0,0 +1,95 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <style>
+ body {
+ height: 2000px;
+ }
+ select {
+ display: block;
+ font-size: 11pt;
+ margin: 10px;
+ border: 1px solid black;
+ }
+ </style>
+ <script src=""
+ <script src=""
+ <script>
+ jsTestIsAsync = true;
+
+ let select;
+ async function scrollTest()
+ {
+ select = document.getElementsByTagName('select')[0];
+
+ select.scrollTop = 90;
+ shouldBe('select.scrollTop', '90');
+ shouldBe('window.pageYOffset', '0');
+
+ if (!window.eventSender) {
+ finishJSTest();
+ return;
+ }
+
+ const selectBounds = select.getBoundingClientRect();
+ const x = selectBounds.left + 10;
+ const y = selectBounds.top + 10;
+
+ // We have to send all these as one sequence, because monitorWheelEvents() clears the latching state.
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(x, y);
+
+ debug('Scrolling up then down in the select');
+ // Scroll to the top of the select.
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 1, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 20, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+
+ await UIHelper.animationFrame();
+
+ // Scroll up again to trigger a document bounce.
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 1, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 20, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+
+ await UIHelper.animationFrame();
+
+ // Scroll down. This should scroll the select, not the document.
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -2, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -20, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -20, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+
+ await UIHelper.waitForScrollCompletion();
+
+ shouldBeTrue('select.scrollTop > 0');
+ shouldBe('window.pageYOffset', '0');
+
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ description("Tests that scrolling over a <select> doesn't latch on the document");
+ setTimeout(scrollTest, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <select size="5">
+ <option>January</option>
+ <option>February</option>
+ <option>March</option>
+ <option>April</option>
+ <option>May</option>
+ <option>June</option>
+ <option>July</option>
+ <option>August</option>
+ <option>September</option>
+ <option>October</option>
+ <option>November</option>
+ <option>December</option>
+ </select>
+<div id="console"></div>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (261426 => 261427)
--- trunk/Source/WebCore/ChangeLog 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/ChangeLog 2020-05-09 00:02:45 UTC (rev 261427)
@@ -1,3 +1,37 @@
+2020-05-08 Simon Fraser <[email protected]>
+
+ Async overflow scroll: sometimes a <select> becomes non-scrollable
+ https://bugs.webkit.org/show_bug.cgi?id=211433
+ <rdar://problem/62338474>
+
+ Reviewed by Dean Jackson.
+
+ Scrollable <select>s (RenderListBox) contribute to the non-fast scrollable region, so
+ wheel events inside them are propagated to the main thread. If the select is scrolled
+ to the end, the event will propagate to the enclosing scroller, then then FrameView::wheelEvent()
+ may send it back to the scrolling thread. If the scrolling thread is processing such an event
+ after the main thread, it should not perform any latching on the main thread.
+
+ Test: scrollingcoordinator/mac/latching/scrolling-select-should-not-latch-mainframe.html
+
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::shouldHandleWheelEventSynchronously):
+ (WebCore::ScrollingTree::handleWheelEvent):
+ * page/scrolling/ScrollingTree.h:
+ * page/scrolling/ScrollingTreeLatchingController.cpp:
+ (WebCore::ScrollingTreeLatchingController::receivedWheelEvent):
+ (WebCore::ScrollingTreeLatchingController::latchedNodeForEvent const):
+ (WebCore::ScrollingTreeLatchingController::nodeDidHandleEvent):
+ (WebCore::ScrollingTreeLatchingController::clearLatchedNode):
+ * page/scrolling/ScrollingTreeLatchingController.h:
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::handleWheelEventAfterMainThread):
+ * page/scrolling/ThreadedScrollingTree.h:
+ * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+ (WebCore::ScrollingCoordinatorMac::handleWheelEvent):
+ * platform/PlatformWheelEvent.cpp:
+ (WebCore::operator<<):
+
2020-05-08 Darin Adler <[email protected]>
Remove now-unneeded HAVE(AVFOUNDATION_LOADER_DELEGATE)
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-05-09 00:02:45 UTC (rev 261427)
@@ -68,10 +68,11 @@
// This method is invoked by the event handling thread
LockHolder lock(m_treeStateMutex);
- if (m_latchingController.latchedNodeForEvent(wheelEvent))
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTree::shouldHandleWheelEventSynchronously " << wheelEvent << " have latched node " << m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching));
+ if (m_latchingController.latchedNodeForEvent(wheelEvent, m_allowLatching))
return false;
- m_latchingController.receivedWheelEvent(wheelEvent);
+ m_latchingController.receivedWheelEvent(wheelEvent, m_allowLatching);
if (!m_treeState.eventTrackingRegions.isEmpty() && m_rootNode) {
FloatPoint position = wheelEvent.position();
@@ -100,7 +101,7 @@
if (isMonitoringWheelEvents())
receivedWheelEvent(wheelEvent);
- m_latchingController.receivedWheelEvent(wheelEvent);
+ m_latchingController.receivedWheelEvent(wheelEvent, m_allowLatching);
auto result = [&] {
if (!asyncFrameOrOverflowScrollingEnabled()) {
@@ -115,7 +116,7 @@
m_gestureState.receivedWheelEvent(wheelEvent);
- if (auto latchedNodeID = m_latchingController.latchedNodeForEvent(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 (is<ScrollingTreeScrollingNode>(node)) {
@@ -138,7 +139,7 @@
auto result = scrollingNode.handleWheelEvent(wheelEvent);
if (result == ScrollingEventResult::DidHandleEvent) {
- m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
+ m_latchingController.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent, m_allowLatching);
m_gestureState.nodeDidHandleEvent(scrollingNode.scrollingNodeID(), wheelEvent);
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-05-09 00:02:45 UTC (rev 261427)
@@ -235,6 +235,9 @@
Lock m_swipeStateMutex;
SwipeState m_swipeState;
+protected:
+ bool m_allowLatching { true };
+
private:
unsigned m_fixedOrStickyNodeCount { 0 };
bool m_isHandlingProgrammaticScroll { false };
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.cpp 2020-05-09 00:02:45 UTC (rev 261427)
@@ -41,8 +41,11 @@
ScrollingTreeLatchingController::ScrollingTreeLatchingController() = default;
-void ScrollingTreeLatchingController::receivedWheelEvent(const PlatformWheelEvent& wheelEvent)
+void ScrollingTreeLatchingController::receivedWheelEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching)
{
+ if (!allowLatching)
+ return;
+
LockHolder locker(m_latchedNodeMutex);
if (wheelEvent.shouldConsiderLatching()) {
if (m_latchedNodeID) {
@@ -55,8 +58,11 @@
}
}
-Optional<ScrollingNodeID> ScrollingTreeLatchingController::latchedNodeForEvent(const PlatformWheelEvent& wheelEvent) const
+Optional<ScrollingNodeID> ScrollingTreeLatchingController::latchedNodeForEvent(const PlatformWheelEvent& wheelEvent, bool allowLatching) const
{
+ if (!allowLatching)
+ return WTF::nullopt;
+
LockHolder locker(m_latchedNodeMutex);
// If we have a latched node, use it.
@@ -74,8 +80,11 @@
return m_latchedNodeID;
}
-void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, const PlatformWheelEvent& wheelEvent)
+void ScrollingTreeLatchingController::nodeDidHandleEvent(ScrollingNodeID scrollingNodeID, const PlatformWheelEvent& wheelEvent, bool allowLatching)
{
+ if (!allowLatching)
+ return;
+
LockHolder locker(m_latchedNodeMutex);
if (wheelEvent.useLatchedEventElement() && m_latchedNodeID == scrollingNodeID) {
@@ -101,6 +110,7 @@
void ScrollingTreeLatchingController::clearLatchedNode()
{
LockHolder locker(m_latchedNodeMutex);
+ LOG_WITH_STREAM(ScrollLatching, stream << "ScrollingTreeLatchingController " << this << " clearLatchedNode (was " << m_latchedNodeID << ")");
m_latchedNodeID.reset();
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeLatchingController.h 2020-05-09 00:02:45 UTC (rev 261427)
@@ -41,9 +41,9 @@
public:
ScrollingTreeLatchingController();
- void receivedWheelEvent(const PlatformWheelEvent&);
- Optional<ScrollingNodeID> latchedNodeForEvent(const PlatformWheelEvent&) const;
- void nodeDidHandleEvent(ScrollingNodeID, const PlatformWheelEvent&);
+ void receivedWheelEvent(const PlatformWheelEvent&, bool allowLatching);
+ Optional<ScrollingNodeID> latchedNodeForEvent(const PlatformWheelEvent&, bool allowLatching) const;
+ void nodeDidHandleEvent(ScrollingNodeID, const PlatformWheelEvent&, bool allowLatching);
Optional<ScrollingNodeID> latchedNodeID() const;
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-05-09 00:02:45 UTC (rev 261427)
@@ -36,6 +36,7 @@
#include "ScrollingTreeOverflowScrollProxyNode.h"
#include "ScrollingTreeScrollingNode.h"
#include <wtf/RunLoop.h>
+#include <wtf/SetForScope.h>
namespace WebCore {
@@ -71,6 +72,12 @@
return ScrollingTree::handleWheelEvent(wheelEvent);
}
+ScrollingEventResult ThreadedScrollingTree::handleWheelEventAfterMainThread(const PlatformWheelEvent& wheelEvent)
+{
+ SetForScope<bool> disallowLatchingScope(m_allowLatching, false);
+ return handleWheelEvent(wheelEvent);
+}
+
void ThreadedScrollingTree::invalidate()
{
// Invalidate is dispatched by the ScrollingCoordinator class on the ScrollingThread
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-05-09 00:02:45 UTC (rev 261427)
@@ -46,6 +46,8 @@
ScrollingEventResult handleWheelEvent(const PlatformWheelEvent&) override;
+ ScrollingEventResult handleWheelEventAfterMainThread(const PlatformWheelEvent&);
+
// Can be called from any thread. Will try to handle the wheel event on the scrolling thread.
// Returns true if the wheel event can be handled on the scrolling thread and false if the
// event must be sent again to the WebCore event handler.
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (261426 => 261427)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2020-05-09 00:02:45 UTC (rev 261427)
@@ -83,7 +83,7 @@
RefPtr<ThreadedScrollingTree> threadedScrollingTree = downcast<ThreadedScrollingTree>(scrollingTree());
ScrollingThread::dispatch([threadedScrollingTree, wheelEvent] {
- threadedScrollingTree->handleWheelEvent(wheelEvent);
+ threadedScrollingTree->handleWheelEventAfterMainThread(wheelEvent);
});
return ScrollingEventResult::DidHandleEvent;
}
Modified: trunk/Source/WebCore/platform/PlatformWheelEvent.cpp (261426 => 261427)
--- trunk/Source/WebCore/platform/PlatformWheelEvent.cpp 2020-05-09 00:02:41 UTC (rev 261426)
+++ trunk/Source/WebCore/platform/PlatformWheelEvent.cpp 2020-05-09 00:02:45 UTC (rev 261427)
@@ -53,7 +53,7 @@
ts << "PlatformWheelEvent " << &event << " at " << event.position() << " deltaX " << event.deltaX() << " deltaY " << event.deltaY();
#if ENABLE(KINETIC_SCROLLING)
- ts << " phase \"" << event.phase() << "\" momentumum phase \"" << event.momentumPhase() << "\"";
+ ts << " phase \"" << event.phase() << "\" momentum phase \"" << event.momentumPhase() << "\"";
#endif
return ts;
