Attachment 360698 Details for Bug 194083 – Updated the change log

[patch] Updated the change log

bug-194083-20190130234442.patch (text/plain), 7.99 KB, created by Ryosuke Niwa on 2019-01-30 23:44:43 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-01-30 23:44:43 PST

Size: 7.99 KB

patch

obsolete

>Subversion Revision: 240692
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 878066d54b74f27623b8945b0bd057ed7277d8a0..1af99de96ba61a64b2ff2112e5cdce892513f35d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-01-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        iOS: Crash in InteractiveUpdateHandler set by ViewGestureController::beginSwipeGesture
>+        https://bugs.webkit.org/show_bug.cgi?id=194083
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We think this crash is a regression from r236966. Prior to r236966, we could only called
>+        removeSwipeSnapshot() only when m_provisionalOrSameDocumentLoadCallback was set but now
>+        we can call it either when m_snapshotRemovalTracker::start was called, or it had been reset.
>+        This can result in m_webPageProxyForBackForwardListForCurrentSwipe getting cleared before
>+        InteractiveUpdateHandler is called by UIGestureRecognizer, resulting in the crash.
>+
>+        This patch tries to restore the behavior prior to r236966 by only invoking removeSwipeSnapshot()
>+        when SnapshotRemovalTracker has a valid removal callback set.
>+
>+        Unfortunately no new tests since there is no reproducible test case, and neither API tests
>+        nor layout tests seem to have the capability to trigger swipe gestures via UIGestureRecognizer,
>+        which is required for this crash to occur. Notably, back-forward swipe tests I enabled in
>+        r240765 bypass UIKit and emulates the action instead.
>+
>+        * UIProcess/Cocoa/ViewGestureController.cpp:
>+        (WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
>+        * UIProcess/Cocoa/ViewGestureController.h:
>+        (WebKit::ViewGestureController::SnapshotRemovalTracker::hasRemovalCallback const):
>+
> 2019-01-29  Youenn Fablet  <youenn@apple.com>
> 
>         Adopt new SPI to evaluate server certificate trust
>diff --git a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>index 2b7030b4ffcd9fbfe644689fe7c93f2c019b4d21..5b9391bfaf0ba9b1b689be8407c14bebe0920d76 100644
>--- a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>+++ b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.cpp
>@@ -188,7 +188,7 @@ void ViewGestureController::didRestoreScrollPosition()
> 
> void ViewGestureController::didReachMainFrameLoadTerminalState()
> {
>-    if (m_snapshotRemovalTracker.isPaused()) {
>+    if (m_snapshotRemovalTracker.isPaused() && m_snapshotRemovalTracker.hasRemovalCallback()) {
>         removeSwipeSnapshot();
>         return;
>     }
>diff --git a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>index f902376410955d2c5fd184e9427572ff640088ab..a18fd8fa45a7a2a74a4df746f146311674ee5109 100644
>--- a/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>+++ b/Source/WebKit/UIProcess/Cocoa/ViewGestureController.h
>@@ -179,6 +179,7 @@ private:
>         void pause() { m_paused = true; }
>         void resume();
>         bool isPaused() const { return m_paused; }
>+        bool hasRemovalCallback() const { return !!m_removalCallback; }
> 
>         bool eventOccurred(Events);
>         bool cancelOutstandingEvent(Events);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 88f1c2a469357d641ff6c1f4007d6f403455ea22..c0ff020857be08291fff36d0225b73d49d56e325 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Enable swipe tests on iOS
>+        https://bugs.webkit.org/show_bug.cgi?id=194076
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Removed the superflous call to prepareForAsyncTask and asyncTaskComplete as
>+        it causes a debug assertion failure in runUIScript.
>+
>+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::beginBackSwipe):
>+        (WTR::UIScriptController::completeBackSwipe):
>+
> 2019-01-29  Youenn Fablet  <youenn@apple.com>
> 
>         Adopt new SPI to evaluate server certificate trust
>diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>index 027bc375682bed78d1be72ad62f6e185a705ae9b..4952765a0acfd3de3f182be28e2fe8e19395683b 100644
>--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>@@ -822,18 +822,12 @@ void UIScriptController::beginBackSwipe(JSValueRef callback)
> {
>     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>     [webView _beginBackSwipeForTesting];
>-
>-    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
>-    m_context->asyncTaskComplete(callbackID);
> }
> 
> void UIScriptController::completeBackSwipe(JSValueRef callback)
> {
>     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>     [webView _completeBackSwipeForTesting];
>-
>-    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
>-    m_context->asyncTaskComplete(callbackID);
> }
> 
> static BOOL forEachViewInHierarchy(UIView *view, void(^mapFunction)(UIView *subview, BOOL *stop))
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 52873e6740e9fbafbaa9e84400e459a0554831c1..c60edeb1010f39352647d27a555d50eb226a0307 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Enable swipe tests on iOS
>+        https://bugs.webkit.org/show_bug.cgi?id=194076
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Enabled tests in LayoutTests/swipe and added iOS specific results for the failing tests.
>+
>+        * platform/ios/TestExpectations:
>+        * platform/ios/swipe/basic-cached-back-swipe-expected.txt: Added.
>+        * platform/ios/swipe/main-frame-pinning-requirement-expected.txt: Added.
>+        * platform/ios/swipe/pushState-cached-back-swipe-expected.txt: Added.
>+
> 2019-01-29  Youenn Fablet  <youenn@apple.com>
> 
>         Adopt new SPI to evaluate server certificate trust
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index 842db9b047649962bcb5f14cb27c8da4f7e30c81..25f027064458b728218602900fb03a5806627980 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -11,6 +11,7 @@ displaylists [ Pass ]
> http/tests/quicklook [ Pass ]
> media/ios [ Pass ]
> quicklook [ Pass ]
>+swipe [ Pass ]
> 
> fast/text-autosizing/ios [ Pass ]
> fast/zooming/ios [ Pass ]
>diff --git a/LayoutTests/platform/ios/swipe/basic-cached-back-swipe-expected.txt b/LayoutTests/platform/ios/swipe/basic-cached-back-swipe-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..56f14de2d01ebca82a835e29c89bf74fd81f8a45
>--- /dev/null
>+++ b/LayoutTests/platform/ios/swipe/basic-cached-back-swipe-expected.txt
>@@ -0,0 +1,6 @@
>+startSwipeGesture
>+didBeginSwipe
>+completeSwipeGesture
>+didEndSwipe
>+didRemoveSwipeSnapshot
>+
>diff --git a/LayoutTests/platform/ios/swipe/main-frame-pinning-requirement-expected.txt b/LayoutTests/platform/ios/swipe/main-frame-pinning-requirement-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..39b0d949ea3010b6f7a7cda953e6837ccbd3170c
>--- /dev/null
>+++ b/LayoutTests/platform/ios/swipe/main-frame-pinning-requirement-expected.txt
>@@ -0,0 +1,7 @@
>+startSwipeGesture
>+Failure. Should never begin a swipe, because we were in the middle of a scrolling gesture that started when the main frame was not pinned to the left.
>+didEndSwipe
>+completeSwipeGesture
>+startSwipeGesture
>+didRemoveSwipeSnapshot
>+
>diff --git a/LayoutTests/platform/ios/swipe/pushState-cached-back-swipe-expected.txt b/LayoutTests/platform/ios/swipe/pushState-cached-back-swipe-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..56f14de2d01ebca82a835e29c89bf74fd81f8a45
>--- /dev/null
>+++ b/LayoutTests/platform/ios/swipe/pushState-cached-back-swipe-expected.txt
>@@ -0,0 +1,6 @@
>+startSwipeGesture
>+didBeginSwipe
>+completeSwipeGesture
>+didEndSwipe
>+didRemoveSwipeSnapshot
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 194083: 360697 | 360698 | 360699