- Revision
- 241501
- Author
- [email protected]
- Date
- 2019-02-14 00:34:17 -0800 (Thu, 14 Feb 2019)
Log Message
Cherry-pick r241336. rdar://problem/48065621
Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
https://bugs.webkit.org/show_bug.cgi?id=194568
<rdar://problem/47944490>
Reviewed by Ryosuke Niwa.
Source/WebKit:
When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
the frame has been removed from the WebProcessProxy at this point.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _provisionalWebProcessIdentifier]):
* UIProcess/API/Cocoa/WKWebViewPrivate.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
(WebKit::WebPageProxy::provisionalProcessDidTerminate):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
(-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241336 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-607-branch/Source/WebKit/ChangeLog (241500 => 241501)
--- branches/safari-607-branch/Source/WebKit/ChangeLog 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/ChangeLog 2019-02-14 08:34:17 UTC (rev 241501)
@@ -1,3 +1,59 @@
+2019-02-13 Babak Shafiei <[email protected]>
+
+ Cherry-pick r241336. rdar://problem/48065621
+
+ Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+ https://bugs.webkit.org/show_bug.cgi?id=194568
+ <rdar://problem/47944490>
+
+ Reviewed by Ryosuke Niwa.
+
+ Source/WebKit:
+
+ When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+ the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+ that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+ the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+ the frame has been removed from the WebProcessProxy at this point.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _provisionalWebProcessIdentifier]):
+ * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+ (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241336 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-12 Chris Dumez <[email protected]>
+
+ Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+ https://bugs.webkit.org/show_bug.cgi?id=194568
+ <rdar://problem/47944490>
+
+ Reviewed by Ryosuke Niwa.
+
+ When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+ the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+ that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+ the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+ the frame has been removed from the WebProcessProxy at this point.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _provisionalWebProcessIdentifier]):
+ * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+ (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+
2019-02-13 Alan Coon <[email protected]>
Apply patch. rdar://problem/40966400
Modified: branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (241500 => 241501)
--- branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2019-02-14 08:34:17 UTC (rev 241501)
@@ -4654,6 +4654,18 @@
return _page->processIdentifier();
}
+- (pid_t)_provisionalWebProcessIdentifier
+{
+ if (![self _isValid])
+ return 0;
+
+ auto* provisionalPage = _page->provisionalPageProxy();
+ if (!provisionalPage)
+ return 0;
+
+ return provisionalPage->process().processIdentifier();
+}
+
- (void)_killWebContentProcess
{
if (![self _isValid])
Modified: branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h (241500 => 241501)
--- branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2019-02-14 08:34:17 UTC (rev 241501)
@@ -147,6 +147,7 @@
@property (nonatomic, setter=_setUserContentExtensionsEnabled:) BOOL _userContentExtensionsEnabled WK_API_AVAILABLE(macosx(10.11), ios(9.0));
@property (nonatomic, readonly) pid_t _webProcessIdentifier;
+@property (nonatomic, readonly) pid_t _provisionalWebProcessIdentifier WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
@property (nonatomic, getter=_isEditable, setter=_setEditable:) BOOL _editable WK_API_AVAILABLE(macosx(10.11), ios(9.0));
Modified: branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (241500 => 241501)
--- branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-14 08:34:17 UTC (rev 241501)
@@ -3938,7 +3938,7 @@
PageClientProtector protector(pageClient());
WebFrameProxy* frame = process->webFrame(frameID);
- MESSAGE_CHECK(m_process, frame);
+ MESSAGE_CHECK(process, frame);
if (m_controlledByAutomation) {
if (auto* automationSession = process->processPool().automationSession())
@@ -6452,7 +6452,6 @@
void WebPageProxy::provisionalProcessDidTerminate()
{
ASSERT(m_provisionalPage);
- m_provisionalPage->cancel();
m_provisionalPage = nullptr;
}
Modified: branches/safari-607-branch/Tools/ChangeLog (241500 => 241501)
--- branches/safari-607-branch/Tools/ChangeLog 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Tools/ChangeLog 2019-02-14 08:34:17 UTC (rev 241501)
@@ -1,3 +1,51 @@
+2019-02-13 Babak Shafiei <[email protected]>
+
+ Cherry-pick r241336. rdar://problem/48065621
+
+ Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+ https://bugs.webkit.org/show_bug.cgi?id=194568
+ <rdar://problem/47944490>
+
+ Reviewed by Ryosuke Niwa.
+
+ Source/WebKit:
+
+ When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+ the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+ that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+ the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+ the frame has been removed from the WebProcessProxy at this point.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _provisionalWebProcessIdentifier]):
+ * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+ (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241336 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-12 Chris Dumez <[email protected]>
+
+ Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+ https://bugs.webkit.org/show_bug.cgi?id=194568
+ <rdar://problem/47944490>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+
2019-02-13 Alan Coon <[email protected]>
Cherry-pick r241288. rdar://problem/47992210
Modified: branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (241500 => 241501)
--- branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-14 08:34:17 UTC (rev 241501)
@@ -98,6 +98,7 @@
@interface PSONNavigationDelegate : NSObject <WKNavigationDelegate> {
@public void (^decidePolicyForNavigationAction)(WKNavigationAction *, void (^)(WKNavigationActionPolicy));
+ @public void (^didStartProvisionalNavigationHandler)();
@public void (^didCommitNavigationHandler)();
}
@end
@@ -125,6 +126,8 @@
- (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(null_unspecified WKNavigation *)navigation
{
didStartProvisionalLoad = true;
+ if (didStartProvisionalNavigationHandler)
+ didStartProvisionalNavigationHandler();
}
- (void)webView:(WKWebView *)webView didCommitNavigation:(null_unspecified WKNavigation *)navigation
@@ -1488,6 +1491,40 @@
EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", [[webView URL] absoluteString]);
}
+TEST(ProcessSwap, TerminateProcessRightAfterSwap)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().processSwapsOnNavigation = 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 delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ [webView configuration].preferences.safeBrowsingEnabled = NO;
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ delegate->didStartProvisionalNavigationHandler = ^{
+ EXPECT_NE(0, [webView _provisionalWebProcessIdentifier]);
+ kill([webView _provisionalWebProcessIdentifier], 9);
+ };
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::sleep(0.5);
+}
+
static const char* linkToWebKitBytes = R"PSONRESOURCE(
<body>
<a id="testLink" href=""