Attachment 349902 Details for Bug 189667 – Patch

[patch] Patch

bug-189667-20180917104637.patch (text/plain), 5.46 KB, created by Tim Horton on 2018-09-17 10:46:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2018-09-17 10:46:37 PDT

Size: 5.46 KB

patch

obsolete

>Subversion Revision: 236034
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ad6ed539f7fc1145430aebe4d742be32aaefbbfa..1ac3fcc246af47840d15b9384fae6e7a5fe833dd 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2018-09-16  Tim Horton  <timothy_horton@apple.com>
>+
>+        Swipe snapshot can get stuck if swiping is disabled while it is visible
>+        https://bugs.webkit.org/show_bug.cgi?id=189667
>+        <rdar://problem/40367780>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        If navigation gestures are disabled while a swipe snapshot is visible,
>+        WKWebView will tear down the ViewGestureController, which means that
>+        the SnapshotRemovalTracker will no longer be around to ever remove
>+        the snapshot.
>+
>+        It's currently very hard to write a test for this because we have
>+        yet to come up with a good mechanism for testing swiping on iOS.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView setAllowsBackForwardNavigationGestures:]):
>+        Instead of tearing down the ViewGestureController when navigation
>+        gestures are disabled, just set a bit on it that disables gestures.
>+
>+        * UIProcess/Cocoa/ViewGestureController.cpp:
>+        (WebKit::ViewGestureController::canSwipeInDirection const):
>+        * UIProcess/Cocoa/ViewGestureController.h:
>+        (WebKit::ViewGestureController::setSwipeGestureEnabled):
>+        (WebKit::ViewGestureController::isSwipeGestureEnabled):
>+        Add a bit to ViewGestureController that makes starting new gestures
>+        always fail, but allows e.g. snapshots from existing swipes to continue
>+        their usual behavior.
>+
> 2018-09-08  Darin Adler  <darin@apple.com>
> 
>         Streamline JSRetainPtr, fix leaks of JSString and JSGlobalContext
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 31c9516d492403a37cac094050be44fb6098a54c..75315e7124b2ce539c7e3d9211842dd577a4b4ea 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -3089,15 +3089,15 @@ - (void)setAllowsBackForwardNavigationGestures:(BOOL)allowsBackForwardNavigation
> 
>     _allowsBackForwardNavigationGestures = allowsBackForwardNavigationGestures;
> 
>-    if (allowsBackForwardNavigationGestures) {
>-        if (!_gestureController) {
>-            _gestureController = std::make_unique<WebKit::ViewGestureController>(*_page);
>-            _gestureController->installSwipeHandler(self, [self scrollView]);
>-            if (WKWebView *alternateWebView = [_configuration _alternateWebViewForNavigationGestures])
>-                _gestureController->setAlternateBackForwardListSourcePage(alternateWebView->_page.get());
>-        }
>-    } else
>-        _gestureController = nullptr;
>+    if (allowsBackForwardNavigationGestures && !_gestureController) {
>+        _gestureController = std::make_unique<WebKit::ViewGestureController>(*_page);
>+        _gestureController->installSwipeHandler(self, [self scrollView]);
>+        if (WKWebView *alternateWebView = [_configuration _alternateWebViewForNavigationGestures])
>+            _gestureController->setAlternateBackForwardListSourcePage(alternateWebView->_page.get());
>+    }
>+
>+    if (_gestureController)
>+        _gestureController->setSwipeGestureEnabled(allowsBackForwardNavigationGestures);
> 
>     _page->setShouldRecordNavigationSnapshots(allowsBackForwardNavigationGestures);
> }
>diff --git a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>index f97730a880135167b3bb16d2744fbad4c6d0863f..cb346da0c832b4fe06bcc4aa47dce0bd92cf2a05 100644
>--- a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>+++ b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>@@ -112,10 +112,14 @@ void ViewGestureController::setAlternateBackForwardListSourcePage(WebPageProxy*
>     
> bool ViewGestureController::canSwipeInDirection(SwipeDirection direction) const
> {
>+    if (!m_swipeGestureEnabled)
>+        return false;
>+
> #if ENABLE(FULLSCREEN_API)
>     if (m_webPageProxy.fullScreenManager() && m_webPageProxy.fullScreenManager()->isFullScreen())
>         return false;
> #endif
>+
>     RefPtr<WebPageProxy> alternateBackForwardListSourcePage = m_alternateBackForwardListSourcePage.get();
>     auto& backForwardList = alternateBackForwardListSourcePage ? alternateBackForwardListSourcePage->backForwardList() : m_webPageProxy.backForwardList();
>     if (direction == SwipeDirection::Back)
>diff --git a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>index 8311a294830bf92e0f9b64452c928df43bfe75f3..1054b0676352c418c91f2f93000c232ca8f5b981 100644
>--- a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>+++ b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>@@ -136,6 +136,9 @@ public:
> 
>     void removeSwipeSnapshot();
> 
>+    void setSwipeGestureEnabled(bool enabled) { m_swipeGestureEnabled = enabled; }
>+    bool isSwipeGestureEnabled() { return m_swipeGestureEnabled; }
>+
>     // Testing
>     bool beginSimulatedSwipeInDirectionForTesting(SwipeDirection);
>     bool completeSimulatedSwipeInDirectionForTesting(SwipeDirection);
>@@ -247,6 +250,8 @@ private:
>     WebPageProxy& m_webPageProxy;
>     ViewGestureType m_activeGestureType { ViewGestureType::None };
> 
>+    bool m_swipeGestureEnabled { true };
>+
>     RunLoop::Timer<ViewGestureController> m_swipeActiveLoadMonitoringTimer;
> 
>     WebCore::Color m_backgroundColorForCurrentSnapshot;

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189667: 349900 | 349901 | 349902