Diff
Modified: trunk/LayoutTests/ChangeLog (259804 => 259805)
--- trunk/LayoutTests/ChangeLog 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/LayoutTests/ChangeLog 2020-04-09 16:25:08 UTC (rev 259805)
@@ -1,3 +1,22 @@
+2020-04-08 Simon Fraser <[email protected]>
+
+ [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
+ https://bugs.webkit.org/show_bug.cgi?id=210095
+ <rdar://problem/61376245>
+
+ Reviewed by Tim Horton.
+
+ Tests that rubberbanding works on a simple page, and that edge swipes work on a simple page.
+
+ Edge swipes inside overflow:scroll are still broken by latching and will be fixed later.
+
+ * resources/ui-helper.js:
+ (window.UIHelper.async mouseWheelScrollAt):
+ * scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt: Added.
+ * scrollingcoordinator/mac/latching/main-frame-back-swipe.html: Added.
+ * scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt: Added.
+ * scrollingcoordinator/mac/latching/simple-page-rubberbands.html: Added.
+
2020-04-09 Youenn Fablet <[email protected]>
[MacOS] Stopping a cloned audio capture track should not stop the original audio track
Modified: trunk/LayoutTests/resources/ui-helper.js (259804 => 259805)
--- trunk/LayoutTests/resources/ui-helper.js 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/LayoutTests/resources/ui-helper.js 2020-04-09 16:25:08 UTC (rev 259805)
@@ -28,13 +28,23 @@
eventSender.mouseMoveTo(x2, y2);
eventSender.mouseUp();
}
-
- static async mouseWheelScrollAt(x, y)
+
+ static async mouseWheelScrollAt(x, y, beginX, beginY, deltaX, deltaY)
{
+ if (beginX === undefined)
+ beginX = 0;
+ if (beginY === undefined)
+ beginY = -1;
+
+ if (deltaX === undefined)
+ deltaX = 0;
+ if (deltaY === undefined)
+ deltaY = -10;
+
eventSender.monitorWheelEvents();
eventSender.mouseMoveTo(x, y);
- eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
- eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(beginX, beginY, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(deltaX, deltaY, "changed", "none");
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
return new Promise(resolve => {
eventSender.callAfterScrollingCompletes(() => {
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt (0 => 259805)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe-expected.txt 2020-04-09 16:25:08 UTC (rev 259805)
@@ -0,0 +1,17 @@
+Swipes should rubber-band on each side.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Swipe to the left
+begin swipe
+PASS minXOffset is 0
+PASS maxXOffset is -1000
+
+Swipe to the right
+PASS minXOffset is 0
+PASS maxXOffset > 0 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe.html (0 => 259805)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/main-frame-back-swipe.html 2020-04-09 16:25:08 UTC (rev 259805)
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+
+ </style>
+ <script src=""
+ <script src=""
+
+ <script>
+ jsTestIsAsync = true;
+
+ var minXOffset;
+ var maxXOffset;
+
+ function reset()
+ {
+ minXOffset = 0;
+ maxXOffset = -1000;
+ }
+
+ async function scrollTest()
+ {
+ description('Swipes should rubber-band on each side.');
+
+ if (window.testRunner) {
+ testRunner.setNavigationGesturesEnabled(true);
+ testRunner.installDidBeginSwipeCallback(() => { debug('begin swipe') });
+ }
+
+ history.pushState({ name: 'backstate' }, 'back');
+
+ window.addEventListener('scroll', () => {
+ minXOffset = Math.min(minXOffset, window.pageXOffset);
+ maxXOffset = Math.max(maxXOffset, window.pageXOffset);
+ }, false);
+
+ if (!window.eventSender) {
+ finishJSTest();
+ return;
+ }
+
+ reset();
+ debug('Swipe to the left');
+
+ await UIHelper.mouseWheelScrollAt(10, 10, 1, 0, 10, 0);
+ // Should not have received any scroll events.
+ shouldBe('minXOffset', '0');
+ shouldBe('maxXOffset', '-1000');
+
+ reset();
+ debug('');
+ debug('Swipe to the right');
+ await UIHelper.mouseWheelScrollAt(10, 10, -1, 0, -10, 0);
+ shouldBe('minXOffset', '0');
+ shouldBeTrue('maxXOffset > 0');
+
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ scrollTest();
+ }, false);
+ </script>
+</head>
+<body>
+
+ <script src=""
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt (0 => 259805)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands-expected.txt 2020-04-09 16:25:08 UTC (rev 259805)
@@ -0,0 +1,33 @@
+Swipes should rubber-band on each side.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+Swipe to the left
+PASS minXOffset < 0 is true
+PASS maxXOffset is 0
+PASS minYOffset is 0
+PASS maxYOffset is 0
+
+Swipe to the right
+PASS minXOffset is 0
+PASS maxXOffset > 0 is true
+PASS minYOffset is 0
+PASS maxYOffset is 0
+
+Swipe down
+PASS minXOffset is 0
+PASS maxXOffset is 0
+PASS minYOffset is 0
+PASS maxYOffset > 0 is true
+
+Swipe up
+PASS minXOffset is 0
+PASS maxXOffset is 0
+PASS minYOffset < 0 is true
+PASS maxYOffset is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands.html (0 => 259805)
--- trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/latching/simple-page-rubberbands.html 2020-04-09 16:25:08 UTC (rev 259805)
@@ -0,0 +1,92 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+
+ </style>
+ <script src=""
+ <script src=""
+
+ <script>
+ jsTestIsAsync = true;
+
+ var minXOffset;
+ var maxXOffset;
+ var minYOffset;
+ var maxYOffset;
+
+ function reset()
+ {
+ minXOffset = 0;
+ maxXOffset = -1000;
+ minYOffset = 0;
+ maxYOffset = -1000;
+ }
+
+ async function scrollTest()
+ {
+ description('Swipes should rubber-band on each side.');
+
+ window.addEventListener('scroll', () => {
+ minXOffset = Math.min(minXOffset, window.pageXOffset);
+ maxXOffset = Math.max(maxXOffset, window.pageXOffset);
+
+ minYOffset = Math.min(minYOffset, window.pageYOffset);
+ maxYOffset = Math.max(maxYOffset, window.pageYOffset);
+ }, false);
+
+
+ if (!window.eventSender) {
+ finishJSTest();
+ return;
+ }
+
+ reset();
+ debug('');
+ debug('Swipe to the left');
+
+ await UIHelper.mouseWheelScrollAt(10, 10, 1, 0, 10, 0);
+ shouldBeTrue('minXOffset < 0');
+ shouldBe('maxXOffset', '0');
+ shouldBe('minYOffset', '0');
+ shouldBe('maxYOffset', '0');
+
+ reset();
+ debug('');
+ debug('Swipe to the right');
+ await UIHelper.mouseWheelScrollAt(10, 10, -1, 0, -10, 0);
+ shouldBe('minXOffset', '0');
+ shouldBeTrue('maxXOffset > 0');
+ shouldBe('minYOffset', '0');
+ shouldBe('maxYOffset', '0');
+
+ reset();
+ debug('');
+ debug('Swipe down');
+ await UIHelper.mouseWheelScrollAt(10, 10, 0, -1, 0, -10);
+ shouldBe('minXOffset', '0');
+ shouldBe('maxXOffset', '0');
+ shouldBe('minYOffset', '0');
+ shouldBeTrue('maxYOffset > 0');
+
+ reset();
+ debug('');
+ debug('Swipe up');
+ await UIHelper.mouseWheelScrollAt(10, 10, 0, 1, 0, 10);
+ shouldBe('minXOffset', '0');
+ shouldBe('maxXOffset', '0');
+ shouldBeTrue('minYOffset < 0');
+ shouldBe('maxYOffset', '0');
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ scrollTest();
+ }, false);
+ </script>
+</head>
+<body>
+
+ <script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (259804 => 259805)
--- trunk/Source/WebCore/ChangeLog 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/ChangeLog 2020-04-09 16:25:08 UTC (rev 259805)
@@ -1,3 +1,52 @@
+2020-04-08 Simon Fraser <[email protected]>
+
+ [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
+ https://bugs.webkit.org/show_bug.cgi?id=210095
+ <rdar://problem/61376245>
+
+ Reviewed by Tim Horton.
+
+ With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
+ determine whether the scrolling tree handled the scroll; we have to wait until the
+ event has been processed by the scrolling thread. To allow that, add a
+ ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
+ a callback that's called when the scrolling thread is done with the event. EventDispatcher
+ uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
+ proceeds with history or reading list swipes.
+
+ Various fixes were necessary to correctly determine whether the event was handled.
+
+ ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
+ and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
+ and reading list navigation).
+
+ Tests: scrollingcoordinator/mac/latching/main-frame-back-swipe.html
+ scrollingcoordinator/mac/latching/simple-page-rubberbands.html
+
+ * page/scrolling/ScrollingCoordinatorTypes.h:
+ * page/scrolling/ScrollingThread.h:
+ * page/scrolling/ScrollingTree.cpp:
+ (WebCore::ScrollingTree::handleWheelEvent):
+ (WebCore::ScrollingTree::mainFrameCanRubberBandInDirection):
+ * page/scrolling/ScrollingTree.h:
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::tryToHandleWheelEvent):
+ * page/scrolling/ThreadedScrollingTree.h:
+ * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::handleWheelEvent):
+ * page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsHorizontalStretching const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::allowsVerticalStretching const):
+ (WebCore::ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection const):
+ * platform/cocoa/ScrollController.h:
+ * platform/cocoa/ScrollController.mm:
+ (WebCore::ScrollController::handleWheelEvent):
+ (WebCore::ScrollController::wheelDeltaBiasingTowardsVertical):
+ (WebCore::ScrollController::directionFromEvent):
+ (WebCore::ScrollController::shouldRubberBandInHorizontalDirection const):
+ (WebCore::ScrollController::shouldRubberBandInDirection const):
+ (WebCore::ScrollController::shouldRubberBandInHorizontalDirection): Deleted.
+
2020-04-09 Alex Christensen <[email protected]>
IPC serialization of enums should serialize std::underlying_type instead of uint64_t
Modified: trunk/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -95,7 +95,8 @@
enum class ScrollingEventResult {
DidNotHandleEvent,
DidHandleEvent,
- SendToMainThread
+ SendToScrollingThread,
+ SendToMainThread,
};
enum class ViewportRectStability {
Modified: trunk/Source/WebCore/page/scrolling/ScrollingThread.h (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ScrollingThread.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ScrollingThread.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -48,7 +48,7 @@
WTF_MAKE_NONCOPYABLE(ScrollingThread);
public:
- static bool isCurrentThread();
+ WEBCORE_EXPORT static bool isCurrentThread();
WEBCORE_EXPORT static void dispatch(Function<void ()>&&);
// Will dispatch the given function on the main thread once all pending functions
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2020-04-09 16:25:08 UTC (rev 259805)
@@ -101,7 +101,8 @@
if (!asyncFrameOrOverflowScrollingEnabled()) {
if (m_rootNode)
- m_rootNode->handleWheelEvent(wheelEvent);
+ return m_rootNode->handleWheelEvent(wheelEvent);
+
return ScrollingEventResult::DidNotHandleEvent;
}
@@ -397,6 +398,20 @@
m_swipeState.canRubberBand = canRubberBand;
}
+bool ScrollingTree::mainFrameCanRubberBandInDirection(ScrollDirection direction)
+{
+ LockHolder locker(m_swipeStateMutex);
+
+ switch (direction) {
+ case ScrollUp: return m_swipeState.canRubberBand.top();
+ case ScrollDown: return m_swipeState.canRubberBand.bottom();
+ case ScrollLeft: return m_swipeState.canRubberBand.left();
+ case ScrollRight: return m_swipeState.canRubberBand.right();
+ };
+
+ return false;
+}
+
// Can be called from the main thread.
void ScrollingTree::setScrollPinningBehavior(ScrollPinningBehavior pinning)
{
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -27,6 +27,7 @@
#if ENABLE(ASYNC_SCROLLING)
+#include "PageIdentifier.h"
#include "PlatformWheelEvent.h"
#include "RectEdges.h"
#include "Region.h"
@@ -62,7 +63,9 @@
bool asyncFrameOrOverflowScrollingEnabled() const { return m_asyncFrameOrOverflowScrollingEnabled; }
void setAsyncFrameOrOverflowScrollingEnabled(bool);
- virtual ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&) = 0;
+ using CompletionFunction = WTF::Function<void (ScrollingEventResult)>;
+ // Note that CompletionFunction may get called on a different thread.
+ virtual ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&, CompletionFunction&& = nullptr) = 0;
WEBCORE_EXPORT bool shouldHandleWheelEventSynchronously(const PlatformWheelEvent&);
void setMainFrameIsRubberBanding(bool);
@@ -123,6 +126,7 @@
// Can be called from any thread. Will update what edges allow rubber-banding.
WEBCORE_EXPORT void setMainFrameCanRubberBand(RectEdges<bool>);
+ bool mainFrameCanRubberBandInDirection(ScrollDirection);
bool isHandlingProgrammaticScroll() const { return m_isHandlingProgrammaticScroll; }
void setIsHandlingProgrammaticScroll(bool isHandlingProgrammaticScroll) { m_isHandlingProgrammaticScroll = isHandlingProgrammaticScroll; }
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2020-04-09 16:25:08 UTC (rev 259805)
@@ -49,20 +49,19 @@
ASSERT(!m_scrollingCoordinator);
}
-ScrollingEventResult ThreadedScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent)
+ScrollingEventResult ThreadedScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent, CompletionFunction&& completionFunction)
{
if (shouldHandleWheelEventSynchronously(wheelEvent))
return ScrollingEventResult::SendToMainThread;
- if (willWheelEventStartSwipeGesture(wheelEvent))
- return ScrollingEventResult::DidNotHandleEvent;
-
RefPtr<ThreadedScrollingTree> protectedThis(this);
- ScrollingThread::dispatch([protectedThis, wheelEvent] {
- protectedThis->handleWheelEvent(wheelEvent);
+ ScrollingThread::dispatch([protectedThis, wheelEvent, completionFunc = WTFMove(completionFunction)] {
+ auto result = protectedThis->handleWheelEvent(wheelEvent);
+ if (completionFunc)
+ completionFunc(result);
});
- return ScrollingEventResult::DidHandleEvent;
+ return ScrollingEventResult::SendToScrollingThread;
}
ScrollingEventResult ThreadedScrollingTree::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -51,7 +51,7 @@
// 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.
- ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&) override;
+ ScrollingEventResult tryToHandleWheelEvent(const PlatformWheelEvent&, CompletionFunction&&) override;
void invalidate() override;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2020-04-09 16:25:08 UTC (rev 259805)
@@ -153,7 +153,7 @@
if (!canHaveScrollbars())
return ScrollingEventResult::DidNotHandleEvent;
- m_delegate.handleWheelEvent(wheelEvent);
+ bool handled = m_delegate.handleWheelEvent(wheelEvent);
#if ENABLE(CSS_SCROLL_SNAP)
if (isRootNode())
@@ -165,8 +165,7 @@
scrollingTree().setOrClearLatchedNode(wheelEvent, scrollingNodeID());
scrollingTree().handleWheelEventPhase(wheelEvent.phase());
- // FIXME: This needs to return whether the event was handled.
- return ScrollingEventResult::DidHandleEvent;
+ return handled ? ScrollingEventResult::DidHandleEvent : ScrollingEventResult::DidNotHandleEvent;
}
FloatPoint ScrollingTreeFrameScrollingNodeMac::adjustedScrollPosition(const FloatPoint& position, ScrollClamping clamp) const
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm (259804 => 259805)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm 2020-04-09 16:25:08 UTC (rev 259805)
@@ -111,6 +111,7 @@
// FIXME: We should find a way to share some of the code from newGestureIsStarting(), isAlreadyPinnedInDirectionOfGesture(),
// allowsVerticalStretching(), and allowsHorizontalStretching() with the implementation in ScrollAnimatorMac.
+// This is also the same as PlatformWheelEvent::shouldConsiderLatching().
static bool newGestureIsStarting(const PlatformWheelEvent& wheelEvent)
{
return wheelEvent.phase() == PlatformWheelEventPhaseMayBegin || wheelEvent.phase() == PlatformWheelEventPhaseBegan;
@@ -140,9 +141,13 @@
}
case ScrollElasticityNone:
return false;
- case ScrollElasticityAllowed:
+ case ScrollElasticityAllowed: {
+ auto scrollDirection = ScrollController::directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
+ if (scrollDirection)
+ return shouldRubberBandInDirection(scrollDirection.value());
return true;
}
+ }
ASSERT_NOT_REACHED();
return false;
@@ -158,9 +163,13 @@
}
case ScrollElasticityNone:
return false;
- case ScrollElasticityAllowed:
+ case ScrollElasticityAllowed: {
+ auto scrollDirection = ScrollController::directionFromEvent(wheelEvent, ScrollEventAxis::Vertical);
+ if (scrollDirection)
+ return shouldRubberBandInDirection(scrollDirection.value());
return true;
}
+ }
ASSERT_NOT_REACHED();
return false;
@@ -184,6 +193,7 @@
return stretch;
}
+// FIXME: Share more with ScrollingTreeScrollingNode::edgePinnedState().
bool ScrollingTreeScrollingNodeDelegateMac::pinnedInDirection(const FloatSize& delta) const
{
FloatSize limitDelta;
@@ -223,8 +233,12 @@
return hasEnabledVerticalScrollbar();
}
-bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection) const
+bool ScrollingTreeScrollingNodeDelegateMac::shouldRubberBandInDirection(ScrollDirection direction) const
{
+ if (scrollingNode().isRootNode())
+ return scrollingTree().mainFrameCanRubberBandInDirection(direction);
+
+ // FIXME: Consult the node.
return true;
}
Modified: trunk/Source/WebCore/platform/cocoa/ScrollController.h (259804 => 259805)
--- trunk/Source/WebCore/platform/cocoa/ScrollController.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/platform/cocoa/ScrollController.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -146,6 +146,11 @@
#endif
#endif
+ static FloatSize wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent&);
+
+ enum class WheelAxisBias { None, Vertical };
+ static Optional<ScrollDirection> directionFromEvent(const PlatformWheelEvent&, Optional<ScrollEventAxis>, WheelAxisBias = WheelAxisBias::None);
+
private:
#if ENABLE(RUBBER_BANDING)
void startSnapRubberbandTimer();
@@ -153,7 +158,8 @@
void snapRubberBand();
void snapRubberBandTimerFired();
- bool shouldRubberBandInHorizontalDirection(const PlatformWheelEvent&);
+ bool shouldRubberBandInHorizontalDirection(const PlatformWheelEvent&) const;
+ bool shouldRubberBandInDirection(ScrollDirection) const;
#endif
#if ENABLE(CSS_SCROLL_SNAP)
Modified: trunk/Source/WebCore/platform/cocoa/ScrollController.mm (259804 => 259805)
--- trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2020-04-09 16:25:08 UTC (rev 259805)
@@ -104,11 +104,16 @@
return false;
#endif
if (wheelEvent.phase() == PlatformWheelEventPhaseBegan) {
- // First, check if we should rubber-band at all.
- if (m_client.pinnedInDirection(FloatSize(-wheelEvent.deltaX(), 0))
- && !shouldRubberBandInHorizontalDirection(wheelEvent))
+ // FIXME: Trying to decide if a gesture is horizontal or vertical at the "began" phase is very error-prone.
+ auto direction = directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
+ // FIXME: pinnedInDirection() needs cleanup.
+ if (direction && m_client.pinnedInDirection(FloatSize(-wheelEvent.deltaX(), 0)) && !shouldRubberBandInDirection(direction.value()))
return false;
+ direction = directionFromEvent(wheelEvent, ScrollEventAxis::Vertical);
+ if (direction && m_client.pinnedInDirection(FloatSize(0, -wheelEvent.deltaY())) && !shouldRubberBandInDirection(direction.value()))
+ return false;
+
m_inScrollGesture = true;
m_momentumScrollInProgress = false;
m_ignoreMomentumScrolls = false;
@@ -164,6 +169,7 @@
deltaY += eventCoalescedDeltaY;
// Slightly prefer scrolling vertically by applying the = case to deltaY
+ // FIXME: Use wheelDeltaBiasingTowardsVertical().
if (fabsf(deltaY) >= fabsf(deltaX))
deltaX = 0;
else
@@ -225,6 +231,8 @@
}
}
+ bool handled = true;
+
if (deltaX || deltaY) {
if (!(shouldStretch || isVerticallyStretched || isHorizontallyStretched)) {
if (deltaY) {
@@ -239,6 +247,7 @@
if (!m_client.allowsHorizontalStretching(wheelEvent)) {
deltaX = 0;
eventCoalescedDeltaX = 0;
+ handled = false;
} else if (deltaX && !isHorizontallyStretched && !m_client.pinnedInDirection(FloatSize(deltaX, 0))) {
deltaX *= scrollWheelMultiplier();
@@ -249,6 +258,7 @@
if (!m_client.allowsVerticalStretching(wheelEvent)) {
deltaY = 0;
eventCoalescedDeltaY = 0;
+ handled = false;
} else if (deltaY && !isVerticallyStretched && !m_client.pinnedInDirection(FloatSize(0, deltaY))) {
deltaY *= scrollWheelMultiplier();
@@ -281,10 +291,69 @@
m_lastMomentumScrollTimestamp = 0;
}
- return true;
+ return handled;
}
-#endif
+#endif // PLATFORM(MAC)
+FloatSize ScrollController::wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent& wheelEvent)
+{
+ auto deltaX = wheelEvent.deltaX();
+ auto deltaY = wheelEvent.deltaY();
+
+ if (fabsf(deltaY) >= fabsf(deltaX))
+ deltaX = 0;
+ else
+ deltaY = 0;
+
+ return { deltaX, deltaY };
+}
+
+Optional<ScrollDirection> ScrollController::directionFromEvent(const PlatformWheelEvent& wheelEvent, Optional<ScrollEventAxis> axis, WheelAxisBias bias)
+{
+ // FIXME: It's impossible to infer direction from a single event, since the start of a gesture is either zero or
+ // has small deltas on both axes.
+
+ auto wheelDelta = FloatSize { wheelEvent.deltaX(), wheelEvent.deltaY() };
+ if (bias == WheelAxisBias::Vertical)
+ wheelDelta = wheelDeltaBiasingTowardsVertical(wheelEvent);
+
+ if (axis) {
+ switch (axis.value()) {
+ case ScrollEventAxis::Vertical:
+ if (wheelDelta.height() < 0)
+ return ScrollDown;
+
+ if (wheelDelta.height() > 0)
+ return ScrollUp;
+ break;
+
+ case ScrollEventAxis::Horizontal:
+ if (wheelDelta.width() > 0)
+ return ScrollLeft;
+
+ if (wheelDelta.width() < 0)
+ return ScrollRight;
+ }
+
+ return WTF::nullopt;
+ }
+
+ // Check Y first because vertical scrolling dominates.
+ if (wheelDelta.height() < 0)
+ return ScrollDown;
+
+ if (wheelDelta.height() > 0)
+ return ScrollUp;
+
+ if (wheelDelta.width() > 0)
+ return ScrollLeft;
+
+ if (wheelDelta.width() < 0)
+ return ScrollRight;
+
+ return WTF::nullopt;
+}
+
#if ENABLE(RUBBER_BANDING)
static inline float roundTowardZero(float num)
{
@@ -421,17 +490,22 @@
m_snapRubberbandTimerIsActive = true;
}
-bool ScrollController::shouldRubberBandInHorizontalDirection(const PlatformWheelEvent& wheelEvent)
+bool ScrollController::shouldRubberBandInHorizontalDirection(const PlatformWheelEvent& wheelEvent) const
{
- if (wheelEvent.deltaX() > 0)
- return m_client.shouldRubberBandInDirection(ScrollLeft);
- if (wheelEvent.deltaX() < 0)
- return m_client.shouldRubberBandInDirection(ScrollRight);
+ auto direction = directionFromEvent(wheelEvent, ScrollEventAxis::Horizontal);
+ if (direction)
+ return shouldRubberBandInDirection(direction.value());
return true;
}
-#endif
+bool ScrollController::shouldRubberBandInDirection(ScrollDirection direction) const
+{
+ return m_client.shouldRubberBandInDirection(direction);
+}
+
+#endif // ENABLE(RUBBER_BANDING)
+
#if ENABLE(CSS_SCROLL_SNAP)
#if PLATFORM(MAC)
Modified: trunk/Source/WebKit/ChangeLog (259804 => 259805)
--- trunk/Source/WebKit/ChangeLog 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/ChangeLog 2020-04-09 16:25:08 UTC (rev 259805)
@@ -1,3 +1,35 @@
+2020-04-08 Simon Fraser <[email protected]>
+
+ [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes
+ https://bugs.webkit.org/show_bug.cgi?id=210095
+ <rdar://problem/61376245>
+
+ Reviewed by Tim Horton.
+
+ With async overflow/frame scrolling, EventDispatcher::wheelEvent() can't immediately
+ determine whether the scrolling tree handled the scroll; we have to wait until the
+ event has been processed by the scrolling thread. To allow that, add a
+ ScrollingEventResult::SendToScrollingThread return value and a give tryToHandleWheelEvent()
+ a callback that's called when the scrolling thread is done with the event. EventDispatcher
+ uses that to send the "didReceiveEvent" with "handled" back to the UI process, which then
+ proceeds with history or reading list swipes.
+
+ Various fixes were necessary to correctly determine whether the event was handled.
+
+ ScrollingTreeFrameScrollingNodeMac::handleWheelEvent() didn't return an accurate ScrollingEventResult,
+ and ScrollController didn't return false in cases where rubber-banding was disabled (which broke navigation swipes
+ and reading list navigation).
+
+ * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
+ (WebKit::RemoteScrollingCoordinatorProxy::handleWheelEvent):
+ * UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp:
+ (WebKit::RemoteScrollingTree::tryToHandleWheelEvent):
+ * UIProcess/RemoteLayerTree/RemoteScrollingTree.h:
+ * WebProcess/WebPage/EventDispatcher.cpp:
+ (WebKit::EventDispatcher::wheelEvent):
+ (WebKit::EventDispatcher::sendDidReceiveEvent):
+ * WebProcess/WebPage/EventDispatcher.h:
+
2020-04-09 Alex Christensen <[email protected]>
IPC serialization of enums should serialize std::underlying_type instead of uint64_t
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp (259804 => 259805)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp 2020-04-09 16:25:08 UTC (rev 259805)
@@ -178,7 +178,7 @@
bool RemoteScrollingCoordinatorProxy::handleWheelEvent(const PlatformWheelEvent& event)
{
- ScrollingEventResult result = m_scrollingTree->tryToHandleWheelEvent(event);
+ ScrollingEventResult result = m_scrollingTree->tryToHandleWheelEvent(event, nullptr);
return result == ScrollingEventResult::DidHandleEvent; // FIXME: handle other values.
}
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp (259804 => 259805)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp 2020-04-09 16:25:08 UTC (rev 259805)
@@ -61,7 +61,7 @@
{
}
-ScrollingEventResult RemoteScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent)
+ScrollingEventResult RemoteScrollingTree::tryToHandleWheelEvent(const PlatformWheelEvent& wheelEvent, CompletionFunction&&)
{
if (shouldHandleWheelEventSynchronously(wheelEvent))
return ScrollingEventResult::SendToMainThread;
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h (259804 => 259805)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -45,7 +45,7 @@
virtual ~RemoteScrollingTree();
bool isRemoteScrollingTree() const override { return true; }
- WebCore::ScrollingEventResult tryToHandleWheelEvent(const WebCore::PlatformWheelEvent&) override;
+ WebCore::ScrollingEventResult tryToHandleWheelEvent(const WebCore::PlatformWheelEvent&, CompletionFunction&&) override;
void handleMouseEvent(const WebCore::PlatformMouseEvent&);
Modified: trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp (259804 => 259805)
--- trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp 2020-04-09 16:25:08 UTC (rev 259805)
@@ -126,10 +126,20 @@
if (platformWheelEvent.phase() == PlatformWheelEventPhaseBegan)
scrollingTree->setMainFrameCanRubberBand({ canRubberBandAtTop, canRubberBandAtRight, canRubberBandAtBottom, canRubberBandAtLeft });
- ScrollingEventResult result = scrollingTree->tryToHandleWheelEvent(platformWheelEvent);
+ auto eventType = wheelEvent.type();
+ ScrollingEventResult result = scrollingTree->tryToHandleWheelEvent(platformWheelEvent, [pageID, eventType](ScrollingEventResult result) {
+ ASSERT(ScrollingThread::isCurrentThread());
+ ASSERT(result != ScrollingEventResult::SendToScrollingThread);
+ ASSERT(result != ScrollingEventResult::SendToMainThread);
+
+ sendDidReceiveEvent(pageID, eventType, result == ScrollingEventResult::DidHandleEvent);
+ });
+ if (result == ScrollingEventResult::SendToScrollingThread)
+ return;
+
if (result == ScrollingEventResult::DidHandleEvent || result == ScrollingEventResult::DidNotHandleEvent) {
- sendDidReceiveEvent(pageID, wheelEvent, result == ScrollingEventResult::DidHandleEvent);
+ sendDidReceiveEvent(pageID, wheelEvent.type(), result == ScrollingEventResult::DidHandleEvent);
return;
}
}
@@ -237,9 +247,9 @@
#endif
#if ENABLE(ASYNC_SCROLLING)
-void EventDispatcher::sendDidReceiveEvent(PageIdentifier pageID, const WebEvent& event, bool didHandleEvent)
+void EventDispatcher::sendDidReceiveEvent(PageIdentifier pageID, WebEvent::Type eventType, bool didHandleEvent)
{
- WebProcess::singleton().parentProcessConnection()->send(Messages::WebPageProxy::DidReceiveEvent(static_cast<uint32_t>(event.type()), didHandleEvent), pageID);
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebPageProxy::DidReceiveEvent(static_cast<uint32_t>(eventType), didHandleEvent), pageID);
}
#endif
Modified: trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.h (259804 => 259805)
--- trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.h 2020-04-09 16:18:19 UTC (rev 259804)
+++ trunk/Source/WebKit/WebProcess/WebPage/EventDispatcher.h 2020-04-09 16:25:08 UTC (rev 259805)
@@ -94,7 +94,7 @@
#endif
#if ENABLE(ASYNC_SCROLLING)
- void sendDidReceiveEvent(WebCore::PageIdentifier, const WebEvent&, bool didHandleEvent);
+ static void sendDidReceiveEvent(WebCore::PageIdentifier, WebEvent::Type, bool didHandleEvent);
#endif
Ref<WorkQueue> m_queue;
