WebKit Bugzilla
Attachment 350072 Details for
Bug 189721
: Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-189721-20180918161631.patch (text/plain), 8.84 KB, created by
Chris Dumez
on 2018-09-18 16:16:32 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2018-09-18 16:16:32 PDT
Size:
8.84 KB
patch
obsolete
>Subversion Revision: 236154 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index efe33716a766bb1f017fb09dc61740fbf6196d4f..f8c7eaaea9d1968e97e9953853734f11323ccc30 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,31 @@ >+2018-09-18 Chris Dumez <cdumez@apple.com> >+ >+ Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&) >+ https://bugs.webkit.org/show_bug.cgi?id=189721 >+ <rdar://problem/44359788> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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-18 Alex Christensen <achristensen@webkit.org> > > Make WebPageProxy always have a API::NavigationClient instead of always having a API::LoaderClient and API::PolicyClient >diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp >index 5d1fc5c31c9409f2e23dffdae5239421678e09b4..533290bf1e116426f83318a5b0c5a77b27a5118e 100644 >--- a/Source/WebKit/UIProcess/SuspendedPageProxy.cpp >+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.cpp >@@ -73,9 +73,9 @@ static const HashSet<IPC::StringReference>& messageNamesToIgnoreWhileSuspended() > } > #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(WebPageProxy& page, WebProcessProxy& proc > > 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); >diff --git a/Source/WebKit/UIProcess/SuspendedPageProxy.h b/Source/WebKit/UIProcess/SuspendedPageProxy.h >index 4921109d3e4e876703249545e4c4a0ded9b5ab47..4c1292a1b0f52fed025ab67608d70f1c5bd24f7f 100644 >--- a/Source/WebKit/UIProcess/SuspendedPageProxy.h >+++ b/Source/WebKit/UIProcess/SuspendedPageProxy.h >@@ -38,13 +38,13 @@ class WebProcessProxy; > > 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 @@ private: > void didFinishLoad(); > > WebPageProxy& m_page; >- WebProcessProxy* m_process; >+ RefPtr<WebProcessProxy> m_process; > WebCore::SecurityOriginData m_origin; > > #if !LOG_DISABLED >diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp >index e4f47d930a7bed4c1b621dff9db3198d80cf4ce8..2714215b676418cb55f8c70661ef448bf6593d32 100644 >--- a/Source/WebKit/UIProcess/WebProcessProxy.cpp >+++ b/Source/WebKit/UIProcess/WebProcessProxy.cpp >@@ -1008,6 +1008,11 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason) > > shutDown(); > >+ for (auto* suspendedPage : copyToVectorOf<SuspendedPageProxy*>(m_suspendedPageMap.values())) >+ suspendedPage->webProcessDidClose(*this); >+ >+ m_suspendedPageMap.clear(); >+ > for (auto& page : pages) > page->processDidTerminate(reason); > } >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 2767dbf827b6eb6488b3239c2403372b2b8f6b4a..595c75e77dd23b274126a3fcb789be7190d90eb5 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,15 @@ >+2018-09-18 Chris Dumez <cdumez@apple.com> >+ >+ Crash under WebProcessProxy::suspendedPageWasDestroyed(WebKit::SuspendedPageProxy&) >+ https://bugs.webkit.org/show_bug.cgi?id=189721 >+ <rdar://problem/44359788> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add API test coverage. >+ >+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm: >+ > 2018-09-18 Jonathan Bedard <jbedard@apple.com> > > webkitpy: Clobbering and building occurs multiple times for iOS Simulator ports >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >index 71b21eb492ed649e93dcbf38f2ba111d77b5e002..8fe073b83ca6e77e9eb6e161a4377083705efbf0 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >@@ -1803,4 +1803,69 @@ TEST(ProcessSwap, APIControlledProcessSwapping) > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189721
:
350070
| 350072 |
350202