- Revision
- 236226
- Author
- [email protected]
- Date
- 2018-09-19 14:33:51 -0700 (Wed, 19 Sep 2018)
Log Message
Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&)
https://bugs.webkit.org/show_bug.cgi?id=189721
<rdar://problem/44359788>
Reviewed by Geoffrey Garen.
Source/WebKit:
Fix crash when destroying a SuspendedPageProxy whose WebProcessProxy was already
destroyed.
* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
(WebKit::SuspendedPageProxy::~SuspendedPageProxy):
* UIProcess/SuspendedPageProxy.h:
(WebKit::SuspendedPageProxy::process const):
Update SuspendedPageProxy::m_process to be a RefPtr<> instead of a raw pointer, similarly
to what we do in WebPageProxy. Relying on the WebProcessProxy to not get destroyed is
risky as this crash demonstrates.
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::requestTermination):
When a WebProcessProxy is terminated (by client or WebKit due to memory / cpu usage), call
webProcessDidClose() on all SuspendedPages, similarly to what we do in case of a crash in
processDidTerminateOrFailedToLaunch(). Failing to do so means that the SuspendedPageProxy
may still have a pointer to this WebProcessProxy, even though WebProcessProxy::shutDown()
has been called (which may destroy the WebProcessProxy).
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (236225 => 236226)
--- trunk/Source/WebKit/ChangeLog 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Source/WebKit/ChangeLog 2018-09-19 21:33:51 UTC (rev 236226)
@@ -1,3 +1,31 @@
+2018-09-19 Chris Dumez <[email protected]>
+
+ Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&)
+ https://bugs.webkit.org/show_bug.cgi?id=189721
+ <rdar://problem/44359788>
+
+ Reviewed by Geoffrey Garen.
+
+ Fix crash when destroying a SuspendedPageProxy whose WebProcessProxy was already
+ destroyed.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+ (WebKit::SuspendedPageProxy::~SuspendedPageProxy):
+ * UIProcess/SuspendedPageProxy.h:
+ (WebKit::SuspendedPageProxy::process const):
+ Update SuspendedPageProxy::m_process to be a RefPtr<> instead of a raw pointer, similarly
+ to what we do in WebPageProxy. Relying on the WebProcessProxy to not get destroyed is
+ risky as this crash demonstrates.
+
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::requestTermination):
+ When a WebProcessProxy is terminated (by client or WebKit due to memory / cpu usage), call
+ webProcessDidClose() on all SuspendedPages, similarly to what we do in case of a crash in
+ processDidTerminateOrFailedToLaunch(). Failing to do so means that the SuspendedPageProxy
+ may still have a pointer to this WebProcessProxy, even though WebProcessProxy::shutDown()
+ has been called (which may destroy the WebProcessProxy).
+
2018-09-19 John Wilander <[email protected]>
Resource Load Statistics: Add optional cap on partitioned cache max age
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (236225 => 236226)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-09-19 21:33:51 UTC (rev 236226)
@@ -73,9 +73,9 @@
}
#endif
-SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, WebProcessProxy& process, WebBackForwardListItem& item)
+SuspendedPageProxy::SuspendedPageProxy(WebPageProxy& page, Ref<WebProcessProxy>&& process, WebBackForwardListItem& item)
: m_page(page)
- , m_process(&process)
+ , m_process(WTFMove(process))
, m_origin(SecurityOriginData::fromURL({ ParsedURLString, item.url() }))
{
item.setSuspendedPage(*this);
@@ -85,7 +85,7 @@
SuspendedPageProxy::~SuspendedPageProxy()
{
- if (auto process = makeRefPtr(m_process)) {
+ if (auto process = m_process) {
process->send(Messages::WebPage::SetIsSuspended(false), m_page.pageID());
process->suspendedPageWasDestroyed(*this);
process->processPool().unregisterSuspendedPageProxy(*this);
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (236225 => 236226)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-09-19 21:33:51 UTC (rev 236226)
@@ -38,13 +38,13 @@
class SuspendedPageProxy : public CanMakeWeakPtr<SuspendedPageProxy> {
public:
- SuspendedPageProxy(WebPageProxy&, WebProcessProxy&, WebBackForwardListItem&);
+ SuspendedPageProxy(WebPageProxy&, Ref<WebProcessProxy>&&, WebBackForwardListItem&);
~SuspendedPageProxy();
void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
WebPageProxy& page() const { return m_page; }
- WebProcessProxy* process() const { return m_process; }
+ WebProcessProxy* process() const { return m_process.get(); }
const WebCore::SecurityOriginData& origin() const { return m_origin; }
void webProcessDidClose(WebProcessProxy&);
@@ -58,7 +58,7 @@
void didFinishLoad();
WebPageProxy& m_page;
- WebProcessProxy* m_process;
+ RefPtr<WebProcessProxy> m_process;
WebCore::SecurityOriginData m_origin;
#if !LOG_DISABLED
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (236225 => 236226)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-09-19 21:33:51 UTC (rev 236226)
@@ -1008,6 +1008,11 @@
shutDown();
+ for (auto* suspendedPage : copyToVectorOf<SuspendedPageProxy*>(m_suspendedPageMap.values()))
+ suspendedPage->webProcessDidClose(*this);
+
+ m_suspendedPageMap.clear();
+
for (auto& page : pages)
page->processDidTerminate(reason);
}
Modified: trunk/Tools/ChangeLog (236225 => 236226)
--- trunk/Tools/ChangeLog 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Tools/ChangeLog 2018-09-19 21:33:51 UTC (rev 236226)
@@ -1,3 +1,15 @@
+2018-09-19 Chris Dumez <[email protected]>
+
+ Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&)
+ https://bugs.webkit.org/show_bug.cgi?id=189721
+ <rdar://problem/44359788>
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-09-19 Thomas Denney <[email protected]>
[WHLSL] Improve test suite type safety
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (236225 => 236226)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-09-19 21:33:01 UTC (rev 236225)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-09-19 21:33:51 UTC (rev 236226)
@@ -1803,4 +1803,69 @@
EXPECT_NE(pid1, pid3);
}
+TEST(ProcessSwap, TerminatedSuspendedPageProcess)
+{
+ 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 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 pid1 = [webView _webProcessIdentifier];
+
+ @autoreleasepool {
+ auto webViewConfiguration2 = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration2 setProcessPool:processPool.get()];
+ [webViewConfiguration2 _setRelatedWebView:webView.get()]; // Make sure it uses the same process.
+ auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration2.get()]);
+ [webView2 setNavigationDelegate:delegate.get()];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"about:blank"]];
+ [webView2 loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [webView2 _webProcessIdentifier];
+ EXPECT_EQ(pid1, pid2);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main2.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ [webView2 _killWebContentProcessAndResetState];
+ webView2 = nullptr;
+ webViewConfiguration2 = nullptr;
+ }
+
+ auto pid3 = [webView _webProcessIdentifier];
+ EXPECT_NE(pid1, pid3);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main2.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid4 = [webView _webProcessIdentifier];
+ EXPECT_NE(pid1, pid4);
+ EXPECT_NE(pid3, pid4);
+}
+
#endif // WK_API_ENABLED
