- Revision
- 240477
- Author
- cdu...@apple.com
- Date
- 2019-01-25 09:32:53 -0800 (Fri, 25 Jan 2019)
Log Message
Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
https://bugs.webkit.org/show_bug.cgi?id=193779
<rdar://problem/46170903>
Reviewed by Antti Koivisto.
Source/WebKit:
* UIProcess/Cocoa/NavigationState.mm:
(WebKit::tryAppLink):
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
We were crashing when trying to get the URL of the main frame, which was sad because we never
ended up using the main frame URL. Therefore, this patch drops the code in question.
* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
Add assertion to make sure that the DecidePolicyForNavigationActionAsync IPC it is getting
from the process is related to its main frame.
Tools:
Add API test that quickly navigates forward to a previous process without waiting for it to
suspend. I suspect the crash could have been happening due to receiving leftover IPC from
the process' previous page when reconnecting the it for the forward navigation.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (240476 => 240477)
--- trunk/Source/WebKit/ChangeLog 2019-01-25 17:23:06 UTC (rev 240476)
+++ trunk/Source/WebKit/ChangeLog 2019-01-25 17:32:53 UTC (rev 240477)
@@ -1,3 +1,22 @@
+2019-01-25 Chris Dumez <cdu...@apple.com>
+
+ Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
+ https://bugs.webkit.org/show_bug.cgi?id=193779
+ <rdar://problem/46170903>
+
+ Reviewed by Antti Koivisto.
+
+ * UIProcess/Cocoa/NavigationState.mm:
+ (WebKit::tryAppLink):
+ (WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
+ We were crashing when trying to get the URL of the main frame, which was sad because we never
+ ended up using the main frame URL. Therefore, this patch drops the code in question.
+
+ * UIProcess/ProvisionalPageProxy.cpp:
+ (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
+ Add assertion to make sure that the DecidePolicyForNavigationActionAsync IPC it is getting
+ from the process is related to its main frame.
+
2019-01-25 Wenson Hsieh <wenson_hs...@apple.com>
Need a way for _javascript_ (or bundle) code to participate in undo
Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm (240476 => 240477)
--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2019-01-25 17:23:06 UTC (rev 240476)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2019-01-25 17:32:53 UTC (rev 240477)
@@ -464,7 +464,7 @@
}
#endif
-static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const String& currentMainFrameURL, WTF::Function<void(bool)>&& completionHandler)
+static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, WTF::Function<void(bool)>&& completionHandler)
{
#if HAVE(APP_LINKS)
if (!navigationAction->shouldOpenAppLinks()) {
@@ -486,8 +486,6 @@
void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageProxy& webPageProxy, Ref<API::NavigationAction>&& navigationAction, Ref<WebFramePolicyListenerProxy>&& listener, API::Object* userInfo)
{
- ASSERT(webPageProxy.mainFrame());
- String mainFrameURLString = webPageProxy.mainFrame()->url();
bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame();
if (!m_navigationState.m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionDecisionHandler
@@ -521,7 +519,7 @@
#endif
listener->ignore();
};
- tryAppLink(WTFMove(navigationAction), mainFrameURLString, WTFMove(completionHandler));
+ tryAppLink(WTFMove(navigationAction), WTFMove(completionHandler));
return;
}
@@ -533,7 +531,7 @@
auto checker = CompletionHandlerCallChecker::create(navigationDelegate.get(), delegateHasWebsitePolicies ? @selector(_webView:decidePolicyForNavigationAction:decisionHandler:) : @selector(webView:decidePolicyForNavigationAction:decisionHandler:));
- auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), mainFrameURLString, webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
+ auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
if (checker->completionHandlerHasBeenCalled())
return;
checker->didCallCompletionHandler();
@@ -556,7 +554,7 @@
switch (actionPolicy) {
case WKNavigationActionPolicyAllow:
case _WKNavigationActionPolicyAllowInNewProcess:
- tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
+ tryAppLink(WTFMove(navigationAction), [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
if (followedLinkToApp) {
localListener->ignore();
return;
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (240476 => 240477)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-01-25 17:23:06 UTC (rev 240476)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-01-25 17:32:53 UTC (rev 240477)
@@ -264,6 +264,8 @@
void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, 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, uint64_t listenerID)
{
+ ASSERT(m_mainFrame);
+ ASSERT(m_mainFrame->frameID() == frameID);
m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID);
}
Modified: trunk/Tools/ChangeLog (240476 => 240477)
--- trunk/Tools/ChangeLog 2019-01-25 17:23:06 UTC (rev 240476)
+++ trunk/Tools/ChangeLog 2019-01-25 17:32:53 UTC (rev 240477)
@@ -1,3 +1,17 @@
+2019-01-25 Chris Dumez <cdu...@apple.com>
+
+ Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
+ https://bugs.webkit.org/show_bug.cgi?id=193779
+ <rdar://problem/46170903>
+
+ Reviewed by Antti Koivisto.
+
+ Add API test that quickly navigates forward to a previous process without waiting for it to
+ suspend. I suspect the crash could have been happening due to receiving leftover IPC from
+ the process' previous page when reconnecting the it for the forward navigation.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2019-01-25 Wenson Hsieh <wenson_hs...@apple.com>
Need a way for _javascript_ (or bundle) code to participate in undo
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (240476 => 240477)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-01-25 17:23:06 UTC (rev 240476)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-01-25 17:32:53 UTC (rev 240477)
@@ -2136,6 +2136,75 @@
EXPECT_WK_STREQ(@"pson://www.google.com/main.html", [backForwardList.forwardList[1].URL absoluteString]);
}
+static const char* keepNavigatingFrameBytes = R"PSONRESOURCE(
+<body>
+<iframe id="testFrame1" src=""
+<iframe id="testFrame2" src=""
+<iframe id="testFrame3" src=""
+<script>
+window.addEventListener('pagehide', function(event) {
+ for (var j = 0; j < 10000; j++);
+});
+
+var i = 0;
+function navigateFrames()
+{
+ testFrame1.src = "" + i + ".html";
+ testFrame2.src = "" + i + ".html";
+ testFrame3.src = "" + i + ".html";
+ i++;
+}
+
+navigateFrames();
+setInterval(() => {
+ navigateFrames();
+}, 0);
+</script>
+</body>
+)PSONRESOURCE";
+
+TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation)
+{
+ 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:keepNavigatingFrameBytes];
+ [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.webkit.org/main1.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto webkitPID = [webView _webProcessIdentifier];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto applePID = [webView _webProcessIdentifier];
+ EXPECT_NE(webkitPID, applePID);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+}
+
static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE(
<script>
function loaded() {
