[webkit-changes] [259818] trunk

Thu, 09 Apr 2020 13:12:24 -0700

Title: [259818] trunk
Revision
259818
Author
simon.fra...@apple.com
Date
2020-04-09 13:11:24 -0700 (Thu, 09 Apr 2020)

Log Message

Reset view navigation gesture state between tests
https://bugs.webkit.org/show_bug.cgi?id=210283

Reviewed by Tim Horton.

State in ViewGestureController could leak between tests if a test did not wait for the gesture to complete.
Specifically m_activeGestureType could be left as non-None.
Source/WebKit:

Fix by plumbing a 'reset' through from TestController::resetStateToConsistentValues().

The implementations leverage code from removeSwipeSnapshot(), but avoid the fact that removeSwipeSnapshot()
early returns in various cases by just always calling reset code, which is factored into a new resetState function.

* UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
* UIProcess/API/Cocoa/WKWebViewTesting.mm:
(-[WKWebView _resetNavigationGestureStateForTesting]):
* UIProcess/ViewGestureController.cpp:
(WebKit::ViewGestureController::willBeginGesture):
(WebKit::ViewGestureController::didEndGesture):
(WebKit::ViewGestureController::PendingSwipeTracker::handleEvent):
(WebKit::ViewGestureController::PendingSwipeTracker::eventWasNotHandledByWebCore):
* UIProcess/ViewGestureController.h:
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::removeSwipeSnapshot):
(WebKit::ViewGestureController::resetState):
(WebKit::ViewGestureController::reset):
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::removeSwipeSnapshot):
(WebKit::ViewGestureController::resetState):
(WebKit::ViewGestureController::reset):

Tools:

Fix by plumbing a 'reset' through from TestController::resetStateToConsistentValues().

The implementations leverage code from removeSwipeSnapshot(), but avoid the fact that removeSwipeSnapshot()
early returns in various cases by just always calling reset code, which is factored into a new resetState function.

* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (259817 => 259818)


--- trunk/Source/WebKit/ChangeLog	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/ChangeLog	2020-04-09 20:11:24 UTC (rev 259818)
@@ -1,3 +1,36 @@
+2020-04-09  Simon Fraser  <simon.fra...@apple.com>
+
+        Reset view navigation gesture state between tests
+        https://bugs.webkit.org/show_bug.cgi?id=210283
+
+        Reviewed by Tim Horton.
+
+        State in ViewGestureController could leak between tests if a test did not wait for the gesture to complete.
+        Specifically m_activeGestureType could be left as non-None.
+        
+        Fix by plumbing a 'reset' through from TestController::resetStateToConsistentValues().
+
+        The implementations leverage code from removeSwipeSnapshot(), but avoid the fact that removeSwipeSnapshot()
+        early returns in various cases by just always calling reset code, which is factored into a new resetState function.
+
+        * UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
+        * UIProcess/API/Cocoa/WKWebViewTesting.mm:
+        (-[WKWebView _resetNavigationGestureStateForTesting]):
+        * UIProcess/ViewGestureController.cpp:
+        (WebKit::ViewGestureController::willBeginGesture):
+        (WebKit::ViewGestureController::didEndGesture):
+        (WebKit::ViewGestureController::PendingSwipeTracker::handleEvent):
+        (WebKit::ViewGestureController::PendingSwipeTracker::eventWasNotHandledByWebCore):
+        * UIProcess/ViewGestureController.h:
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        (WebKit::ViewGestureController::resetState):
+        (WebKit::ViewGestureController::reset):
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::removeSwipeSnapshot):
+        (WebKit::ViewGestureController::resetState):
+        (WebKit::ViewGestureController::reset):
+
 2020-04-09  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, reverting r259804.

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h	2020-04-09 20:11:24 UTC (rev 259818)
@@ -45,6 +45,7 @@
 
 - (BOOL)_beginBackSwipeForTesting;
 - (BOOL)_completeBackSwipeForTesting;
+- (void)_resetNavigationGestureStateForTesting;
 - (void)_setDefersLoadingForTesting:(BOOL)defersLoading;
 
 - (void)_setShareSheetCompletesImmediatelyWithResolutionForTesting:(BOOL)resolved;

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm	2020-04-09 20:11:24 UTC (rev 259818)
@@ -143,6 +143,17 @@
 #endif
 }
 
