Attachment 356230 Details for Bug 192242 – Patch

[patch] Patch

bug-192242-20181130135659.patch (text/plain), 8.58 KB, created by Chris Dumez on 2018-11-30 13:57:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-11-30 13:57:01 PST

Size: 8.58 KB

patch

obsolete

>Subversion Revision: 238746
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0abaa4022a9a3f28c453869eeb5ffb1b79640132..f386b62c0c82f835c211f315ba6f7bbec98b67bd 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-11-30  Chris Dumez  <cdumez@apple.com>
>+
>+        [PSON] We are sometimes swapping processes even though there is an opened window with an opener link to us
>+        https://bugs.webkit.org/show_bug.cgi?id=192242
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Move the setting of the openedViaWindowOpenWithOpener & hasOpenedFrames flags on the
>+        NavigationAction from FrameLoader::loadURL(), to PolicyChecker::checkNavigationPolicy()
>+        to make sure those are always accurate and so that the UIProcess can make correct process
>+        swapping decisions.
>+
>+        NavigationAction objects are created in other places than FrameLoader::loadURL() as well.
>+        Even PolicyChecker::checkNavigationPolicy() will create a NavigationAction object if
>+        there is not already one.
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::loadURL):
>+        * loader/FrameLoader.h:
>+        (WebCore::FrameLoader::hasOpenedFrames const):
>+        * loader/PolicyChecker.cpp:
>+        (WebCore::PolicyChecker::checkNavigationPolicy):
>+
> 2018-11-30  Basuke Suzuki  <basuke.suzuki@sony.com>
> 
>         [Curl] Add API for ProtectionSpace.
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index b0d28a917e047de6f65220ac7d4a500e2eb12ac3..bb95ab737e4bd165160b77a18b7339368d622352 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -1358,9 +1358,6 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
>         return;
> 
>     NavigationAction action { frameLoadRequest.requester(), request, frameLoadRequest.initiatedByMainFrame(), newLoadType, isFormSubmission, event, frameLoadRequest.shouldOpenExternalURLsPolicy(), frameLoadRequest.downloadAttribute() };
>-    if (m_frame.page() && m_frame.page()->openedViaWindowOpenWithOpener())
>-        action.setOpenedViaWindowOpenWithOpener();
>-    action.setHasOpenedFrames(!m_openedFrames.isEmpty());
>     action.setLockHistory(lockHistory);
>     action.setLockBackForwardList(frameLoadRequest.lockBackForwardList());
> 
>diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
>index f858ef99e2297abbcfad13ed9c93494a4cf93128..f7be745cd3b4d58efacf2c5e70682f7c03ec2c69 100644
>--- a/Source/WebCore/loader/FrameLoader.h
>+++ b/Source/WebCore/loader/FrameLoader.h
>@@ -247,6 +247,7 @@ public:
> 
>     WEBCORE_EXPORT Frame* opener();
>     WEBCORE_EXPORT void setOpener(Frame*);
>+    bool hasOpenedFrames() const { return !m_openedFrames.isEmpty(); }
> 
>     void resetMultipleFormSubmissionProtection();
> 
>diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
>index 8302539cb377bcfc46ccf1d59e17fbf87813c800..b866272659a31fbd1347fe268af2fe1fdcc7949d 100644
>--- a/Source/WebCore/loader/PolicyChecker.cpp
>+++ b/Source/WebCore/loader/PolicyChecker.cpp
>@@ -106,6 +106,10 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, const Resou
>         loader->setTriggeringAction(NavigationAction { action });
>     }
> 
>+    if (m_frame.page() && m_frame.page()->openedViaWindowOpenWithOpener())
>+        action.setOpenedViaWindowOpenWithOpener();
>+    action.setHasOpenedFrames(m_frame.loader().hasOpenedFrames());
>+
>     // Don't ask more than once for the same request or if we are loading an empty URL.
>     // This avoids confusion on the part of the client.
>     if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) {
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 81899132c32699c85da2a9b98f087a79deeecf5b..ae15673773fde2ee28f335e1b4db178aff3ca4d0 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-11-30  Chris Dumez  <cdumez@apple.com>
>+
>+        [PSON] We are sometimes swapping processes even though there is an opened window with an opener link to us
>+        https://bugs.webkit.org/show_bug.cgi?id=192242
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2018-11-30  David Quesada  <david_quesada@apple.com>
> 
>         -[WKProcessPool _downloadURLRequest:] should allow specifying the initiating web view
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index f32fae510964deb6b6417e2bc9fdbca0b13cf942..fc9571d30506c7cb66950a655546b0df84f85886 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -343,6 +343,14 @@ window.onload = function() {
> </script>
> )PSONRESOURCE";
> 
>+static const char* windowOpenSameSiteWithOpenerTestBytes = R"PSONRESOURCE(
>+<script>
>+window.onload = function() {
>+    w = window.open("pson://www.webkit.org/main2.html");
>+}
>+</script>
>+)PSONRESOURCE";
>+
> static const char* windowOpenSameSiteNoOpenerTestBytes = R"PSONRESOURCE(
> <script>
> window.onload = function() {
>@@ -2901,6 +2909,74 @@ TEST(ProcessSwap, TerminateProcessAfterProcessSwap)
>     done = false;
> }
> 
>+TEST(ProcessSwap, NavigateCrossOriginWithOpenee)
>+{
>+    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:windowOpenSameSiteWithOpenerTestBytes]; // Opens "pson://www.webkit.org/main2.html".
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
>+    [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"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()];
>+    auto uiDelegate = adoptNS([[PSONUIDelegate alloc] initWithNavigationDelegate:navigationDelegate.get()]);
>+    [webView setUIDelegate:uiDelegate.get()];
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
>+
>+    [webView loadRequest:request];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    TestWebKitAPI::Util::run(&didCreateWebView);
>+    didCreateWebView = false;
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ([webView _webProcessIdentifier], [createdWebView _webProcessIdentifier]);
>+    auto webkitPID = [webView _webProcessIdentifier];
>+
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", [[webView URL] absoluteString]);
>+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main2.html", [[createdWebView URL] absoluteString]);
>+
>+    // New window should have an opener.
>+    [createdWebView evaluateJavaScript:@"window.opener ? 'true' : 'false'" completionHandler: [&] (id hasOpener, NSError *error) {
>+        EXPECT_WK_STREQ(@"true", hasOpener);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    // Navigate cross-origin.
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
>+
>+    [webView loadRequest:request];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
>+
>+    // Auxiliary window should still have an opener.
>+    [createdWebView evaluateJavaScript:@"window.opener ? 'true' : 'false'" completionHandler: [&] (id hasOpener, NSError *error) {
>+        EXPECT_WK_STREQ(@"true", hasOpener);
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    // We should not have process-swapped since an auxiliary window has an opener link to us.
>+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
>+}
>+
> TEST(ProcessSwap, GoBackToSuspendedPageWithMainFrameIDThatIsNotOne)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);

Actions: View | Formatted Diff | Diff

Attachments on bug 192242: 356230