Diff
Modified: trunk/Source/WebKit/ChangeLog (241822 => 241823)
--- trunk/Source/WebKit/ChangeLog 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Source/WebKit/ChangeLog 2019-02-20 19:19:26 UTC (rev 241823)
@@ -1,3 +1,26 @@
+2019-02-20 Chris Dumez <[email protected]>
+
+ Regression(PSON) Crash under WebKit::WebPageProxy::decidePolicyForNavigationActionSync
+ https://bugs.webkit.org/show_bug.cgi?id=194857
+ <rdar://problem/47759323>
+
+ Reviewed by Alex Christensen.
+
+ The ProvisionalPageProxy was blindly forwarding the DecidePolicyForNavigationActionSync
+ synchronous IPC to the WebPageProxy, without passing it the process the IPC came from.
+ As a result, WebPageProxy::decidePolicyForNavigationActionSync() would try to look up
+ a WebFrameProxy using the provided frameID from the wrong process and we would end up
+ hitting a RELEASE_ASSERT().
+
+ * UIProcess/ProvisionalPageProxy.cpp:
+ (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync):
+ (WebKit::ProvisionalPageProxy::didReceiveSyncMessage):
+ * UIProcess/ProvisionalPageProxy.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
+ (WebKit::WebPageProxy::decidePolicyForNavigationActionSyncShared):
+ * UIProcess/WebPageProxy.h:
+
2019-02-20 Don Olmstead <[email protected]>
[MSVC] Fix compilation errors with lambdas in Service Workers
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (241822 => 241823)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-02-20 19:19:26 UTC (rev 241823)
@@ -304,6 +304,29 @@
m_page.backForwardGoToItemShared(m_process.copyRef(), identifier, handle);
}
+void ProvisionalPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, WebCore::PolicyCheckIdentifier identifier,
+ uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID,
+ const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
+ const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
+{
+ ASSERT(isMainFrame);
+ ASSERT(!m_mainFrame || m_mainFrame->frameID() == frameID);
+
+ if (!isMainFrame || (m_mainFrame && m_mainFrame->frameID() != frameID)) {
+ reply(identifier, WebCore::PolicyAction::Ignore, navigationID, DownloadID(), WTF::nullopt);
+ return;
+ }
+
+ if (!m_mainFrame) {
+ // This synchronous IPC message was processed before the asynchronous DidCreateMainFrame one so we do not know about this frameID yet.
+ didCreateMainFrame(frameID);
+ }
+ ASSERT(m_mainFrame);
+
+ m_page.decidePolicyForNavigationActionSyncShared(m_process.copyRef(), frameID, isMainFrame, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData),
+ WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
+}
+
#if PLATFORM(COCOA)
void ProvisionalPageProxy::registerWebProcessAccessibilityToken(const IPC::DataReference& data)
{
@@ -404,6 +427,11 @@
return;
}
+ if (decoder.messageName() == Messages::WebPageProxy::DecidePolicyForNavigationActionSync::name()) {
+ IPC::handleMessageDelayed<Messages::WebPageProxy::DecidePolicyForNavigationActionSync>(connection, decoder, replyEncoder, this, &ProvisionalPageProxy::decidePolicyForNavigationActionSync);
+ return;
+ }
+
m_page.didReceiveSyncMessage(connection, decoder, replyEncoder);
}
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h (241822 => 241823)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2019-02-20 19:19:26 UTC (rev 241823)
@@ -102,6 +102,9 @@
void didFailProvisionalLoadForFrame(uint64_t frameID, const WebCore::SecurityOriginData& frameSecurityOrigin, uint64_t navigationID, const String& provisionalURL, const WebCore::ResourceError&, const UserData&);
void startURLSchemeTask(URLSchemeTaskParameters&&);
void backForwardGoToItem(const WebCore::BackForwardItemIdentifier&, SandboxExtension::Handle&);
+ void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&,
+ FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
+ WebCore::ResourceResponse&& redirectResponse, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
#if PLATFORM(COCOA)
void registerWebProcessAccessibilityToken(const IPC::DataReference&);
#endif
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (241822 => 241823)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-20 19:19:26 UTC (rev 241823)
@@ -4621,8 +4621,6 @@
const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
{
- auto sender = PolicyDecisionSender::create(identifier, WTFMove(reply));
-
auto* frame = m_process->webFrame(frameID);
if (!frame) {
// This synchronous IPC message was processed before the asynchronous DidCreateMainFrame / DidCreateSubframe one so we do not know about this frameID yet.
@@ -4630,12 +4628,23 @@
didCreateMainFrame(frameID);
else
didCreateSubframe(frameID);
-
- frame = m_process->webFrame(frameID);
- RELEASE_ASSERT(frame);
}
- decidePolicyForNavigationAction(m_process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData),
+ decidePolicyForNavigationActionSyncShared(m_process.copyRef(), frameID, isMainFrame, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData),
+ WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
+}
+
+void WebPageProxy::decidePolicyForNavigationActionSyncShared(Ref<WebProcessProxy>&& process, uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, PolicyCheckIdentifier identifier,
+ uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID,
+ const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
+ const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
+{
+ auto sender = PolicyDecisionSender::create(identifier, WTFMove(reply));
+
+ auto* frame = process->webFrame(frameID);
+ MESSAGE_CHECK(process, frame);
+
+ decidePolicyForNavigationAction(WTFMove(process), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData),
originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, sender.copyRef());
// If the client did not respond synchronously, proceed with the load.
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (241822 => 241823)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-02-20 19:19:26 UTC (rev 241823)
@@ -1455,6 +1455,9 @@
void loadDataWithNavigationShared(Ref<WebProcessProxy>&&, API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& = WTF::nullopt);
void loadRequestWithNavigationShared(Ref<WebProcessProxy>&&, API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& = WTF::nullopt);
void backForwardGoToItemShared(Ref<WebProcessProxy>&&, const WebCore::BackForwardItemIdentifier&, SandboxExtension::Handle&);
+ void decidePolicyForNavigationActionSyncShared(Ref<WebProcessProxy>&&, uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&,
+ FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
+ WebCore::ResourceResponse&& redirectResponse, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
void dumpAdClickAttribution(CompletionHandler<void(const String&)>&&);
void clearAdClickAttribution(CompletionHandler<void()>&&);
Modified: trunk/Tools/ChangeLog (241822 => 241823)
--- trunk/Tools/ChangeLog 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Tools/ChangeLog 2019-02-20 19:19:26 UTC (rev 241823)
@@ -1,5 +1,17 @@
2019-02-20 Chris Dumez <[email protected]>
+ Regression(PSON) Crash under WebKit::WebPageProxy::decidePolicyForNavigationActionSync
+ https://bugs.webkit.org/show_bug.cgi?id=194857
+ <rdar://problem/47759323>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+2019-02-20 Chris Dumez <[email protected]>
+
[WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests
https://bugs.webkit.org/show_bug.cgi?id=194829
<rdar://problem/47889906>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (241822 => 241823)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-20 19:15:32 UTC (rev 241822)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-20 19:19:26 UTC (rev 241823)
@@ -1483,6 +1483,55 @@
EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", [[webView URL] absoluteString]);
}
+TEST(ProcessSwap, ServerRedirectToAboutBlank)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ 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 addRedirectFromURLString:@"pson://www.webkit.org/main.html" toURLString:@"about:blank"];
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pidAfterFirstLoad = [webView _webProcessIdentifier];
+
+ EXPECT_EQ(1, numberOfDecidePolicyCalls);
+ EXPECT_EQ(1u, seenPIDs.size());
+ EXPECT_TRUE(*seenPIDs.begin() == pidAfterFirstLoad);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&serverRedirected);
+ serverRedirected = false;
+
+ seenPIDs.add([webView _webProcessIdentifier]);
+ if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
+ seenPIDs.add(provisionalPID);
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ seenPIDs.add([webView _webProcessIdentifier]);
+ if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
+ seenPIDs.add(provisionalPID);
+
+ EXPECT_FALSE(serverRedirected);
+ EXPECT_EQ(3, numberOfDecidePolicyCalls);
+ EXPECT_EQ(2u, seenPIDs.size());
+}
+
enum class ShouldCacheProcessFirst { No, Yes };
static void runSameOriginServerRedirectTest(ShouldCacheProcessFirst shouldCacheProcessFirst)
{
