Attachment 360036 Details for Bug 193779 – Patch

[patch] Patch

bug-193779-20190124140554.patch (text/plain), 10.10 KB, created by Chris Dumez on 2019-01-24 14:05:55 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-24 14:05:55 PST

Size: 10.10 KB

patch

obsolete

>Subversion Revision: 240443
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 034c6545c8969630194377b0f2d7e8ca56950d15..24f7d35cfa8cfcfeec23ecf8742bab527c7231cc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-24  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
>+        https://bugs.webkit.org/show_bug.cgi?id=193779
>+        <rdar://problem/46170903>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/Cocoa/NavigationState.mm:
>+        (WebKit::tryAppLink):
>+        (WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
>+        We were crashing when trying to get the URL of the main frame, which was sad because we never
>+        ended up using the main frame URL. Therefore, this patch drops the code in question.
>+
>+        * UIProcess/ProvisionalPageProxy.cpp:
>+        (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
>+        Add assertion to make sure that the DecidePolicyForNavigationActionAsync IPC it is getting
>+        from the process is related to its main frame.
>+
> 2019-01-24  Antti Koivisto  <antti@apple.com>
> 
>         [PSON] Flash on back navigation on Mac
>diff --git a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>index 285d66b784c3b258254c5e36c39bdd34414d33ee..1f732ca66583112de76742ba758cb2144395cd0f 100644
>--- a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>@@ -464,7 +464,7 @@ bool NavigationState::NavigationClient::willGoToBackForwardListItem(WebPageProxy
> }
> #endif
> 
>-static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const String& currentMainFrameURL, WTF::Function<void(bool)>&& completionHandler)
>+static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, WTF::Function<void(bool)>&& completionHandler)
> {
> #if HAVE(APP_LINKS)
>     if (!navigationAction->shouldOpenAppLinks()) {
>@@ -486,8 +486,6 @@ static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const Stri
> 
> void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageProxy& webPageProxy, Ref<API::NavigationAction>&& navigationAction, Ref<WebFramePolicyListenerProxy>&& listener, API::Object* userInfo)
> {
>-    ASSERT(webPageProxy.mainFrame());
>-    String mainFrameURLString = webPageProxy.mainFrame()->url();
>     bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame();
> 
>     if (!m_navigationState.m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionDecisionHandler
>@@ -521,7 +519,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP
> #endif
>             listener->ignore();
>         };
>-        tryAppLink(WTFMove(navigationAction), mainFrameURLString, WTFMove(completionHandler));
>+        tryAppLink(WTFMove(navigationAction), WTFMove(completionHandler));
>         return;
>     }
> 
>@@ -533,7 +531,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP
>     
>     auto checker = CompletionHandlerCallChecker::create(navigationDelegate.get(), delegateHasWebsitePolicies ? @selector(_webView:decidePolicyForNavigationAction:decisionHandler:) : @selector(webView:decidePolicyForNavigationAction:decisionHandler:));
>     
>-    auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), mainFrameURLString, webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
>+    auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
>         if (checker->completionHandlerHasBeenCalled())
>             return;
>         checker->didCallCompletionHandler();
>@@ -556,7 +554,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP
>         switch (actionPolicy) {
>         case WKNavigationActionPolicyAllow:
>         case _WKNavigationActionPolicyAllowInNewProcess:
>-            tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
>+            tryAppLink(WTFMove(navigationAction), [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
>                 if (followedLinkToApp) {
>                     localListener->ignore();
>                     return;
>diff --git a/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp b/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
>index e0fc58b81f89c8f541d00aeb61c3c9bc95ac6ab1..8d44829cda146ff1a29532c743f76f78b21c7efa 100644
>--- a/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
>@@ -264,6 +264,8 @@ void ProvisionalPageProxy::didChangeProvisionalURLForFrame(uint64_t frameID, uin
> 
> void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID)
> {
>+    ASSERT(m_mainFrame);
>+    ASSERT(m_mainFrame->frameID() == frameID);
>     m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID);
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 13073c5cf042a81372b99d84c831e165c94173d4..4dade385bf72a28e7328ae25b018f7d37e6dbd91 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-24  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
>+        https://bugs.webkit.org/show_bug.cgi?id=193779
>+        <rdar://problem/46170903>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test that quickly navigates forward to a previous process without waiting for it to
>+        suspend. I suspect the crash could have been happening due to receiving leftover IPC from
>+        the process' previous page when reconnecting the it for the forward navigation.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-01-24  Antti Koivisto  <antti@apple.com>
> 
>         [PSON] Flash on back navigation on Mac
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 2e1b4d2b78031b4089cbcd0f6ec3b8e7086a1c56..c1661879bea2aabca9d9917f2865836e53908628 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -2136,6 +2136,75 @@ TEST(ProcessSwap, HistoryItemIDConfusion)
>     EXPECT_WK_STREQ(@"pson://www.google.com/main.html", [backForwardList.forwardList[1].URL absoluteString]);
> }
> 
>+static const char* keepNavigatingFrameBytes = R"PSONRESOURCE(
>+<body>
>+<iframe id="testFrame1" src="about:blank"></iframe>
>+<iframe id="testFrame2" src="about:blank"></iframe>
>+<iframe id="testFrame3" src="about:blank"></iframe>
>+<script>
>+window.addEventListener('pagehide', function(event) {
>+    for (var j = 0; j < 10000; j++);
>+});
>+
>+var i = 0;
>+function navigateFrames()
>+{
>+    testFrame1.src = "pson://www.google.com/main" + i + ".html";
>+    testFrame2.src = "pson://www.google.com/main" + i + ".html";
>+    testFrame3.src = "pson://www.google.com/main" + i + ".html";
>+    i++;
>+}
>+
>+navigateFrames();
>+setInterval(() => {
>+    navigateFrames();
>+}, 0);
>+</script>
>+</body>
>+)PSONRESOURCE";
>+
>+TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation)
>+{
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    [processPoolConfiguration setProcessSwapsOnNavigation:YES];
>+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
>+
>+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [webViewConfiguration setProcessPool:processPool.get()];
>+    auto handler = adoptNS([[PSONScheme alloc] init]);
>+    [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:keepNavigatingFrameBytes];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:delegate.get()];
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto webkitPID = [webView _webProcessIdentifier];
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto applePID = [webView _webProcessIdentifier];
>+    EXPECT_NE(webkitPID, applePID);
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
>+}
>+
> static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE(
> <script>
> function loaded() {

Actions: View | Formatted Diff | Diff

Attachments on bug 193779: 360036