Diff
Modified: trunk/Source/WebKit/ChangeLog (237698 => 237699)
--- trunk/Source/WebKit/ChangeLog 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Source/WebKit/ChangeLog 2018-11-01 20:53:50 UTC (rev 237699)
@@ -1,5 +1,23 @@
2018-11-01 Chris Dumez <[email protected]>
+ [PSON] WebPageProxy::receivedNavigationPolicyDecision() should not schedule the new load asynchronously when process-swapping
+ https://bugs.webkit.org/show_bug.cgi?id=191076
+
+ Reviewed by Geoffrey Garen.
+
+ WebPageProxy::receivedNavigationPolicyDecision() should not schedule the new load asynchronously when process-swapping.
+ The client can request a new load synchronously after answering the policy decision, in which case we'd end up loading
+ the wrong URL.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::processForNavigation):
+ (WebKit::WebProcessPool::processForNavigationInternal):
+ * UIProcess/WebProcessPool.h:
+
+2018-11-01 Chris Dumez <[email protected]>
+
[PSON] Unable to submit a file in FormData cross-site
https://bugs.webkit.org/show_bug.cgi?id=191138
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (237698 => 237699)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-11-01 20:53:50 UTC (rev 237699)
@@ -2496,25 +2496,24 @@
if (policies->websiteDataStore())
changeWebsiteDataStore(policies->websiteDataStore()->websiteDataStore());
}
-
+
+ Ref<WebProcessProxy> processForNavigation = process();
if (policyAction == PolicyAction::Use && frame.isMainFrame()) {
String reason;
- auto proposedProcess = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, policyAction, reason);
+ processForNavigation = process().processPool().processForNavigation(*this, *navigation, processSwapRequestedByClient, reason);
ASSERT(!reason.isNull());
-
- if (proposedProcess.ptr() != &process()) {
- RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), proposedProcess->processIdentifier(), reason.utf8().data());
- LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), proposedProcess->processIdentifier(), navigation->navigationID(), navigation->loggingString());
-
- RunLoop::main().dispatch([this, protectedThis = makeRef(*this), navigation = makeRef(*navigation), proposedProcess = WTFMove(proposedProcess)]() mutable {
- continueNavigationInNewProcess(navigation, WTFMove(proposedProcess));
- });
+ if (processForNavigation.ptr() != &process()) {
+ policyAction = PolicyAction::Suspend;
+ RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), processForNavigation->processIdentifier(), reason.utf8().data());
+ LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), processForNavigation->processIdentifier(), navigation->navigationID(), navigation->loggingString());
} else
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
}
-
+
receivedPolicyDecision(policyAction, navigation, WTFMove(data), WTFMove(sender));
+ if (processForNavigation.ptr() != &process())
+ continueNavigationInNewProcess(*navigation, WTFMove(processForNavigation));
}
void WebPageProxy::receivedPolicyDecision(PolicyAction action, API::Navigation* navigation, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<PolicyDecisionSender>&& sender)
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (237698 => 237699)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-01 20:53:50 UTC (rev 237699)
@@ -2071,9 +2071,9 @@
m_swappedProcessesPerRegistrableDomain.remove(registrableDomain);
}
-Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action, String& reason)
+Ref<WebProcessProxy> WebProcessPool::processForNavigation(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, String& reason)
{
- auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, action, reason);
+ auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, reason);
if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
static std::once_flag onceFlag;
@@ -2089,7 +2089,7 @@
return process;
}
-Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, PolicyAction& action, String& reason)
+Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API::Navigation& navigation, ProcessSwapRequestedByClient processSwapRequestedByClient, String& reason)
{
auto& targetURL = navigation.currentRequest().url();
@@ -2135,7 +2135,6 @@
if (auto* backForwardListItem = navigation.targetItem()) {
if (auto* suspendedPage = backForwardListItem->suspendedPage()) {
- action = ""
reason = "Using target back/forward item's process"_s;
return suspendedPage->process();
}
@@ -2198,8 +2197,6 @@
}
}
- action = ""
-
if (RefPtr<WebProcessProxy> process = tryTakePrewarmedProcess(page.websiteDataStore())) {
tryPrewarmWithDomainInformation(*process, targetURL);
return process.releaseNonNull();
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (237698 => 237699)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2018-11-01 20:53:50 UTC (rev 237699)
@@ -440,7 +440,7 @@
BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
#endif
- Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&, String& reason);
+ Ref<WebProcessProxy> processForNavigation(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, String& reason);
// SuspendedPageProxy management.
void addSuspendedPageProxy(std::unique_ptr<SuspendedPageProxy>&&);
@@ -472,7 +472,7 @@
void platformInitializeWebProcess(WebProcessCreationParameters&);
void platformInvalidateContext();
- Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, WebCore::PolicyAction&, String& reason);
+ Ref<WebProcessProxy> processForNavigationInternal(WebPageProxy&, const API::Navigation&, ProcessSwapRequestedByClient, String& reason);
RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&);
Modified: trunk/Tools/ChangeLog (237698 => 237699)
--- trunk/Tools/ChangeLog 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Tools/ChangeLog 2018-11-01 20:53:50 UTC (rev 237699)
@@ -1,3 +1,16 @@
+2018-11-01 Chris Dumez <[email protected]>
+
+ [PSON] WebPageProxy::receivedNavigationPolicyDecision() should not schedule the new load asynchronously when process-swapping
+ https://bugs.webkit.org/show_bug.cgi?id=191076
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ (-[PSONNavigationDelegate init]):
+ (-[PSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+
2018-11-01 Adrian Perez de Castro <[email protected]>
Fix build with VIDEO and WEB_AUDIO disabled
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (237698 => 237699)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-01 20:46:22 UTC (rev 237698)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-01 20:53:50 UTC (rev 237699)
@@ -91,7 +91,7 @@
@end
@interface PSONNavigationDelegate : NSObject <WKNavigationDelegate> {
- @public WKNavigationActionPolicy navigationActionPolicyToUse;
+ @public void (^decidePolicyForNavigationAction)(WKNavigationAction *, void (^)(WKNavigationActionPolicy));
}
@end
@@ -100,7 +100,6 @@
- (instancetype) init
{
self = [super init];
- navigationActionPolicyToUse = WKNavigationActionPolicyAllow;
return self;
}
@@ -120,7 +119,10 @@
{
++numberOfDecidePolicyCalls;
seenPIDs.add([webView _webProcessIdentifier]);
- decisionHandler(navigationActionPolicyToUse);
+ if (decidePolicyForNavigationAction)
+ decidePolicyForNavigationAction(navigationAction, decisionHandler);
+ else
+ decisionHandler(WKNavigationActionPolicyAllow);
}
- (void)webView:(WKWebView *)webView didReceiveServerRedirectForProvisionalNavigation:(WKNavigation *)navigation
@@ -442,6 +444,47 @@
EXPECT_EQ(numberOfDecidePolicyCalls, 3);
}
+TEST(ProcessSwap, LoadAfterPolicyDecision)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().processSwapsOnNavigation = YES;
+ processPoolConfiguration.get().prewarmsProcessesAutomatically = 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()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+ decisionHandler(WKNavigationActionPolicyAllow);
+
+ // Synchronously navigate again right after answering the policy delegate for the previous navigation.
+ navigationDelegate->decidePolicyForNavigationAction = nil;
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+ [webView loadRequest:request];
+ };
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // Should navigate to pson://www.webkit.org/main2.html and not pson://www.apple.com/main.html.
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main2.html", [[webView URL] absoluteString]);
+}
+
TEST(ProcessSwap, NoSwappingForeTLDPlus2)
{
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
@@ -2304,7 +2347,9 @@
// Navigating from the above URL to this URL normally should not process swap,
// but we'll explicitly ask for a swap.
- navigationDelegate->navigationActionPolicyToUse = _WKNavigationActionPolicyAllowInNewProcess;
+ navigationDelegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+ decisionHandler(_WKNavigationActionPolicyAllowInNewProcess);
+ };
[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webit.org/3"]]];
TestWebKitAPI::Util::run(&done);
done = false;
