Attachment 360487 Details for Bug 193967 – Patch

[patch] Patch

bug-193967-20190129121505.patch (text/plain), 9.27 KB, created by Chris Dumez on 2019-01-29 12:15:06 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-29 12:15:06 PST

Size: 9.27 KB

patch

obsolete

>Subversion Revision: 240661
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 93c34b582d3abedada2a4381cf07956f1df6c8e9..b2879d9ab76606924c19e5125c14d1858e1c9fcc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-01-29  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression (r240046): [PSON] Spurious changes to [WKWebView url] and [WKWebView loading] after [WKWebView loadRequest]
>+        https://bugs.webkit.org/show_bug.cgi?id=193967
>+        <rdar://problem/47635348>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update WebPageProxy::receivedPolicyDecision() to no longer clear the pending URL when we send PolicyAction::Ignore to
>+        the old process when the load is continuing in a new process due to PSON.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
>+        (WebKit::WebPageProxy::receivedPolicyDecision):
>+        * UIProcess/WebPageProxy.h:
>+
> 2019-01-29  Chris Dumez  <cdumez@apple.com>
> 
>         Make sure WTF::generateObjectIdentifier() internal counter does not get duplicated
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 1b323fed261e5f00138bc7d9f056da9b11f1dcc4..c43ad934b7036a1e7f5f6e27f38adfa25498e250 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -2717,7 +2717,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
>             RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "decidePolicyForNavigationAction: keep using process %i for navigation, reason: %{public}s", processIdentifier(), reason.utf8().data());
> 
>         bool shouldProcessSwap = processForNavigation.ptr() != &process();
>-        receivedPolicyDecision(policyAction, navigation.ptr(), shouldProcessSwap ? WTF::nullopt : WTFMove(data), WTFMove(sender));
>+        receivedPolicyDecision(policyAction, navigation.ptr(), shouldProcessSwap ? WTF::nullopt : WTFMove(data), WTFMove(sender), shouldProcessSwap ? WillContinueLoadInNewProcess::Yes : WillContinueLoadInNewProcess::No);
> 
>         if (!shouldProcessSwap)
>             return;
>@@ -2730,7 +2730,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
>     });
> }
> 
>-void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, Optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender)
>+void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, Optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender, WillContinueLoadInNewProcess willContinueLoadInNewProcess)
> {
>     if (!isValid()) {
>         sender->send(PolicyAction::Ignore, 0, DownloadID(), WTF::nullopt);
>@@ -2739,7 +2739,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation*
> 
>     auto transaction = m_pageLoadState.transaction();
> 
>-    if (action == PolicyAction::Ignore)
>+    if (action == PolicyAction::Ignore && willContinueLoadInNewProcess == WillContinueLoadInNewProcess::No)
>         m_pageLoadState.clearPendingAPIRequestURL(transaction);
> 
>     DownloadID downloadID = { };
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index a5ec160591afc3a1720b72c84793c44434d1372b..29ee96559031989d35f2950f716f6bda5e5ff76d 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -972,7 +972,8 @@ public:
> #endif
> 
>     class PolicyDecisionSender;
>-    void receivedPolicyDecision(WebCore::PolicyAction, API::Navigation*, Optional<WebsitePoliciesData>&&, Ref<PolicyDecisionSender>&&);
>+    enum class WillContinueLoadInNewProcess : bool { No, Yes };
>+    void receivedPolicyDecision(WebCore::PolicyAction, API::Navigation*, Optional<WebsitePoliciesData>&&, Ref<PolicyDecisionSender>&&, WillContinueLoadInNewProcess = WillContinueLoadInNewProcess::No);
>     void receivedNavigationPolicyDecision(WebCore::PolicyAction, API::Navigation*, ProcessSwapRequestedByClient, WebFrameProxy&, API::WebsitePolicies*, Ref<PolicyDecisionSender>&&);
> 
>     void backForwardRemovedItem(const WebCore::BackForwardItemIdentifier&);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index a9a8c059f90f39226300b27b60ebb8ed4ef8b6a9..7f602342017af90828582dcbeaa5e4866be7aa79 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-01-29  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression (r240046): [PSON] Spurious changes to [WKWebView url] and [WKWebView loading] after [WKWebView loadRequest]
>+        https://bugs.webkit.org/show_bug.cgi?id=193967
>+        <rdar://problem/47635348>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+        (-[PSONLoadingObserver observeValueForKeyPath:ofObject:change:context:]):
>+
> 2019-01-29  Chris Dumez  <cdumez@apple.com>
> 
>         REGRESSION (PSON): Twitter link gets stuck at t.co after navigating back in tab
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index d5dc35bb7c1f8cdc6de50141ee15ea0b17f58a8d..db7c056a14983bec16c1b8cef4a5deead478a960 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -3464,6 +3464,98 @@ TEST(ProcessSwap, ClosePageAfterCrossSiteProvisionalLoad)
>     TestWebKitAPI::Util::sleep(0.5);
> }
> 
>+static Vector<bool> loadingStateChanges;
>+static unsigned urlChangeCount;
>+
>+@interface PSONLoadingObserver : NSObject
>+@end
>+
>+@implementation PSONLoadingObserver
>+
>+-(void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
>+{
>+    WKWebView *webView = (WKWebView *)context;
>+
>+    if ([keyPath isEqualToString:@"loading"])
>+        loadingStateChanges.append([webView isLoading]);
>+    else if ([keyPath isEqualToString:@"URL"])
>+        ++urlChangeCount;
>+
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+}
>+
>+@end
>+
>+TEST(ProcessSwap, LoadingStateAfterPolicyDecision)
>+{
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    processPoolConfiguration.get().processSwapsOnNavigation = 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]);
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView setNavigationDelegate:navigationDelegate.get()];
>+
>+    [webView configuration].preferences.safeBrowsingEnabled = NO;
>+
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]]];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto loadObserver = adoptNS([[PSONLoadingObserver alloc] init]);
>+    [webView addObserver:loadObserver.get() forKeyPath:@"loading" options:0 context:webView.get()];
>+    [webView addObserver:loadObserver.get() forKeyPath:@"URL" options:0 context:webView.get()];
>+
>+    urlChangeCount = 0;
>+    didStartProvisionalLoad = false;
>+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]]];
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+    EXPECT_TRUE([webView isLoading]);
>+
>+    EXPECT_EQ(1U, loadingStateChanges.size());
>+    EXPECT_TRUE(loadingStateChanges[0]);
>+    EXPECT_EQ(1U, urlChangeCount);
>+
>+    navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
>+        EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+        EXPECT_TRUE([webView isLoading]);
>+
>+        decisionHandler(WKNavigationActionPolicyAllow);
>+
>+        EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+        EXPECT_TRUE([webView isLoading]);
>+    };
>+
>+    EXPECT_EQ(1U, loadingStateChanges.size());
>+
>+    TestWebKitAPI::Util::run(&didStartProvisionalLoad);
>+    didStartProvisionalLoad = false;
>+
>+    EXPECT_EQ(1U, loadingStateChanges.size());
>+    EXPECT_EQ(1U, urlChangeCount);
>+
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+    EXPECT_TRUE([webView isLoading]);
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+    EXPECT_FALSE([webView isLoading]);
>+
>+    EXPECT_EQ(2U, loadingStateChanges.size());
>+    EXPECT_FALSE(loadingStateChanges[1]);
>+    EXPECT_EQ(1U, urlChangeCount);
>+
>+    [webView removeObserver:loadObserver.get() forKeyPath:@"loading" context:webView.get()];
>+    [webView removeObserver:loadObserver.get() forKeyPath:@"URL" context:webView.get()];
>+}
>+
> #if PLATFORM(MAC)
> 
> static const char* saveOpenerTestBytes = R"PSONRESOURCE(

Actions: View | Formatted Diff | Diff

Attachments on bug 193967: 360473 | 360487