Diff
Modified: trunk/Source/WebKit/ChangeLog (238825 => 238826)
--- trunk/Source/WebKit/ChangeLog 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Source/WebKit/ChangeLog 2018-12-03 23:57:41 UTC (rev 238826)
@@ -1,3 +1,30 @@
+2018-12-03 Chris Dumez <[email protected]>
+
+ [PSON] Request by the client to process-swap is ignored if the window has an opener
+ https://bugs.webkit.org/show_bug.cgi?id=192267
+ <rdar://problem/46386886>
+
+ Reviewed by Brady Eidson.
+
+ If the client forces a process-swap, we should process-swap, even if the browsing
+ context has an opener (or openees). Previously, we would only bypass the cross-site
+ check, not the openee / openees checks.
+
+ The issue when doing so is that the openee still has a opener link to the window
+ in the old process. They would see that the window is still there and navigated
+ to 'about:blank' due to our page suspension logic. To address the issue, we no
+ longer suspend the old WebPage if the process swap was forced by the client and
+ we make sure that the old WebPage gets closed.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+ (WebKit::WebPageProxy::swapToWebProcess):
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+ (WebKit::WebPageProxy::continueNavigationInNewProcess):
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::processForNavigationInternal):
+
2018-12-03 Keith Rollin <[email protected]>
Add .xcfilelist files
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (238825 => 238826)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-03 23:57:41 UTC (rev 238826)
@@ -737,11 +737,15 @@
finishAttachingToWebProcess();
}
-bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, std::optional<uint64_t> mainFrameID)
+bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, std::optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient processSwapRequestedByClient)
{
if (!mainFrameID)
return false;
+ // If the client forced a swap then it may not be Web-compatible to suspend the previous page because other windows may have an opener link to the page.
+ if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes)
+ return false;
+
auto* currentItem = navigation.fromItem();
if (!currentItem) {
LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " unable to create suspended page for process pid %i - No current back/forward item", pageID(), m_process->processIdentifier());
@@ -756,7 +760,7 @@
return true;
}
-void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation& navigation, std::optional<uint64_t> mainFrameIDInPreviousProcess, CompletionHandler<void()>&& completionHandler)
+void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation& navigation, std::optional<uint64_t> mainFrameIDInPreviousProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, CompletionHandler<void(bool)>&& completionHandler)
{
ASSERT(!m_isClosed);
RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::swapToWebProcess\n", this);
@@ -770,11 +774,11 @@
}
auto* destinationSuspendedPagePtr = destinationSuspendedPage.get();
- auto doSwap = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), process = WTFMove(process), mainFrameIDInPreviousProcess, destinationSuspendedPage = WTFMove(destinationSuspendedPage), completionHandler = WTFMove(completionHandler)]() mutable {
+ auto doSwap = [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), process = WTFMove(process), mainFrameIDInPreviousProcess, processSwapRequestedByClient, destinationSuspendedPage = WTFMove(destinationSuspendedPage), completionHandler = WTFMove(completionHandler)]() mutable {
processDidTerminate(ProcessTerminationReason::NavigationSwap);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
- bool didSuspendPreviousPage = suspendCurrentPageIfPossible(navigation, mainFrameIDInPreviousProcess);
+ bool didSuspendPreviousPage = suspendCurrentPageIfPossible(navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient);
m_process->removeWebPage(*this, m_pageID);
m_process = WTFMove(process);
@@ -792,7 +796,7 @@
}
finishAttachingToWebProcess(didSuspendPreviousPage ? ShouldDelayAttachingDrawingArea::Yes : ShouldDelayAttachingDrawingArea::No);
- completionHandler();
+ completionHandler(didSuspendPreviousPage);
};
if (destinationSuspendedPagePtr)
@@ -2623,7 +2627,7 @@
receivedPolicyDecision(policyAction, navigation, WTFMove(data), WTFMove(sender));
if (processForNavigation.ptr() != &process())
- continueNavigationInNewProcess(*navigation, WTFMove(processForNavigation));
+ continueNavigationInNewProcess(*navigation, WTFMove(processForNavigation), processSwapRequestedByClient);
}
void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender)
@@ -2658,7 +2662,7 @@
sender->send(action, navigation ? navigation->navigationID() : 0, downloadID, WTFMove(websitePolicies));
}
-void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, Ref<WebProcessProxy>&& process)
+void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, Ref<WebProcessProxy>&& process, ProcessSwapRequestedByClient processSwapRequestedByClient)
{
LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
@@ -2668,7 +2672,7 @@
ASSERT(m_process.ptr() != process.ptr());
- swapToWebProcess(WTFMove(process), navigation, mainFrameIDInPreviousProcess, [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess, mainFrameURL = WTFMove(mainFrameURL)]() mutable {
+ swapToWebProcess(WTFMove(process), navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient, [this, protectedThis = makeRef(*this), navigation = makeRef(navigation), previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess, mainFrameURL = WTFMove(mainFrameURL)](bool didSuspendPreviousPage) mutable {
if (auto* item = navigation->targetItem()) {
LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
@@ -2726,8 +2730,12 @@
};
bool isInitialNavigationInNewWindow = openedByDOM() && !hasCommittedAnyProvisionalLoads();
- if (!isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess)
+ if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess) {
+ // There is no way we'll be able to return to the page in the previous page so close it.
+ if (!didSuspendPreviousPage)
+ previousProcess->send(Messages::WebPage::Close(), m_pageID);
return;
+ }
m_mainFrameWindowCreationHandler = [this, previousProcess = WTFMove(previousProcess), mainFrameIDInPreviousProcess = *mainFrameIDInPreviousProcess](const GlobalWindowIdentifier& windowIdentifier) {
ASSERT(m_mainFrame);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (238825 => 238826)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2018-12-03 23:57:41 UTC (rev 238826)
@@ -1411,7 +1411,7 @@
void updateThrottleState();
void updateHiddenPageThrottlingAutoIncreases();
- bool suspendCurrentPageIfPossible(API::Navigation&, std::optional<uint64_t> mainFrameID);
+ bool suspendCurrentPageIfPossible(API::Navigation&, std::optional<uint64_t> mainFrameID, ProcessSwapRequestedByClient);
enum class ResetStateReason {
PageInvalidated,
@@ -1559,7 +1559,7 @@
void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
void reattachToWebProcess();
- void swapToWebProcess(Ref<WebProcessProxy>&&, API::Navigation&, std::optional<uint64_t> mainFrameIDInPreviousProcess, CompletionHandler<void()>&&);
+ void swapToWebProcess(Ref<WebProcessProxy>&&, API::Navigation&, std::optional<uint64_t> mainFrameIDInPreviousProcess, ProcessSwapRequestedByClient, CompletionHandler<void(bool)>&&);
void finishAttachingToWebProcess(ShouldDelayAttachingDrawingArea = ShouldDelayAttachingDrawingArea::No);
@@ -1878,7 +1878,7 @@
void reportPageLoadResult(const WebCore::ResourceError& = { });
- void continueNavigationInNewProcess(API::Navigation&, Ref<WebProcessProxy>&&);
+ void continueNavigationInNewProcess(API::Navigation&, Ref<WebProcessProxy>&&, ProcessSwapRequestedByClient);
void setNeedsFontAttributes(bool);
void updateFontAttributesAfterEditorStateChange();
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (238825 => 238826)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-12-03 23:57:41 UTC (rev 238826)
@@ -2138,7 +2138,21 @@
{
auto& targetURL = navigation.currentRequest().url();
- if (!m_configuration->processSwapsOnNavigation() && processSwapRequestedByClient == ProcessSwapRequestedByClient::No) {
+ auto createNewProcess = [&] () -> Ref<WebProcessProxy> {
+ if (RefPtr<WebProcessProxy> process = tryTakePrewarmedProcess(page.websiteDataStore())) {
+ tryPrewarmWithDomainInformation(*process, targetURL);
+ return process.releaseNonNull();
+ }
+
+ return createNewWebProcess(page.websiteDataStore());
+ };
+
+ if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes) {
+ reason = "Process swap was requested by the client"_s;
+ return createNewProcess();
+ }
+
+ if (!m_configuration->processSwapsOnNavigation()) {
reason = "Feature is disabled"_s;
return page.process();
}
@@ -2186,31 +2200,28 @@
}
}
- if (processSwapRequestedByClient == ProcessSwapRequestedByClient::No) {
- if (navigation.treatAsSameOriginNavigation()) {
- reason = "The treatAsSameOriginNavigation flag is set"_s;
- return page.process();
- }
+ if (navigation.treatAsSameOriginNavigation()) {
+ reason = "The treatAsSameOriginNavigation flag is set"_s;
+ return page.process();
+ }
- bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();
- URL sourceURL;
- if (isInitialLoadInNewWindowOpenedByDOM && !navigation.requesterOrigin().isEmpty())
- sourceURL = URL { URL(), navigation.requesterOrigin().toString() };
- else
- sourceURL = URL { { }, page.pageLoadState().url() };
+ bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();
+ URL sourceURL;
+ if (isInitialLoadInNewWindowOpenedByDOM && !navigation.requesterOrigin().isEmpty())
+ sourceURL = URL { URL(), navigation.requesterOrigin().toString() };
+ else
+ sourceURL = URL { { }, page.pageLoadState().url() };
- if (sourceURL.isEmpty() && page.configuration().relatedPage()) {
- sourceURL = URL { { }, page.configuration().relatedPage()->pageLoadState().url() };
- RELEASE_LOG(ProcessSwapping, "Using related page %p's URL as source URL for process swap decision", page.configuration().relatedPage());
- }
+ if (sourceURL.isEmpty() && page.configuration().relatedPage()) {
+ sourceURL = URL { { }, page.configuration().relatedPage()->pageLoadState().url() };
+ RELEASE_LOG(ProcessSwapping, "Using related page %p's URL as source URL for process swap decision", page.configuration().relatedPage());
+ }
- if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.protocolIsAbout() || registrableDomainsAreEqual(sourceURL, targetURL)) {
- reason = "Navigation is same-site"_s;
- return page.process();
- }
- reason = "Navigation is cross-site"_s;
- } else
- reason = "Process swap was requested by the client"_s;
+ if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.protocolIsAbout() || registrableDomainsAreEqual(sourceURL, targetURL)) {
+ reason = "Navigation is same-site"_s;
+ return page.process();
+ }
+ reason = "Navigation is cross-site"_s;
auto registrableDomain = toRegistrableDomain(targetURL);
if (m_configuration->alwaysKeepAndReuseSwappedProcesses()) {
@@ -2251,12 +2262,7 @@
return process;
}
- if (RefPtr<WebProcessProxy> process = tryTakePrewarmedProcess(page.websiteDataStore())) {
- tryPrewarmWithDomainInformation(*process, targetURL);
- return process.releaseNonNull();
- }
-
- return createNewWebProcess(page.websiteDataStore());
+ return createNewProcess();
}
void WebProcessPool::addSuspendedPage(std::unique_ptr<SuspendedPageProxy>&& suspendedPage)
Modified: trunk/Tools/ChangeLog (238825 => 238826)
--- trunk/Tools/ChangeLog 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Tools/ChangeLog 2018-12-03 23:57:41 UTC (rev 238826)
@@ -1,3 +1,15 @@
+2018-12-03 Chris Dumez <[email protected]>
+
+ [PSON] Request by the client to process-swap is ignored if the window has an opener
+ https://bugs.webkit.org/show_bug.cgi?id=192267
+ <rdar://problem/46386886>
+
+ Reviewed by Brady Eidson.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-12-03 Keith Rollin <[email protected]>
Add .xcfilelist files
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (238825 => 238826)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-12-03 23:16:25 UTC (rev 238825)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-12-03 23:57:41 UTC (rev 238826)
@@ -2601,6 +2601,141 @@
EXPECT_NE(pid1, pid3);
}
+#if PLATFORM(MAC)
+
+static const char* saveOpenerTestBytes = R"PSONRESOURCE(
+<script>
+window._onload_ = function() {
+ savedOpener = opener;
+}
+</script>
+)PSONRESOURCE";
+
+TEST(ProcessSwap, OpenerLinkAfterAPIControlledProcessSwappingOfOpener)
+{
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:windowOpenSameSiteWithOpenerTestBytes]; // Opens "pson://www.webkit.org/main2.html".
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main2.html" toData:saveOpenerTestBytes];
+ [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()];
+
+ auto uiDelegate = adoptNS([[PSONUIDelegate alloc] initWithNavigationDelegate:navigationDelegate.get()]);
+ [webView setUIDelegate:uiDelegate.get()];
+
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]]];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+ auto pid1 = [webView _webProcessIdentifier];
+
+ TestWebKitAPI::Util::run(&didCreateWebView);
+ didCreateWebView = false;
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_EQ(pid1, [createdWebView _webProcessIdentifier]);
+
+ // Auxiliary window should have an opener.
+ [createdWebView evaluateJavaScript:@"window.opener.location.href" completionHandler: [&] (id hasOpener, NSError *error) {
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", hasOpener);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We force a proces-swap via client API.
+ navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+ decisionHandler(_WKNavigationActionPolicyAllowInNewProcess);
+ };
+
+ // Navigating from the above URL to this URL normally should not process swap.
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main3.html"]]];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [webView _webProcessIdentifier];
+ EXPECT_NE(pid1, pid2);
+
+ // Auxiliary window's opener should no longer have an opener.
+ [createdWebView evaluateJavaScript:@"window.opener ? 'true' : 'false'" completionHandler: [&] (id hasOpener, NSError *error) {
+ EXPECT_WK_STREQ(@"false", hasOpener);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ [createdWebView evaluateJavaScript:@"savedOpener.closed ? 'true' : 'false'" completionHandler: [&] (id savedOpenerIsClosed, NSError *error) {
+ EXPECT_WK_STREQ(@"true", savedOpenerIsClosed);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+}
+
+TEST(ProcessSwap, OpenerLinkAfterAPIControlledProcessSwappingOfOpenee)
+{
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:windowOpenSameSiteWithOpenerTestBytes]; // Opens "pson://www.webkit.org/main2.html".
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main2.html" toData:saveOpenerTestBytes];
+ [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()];
+
+ auto uiDelegate = adoptNS([[PSONUIDelegate alloc] initWithNavigationDelegate:navigationDelegate.get()]);
+ [webView setUIDelegate:uiDelegate.get()];
+
+ [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]]];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+ auto pid1 = [webView _webProcessIdentifier];
+
+ TestWebKitAPI::Util::run(&didCreateWebView);
+ didCreateWebView = false;
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_EQ(pid1, [createdWebView _webProcessIdentifier]);
+
+ // Auxiliary window should have an opener.
+ [webView evaluateJavaScript:@"w.opener.location.href" completionHandler: [&] (id hasOpener, NSError *error) {
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", hasOpener);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We force a proces-swap via client API.
+ navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+ decisionHandler(_WKNavigationActionPolicyAllowInNewProcess);
+ };
+
+ // Navigating from the above URL to this URL normally should not process swap.
+ [createdWebView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main3.html"]]];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [createdWebView _webProcessIdentifier];
+ EXPECT_NE(pid1, pid2);
+
+ // Auxiliary window's opener should no longer have an opener.
+ [webView evaluateJavaScript:@"w.opener ? 'true' : 'false'" completionHandler: [&] (id hasOpener, NSError *error) {
+ EXPECT_WK_STREQ(@"false", hasOpener);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+}
+
+#endif // PLATFORM(MAC)
+
enum class ExpectSwap { No, Yes };
static void runProcessSwapDueToRelatedWebViewTest(NSURL* relatedViewURL, NSURL* targetURL, ExpectSwap expectSwap)
{
