- Revision
- 238755
- Author
- [email protected]
- Date
- 2018-11-30 14:27:18 -0800 (Fri, 30 Nov 2018)
Log Message
[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 Geoffrey Garen.
Source/WebCore:
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):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (238754 => 238755)
--- trunk/Source/WebCore/ChangeLog 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Source/WebCore/ChangeLog 2018-11-30 22:27:18 UTC (rev 238755)
@@ -1,3 +1,26 @@
+2018-11-30 Chris Dumez <[email protected]>
+
+ [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 Geoffrey Garen.
+
+ 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 Don Olmstead <[email protected]>
Rename ENABLE_SUBTLE_CRYPTO to ENABLE_WEB_CRYPTO
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (238754 => 238755)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-11-30 22:27:18 UTC (rev 238755)
@@ -1358,9 +1358,6 @@
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());
Modified: trunk/Source/WebCore/loader/FrameLoader.h (238754 => 238755)
--- trunk/Source/WebCore/loader/FrameLoader.h 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2018-11-30 22:27:18 UTC (rev 238755)
@@ -247,6 +247,7 @@
WEBCORE_EXPORT Frame* opener();
WEBCORE_EXPORT void setOpener(Frame*);
+ bool hasOpenedFrames() const { return !m_openedFrames.isEmpty(); }
void resetMultipleFormSubmissionProtection();
Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (238754 => 238755)
--- trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-11-30 22:27:18 UTC (rev 238755)
@@ -106,6 +106,10 @@
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())) {
Modified: trunk/Tools/ChangeLog (238754 => 238755)
--- trunk/Tools/ChangeLog 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Tools/ChangeLog 2018-11-30 22:27:18 UTC (rev 238755)
@@ -1,3 +1,14 @@
+2018-11-30 Chris Dumez <[email protected]>
+
+ [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 Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-11-30 Don Olmstead <[email protected]>
Rename ENABLE_SUBTLE_CRYPTO to ENABLE_WEB_CRYPTO
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (238754 => 238755)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-30 22:23:17 UTC (rev 238754)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-30 22:27:18 UTC (rev 238755)
@@ -343,6 +343,14 @@
</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 @@
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]);