+- (void)_resetNavigationGestureStateForTesting
+{
+#if PLATFORM(MAC)
+    if (auto gestureController = _impl->gestureController())
+        gestureController->reset();
+#else
+    if (_gestureController)
+        _gestureController->reset();
+#endif
+}
+
 - (void)_setDefersLoadingForTesting:(BOOL)defersLoading
 {
     _page->setDefersLoadingForTesting(defersLoading);

Modified: trunk/Source/WebKit/UIProcess/ViewGestureController.cpp (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/ViewGestureController.cpp	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/ViewGestureController.cpp	2020-04-09 20:11:24 UTC (rev 259818)
@@ -129,6 +129,8 @@
 
 void ViewGestureController::willBeginGesture(ViewGestureType type)
 {
+    LOG(ViewGestures, "ViewGestureController::willBeginGesture %d", (int)type);
+
     m_activeGestureType = type;
     m_currentGestureID = takeNextGestureID();
 }
@@ -135,6 +137,8 @@
 
 void ViewGestureController::didEndGesture()
 {
+    LOG(ViewGestures, "ViewGestureController::didEndGesture");
+
     m_activeGestureType = ViewGestureType::None;
     m_currentGestureID = 0;
 }
@@ -430,6 +434,8 @@
 
 bool ViewGestureController::PendingSwipeTracker::handleEvent(PlatformScrollEvent event)
 {
+    LOG(ViewGestures, "PendingSwipeTracker::handleEvent - state %d", (int)m_state);
+
     if (scrollEventCanEndSwipe(event)) {
         reset("gesture ended");
         return false;
@@ -436,6 +442,8 @@
     }
 
     if (m_state == State::None) {
+        LOG(ViewGestures, "PendingSwipeTracker::handleEvent - scroll can become swipe %d shouldIgnorePinnedState %d, page will handle scrolls %d", scrollEventCanBecomeSwipe(event, m_direction), m_shouldIgnorePinnedState, m_webPageProxy.willHandleHorizontalScrollEvents());
+
         if (!scrollEventCanBecomeSwipe(event, m_direction))
             return false;
 
@@ -453,10 +461,11 @@
 
 void ViewGestureController::PendingSwipeTracker::eventWasNotHandledByWebCore(PlatformScrollEvent event)
 {
+    LOG(ViewGestures, "Swipe Start Hysteresis - WebCore didn't handle event, state %d", (int)m_state);
+
     if (m_state != State::WaitingForWebCore)
         return;
 
-    LOG(ViewGestures, "Swipe Start Hysteresis - WebCore didn't handle event");
     m_state = State::None;
     m_cumulativeDelta = FloatSize();
     tryToStartSwipe(event);

Modified: trunk/Source/WebKit/UIProcess/ViewGestureController.h (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/ViewGestureController.h	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/ViewGestureController.h	2020-04-09 20:11:24 UTC (rev 259818)
@@ -162,6 +162,7 @@
     void checkForActiveLoads();
 
     void removeSwipeSnapshot();
+    void reset();
 
     void setSwipeGestureEnabled(bool enabled) { m_swipeGestureEnabled = enabled; }
     bool isSwipeGestureEnabled() { return m_swipeGestureEnabled; }
@@ -184,6 +185,7 @@
     static GestureID takeNextGestureID();
     void willBeginGesture(ViewGestureType);
     void didEndGesture();
+    void resetState();
 
     void didStartProvisionalOrSameDocumentLoadForMainFrame();

Modified: trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/ios/ViewGestureControllerIOS.mm	2020-04-09 20:11:24 UTC (rev 259818)
@@ -418,6 +418,11 @@
         return;
     }
 
+    resetState();
+}
+
+void ViewGestureController::resetState()
+{
     [m_snapshotView removeFromSuperview];
     m_snapshotView = nullptr;
     
@@ -433,6 +438,12 @@
     didEndGesture();
 }
 
+void ViewGestureController::reset()
+{
+    removeSwipeSnapshot();
+    resetState();
+}
+
 bool ViewGestureController::beginSimulatedSwipeInDirectionForTesting(SwipeDirection direction)
 {
     if (!canSwipeInDirection(direction))

Modified: trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm (259817 => 259818)


--- trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Source/WebKit/UIProcess/mac/ViewGestureControllerMac.mm	2020-04-09 20:11:24 UTC (rev 259818)
@@ -609,6 +609,11 @@
         return;
     }
 
+    resetState();
+}
+
+void ViewGestureController::resetState()
+{
     if (m_currentSwipeSnapshot)
         m_currentSwipeSnapshot->setVolatile(true);
     m_currentSwipeSnapshot = nullptr;
@@ -637,6 +642,13 @@
     didEndGesture();
 }
 
+void ViewGestureController::reset()
+{
+    removeSwipeSnapshot();
+    resetState();
+    m_swipeCancellationTracker = nil; // FIXME: Move to reset state()?
+}
+
 double ViewGestureController::magnification() const
 {
     if (m_activeGestureType == ViewGestureType::Magnification)

Modified: trunk/Tools/ChangeLog (259817 => 259818)


--- trunk/Tools/ChangeLog	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Tools/ChangeLog	2020-04-09 20:11:24 UTC (rev 259818)
@@ -1,3 +1,21 @@
+2020-04-09  Simon Fraser  <simon.fra...@apple.com>
+
+        Reset view navigation gesture state between tests
+        https://bugs.webkit.org/show_bug.cgi?id=210283
+
+        Reviewed by Tim Horton.
+
+        State in ViewGestureController could leak between tests if a test did not wait for the gesture to complete.
+        Specifically m_activeGestureType could be left as non-None.
+
+        Fix by plumbing a 'reset' through from TestController::resetStateToConsistentValues().
+
+        The implementations leverage code from removeSwipeSnapshot(), but avoid the fact that removeSwipeSnapshot()
+        early returns in various cases by just always calling reset code, which is factored into a new resetState function.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+
 2020-04-09  Aakash Jain  <aakash_j...@apple.com>
 
         [ews] Add unit tests to ensure that step names are valid identifier

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm (259817 => 259818)


--- trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2020-04-09 20:00:41 UTC (rev 259817)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2020-04-09 20:11:24 UTC (rev 259818)
@@ -277,6 +277,7 @@
         platformView._minimumEffectiveDeviceWidth = 0;
         [platformView _setContinuousSpellCheckingEnabledForTesting:options.shouldShowSpellCheckingDots];
         [platformView resetInteractionCallbacks];
+        [platformView _resetNavigationGestureStateForTesting];
     }
 
     [globalWebsiteDataStoreDelegateClient setAllowRaisingQuota:YES];

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to