From b2d15914ac5e5c140a1376d448070e478a698478 Mon Sep 17 00:00:00 2001 From: "ap@apple.com" Date: Fri, 29 Aug 2014 21:23:24 +0000 Subject: [PATCH] WebPageProxy::close() is a no-op for terminated processes https://bugs.webkit.org/show_bug.cgi?id=136378 Related to and to Reviewed by Brady Eidson. Also fixes issues that got uncovered after making close() work. * UIProcess/WebInspectorProxy.cpp: (WebKit::WebInspectorProxy::invalidate): Don't close the page, because it makes no sense, and causes an assertion now. Previosly, this was OK because the page was invalid already, and close() was a no-op. * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::~WebPageProxy): Added some assertions to catch invalidation issues earlier. (WebKit::WebPageProxy::reattachToWebProcess): Make it an invariant that a page's process always has a message receiver for it, until close() removes it. (WebKit::WebPageProxy::close): Make this function work for all open pages, whether they have a page or not. (WebKit::WebPageProxy::processDidFinishLaunching): Added an asserion that process agrees about its state. (WebKit::WebPageProxy::resetStateAfterProcessExited): Don't remove a message receiver, we now only do this in reattach or close. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173118 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebKit2/ChangeLog | 26 +++++++++++++++++++ .../WebKit2/UIProcess/WebInspectorProxy.cpp | 2 -- Source/WebKit2/UIProcess/WebPageProxy.cpp | 13 +++++++--- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog index 70a37d4805b8..1a8047e20ee2 100644 --- a/Source/WebKit2/ChangeLog +++ b/Source/WebKit2/ChangeLog @@ -1,3 +1,29 @@ +2014-08-29 Alexey Proskuryakov + + WebPageProxy::close() is a no-op for terminated processes + https://bugs.webkit.org/show_bug.cgi?id=136378 + Related to and to + + Reviewed by Brady Eidson. + + Also fixes issues that got uncovered after making close() work. + + * UIProcess/WebInspectorProxy.cpp: (WebKit::WebInspectorProxy::invalidate): Don't + close the page, because it makes no sense, and causes an assertion now. Previosly, + this was OK because the page was invalid already, and close() was a no-op. + + * UIProcess/WebPageProxy.cpp: + (WebKit::WebPageProxy::~WebPageProxy): Added some assertions to catch invalidation + issues earlier. + (WebKit::WebPageProxy::reattachToWebProcess): Make it an invariant that a page's + process always has a message receiver for it, until close() removes it. + (WebKit::WebPageProxy::close): Make this function work for all open pages, whether + they have a page or not. + (WebKit::WebPageProxy::processDidFinishLaunching): Added an asserion that process + agrees about its state. + (WebKit::WebPageProxy::resetStateAfterProcessExited): Don't remove a message receiver, + we now only do this in reattach or close. + 2014-08-29 Antti Koivisto Remove NetworkResourceLoaderClient and subclasses. diff --git a/Source/WebKit2/UIProcess/WebInspectorProxy.cpp b/Source/WebKit2/UIProcess/WebInspectorProxy.cpp index 1cd9589bd2a2..be37704cba83 100644 --- a/Source/WebKit2/UIProcess/WebInspectorProxy.cpp +++ b/Source/WebKit2/UIProcess/WebInspectorProxy.cpp @@ -163,8 +163,6 @@ void WebInspectorProxy::invalidate() m_page->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_page->pageID()); - m_page->close(); - didClose(); m_page = 0; diff --git a/Source/WebKit2/UIProcess/WebPageProxy.cpp b/Source/WebKit2/UIProcess/WebPageProxy.cpp index bc3703365233..fbce8106552e 100644 --- a/Source/WebKit2/UIProcess/WebPageProxy.cpp +++ b/Source/WebKit2/UIProcess/WebPageProxy.cpp @@ -425,6 +425,12 @@ WebPageProxy::WebPageProxy(PageClient& pageClient, WebProcessProxy& process, uin WebPageProxy::~WebPageProxy() { + ASSERT(m_process->webPage(m_pageID) != this); +#if !ASSERT_DISABLED + for (WebPageProxy* page : m_process->pages()) + ASSERT(page != this); +#endif + if (!m_isClosed) close(); @@ -546,6 +552,7 @@ void WebPageProxy::reattachToWebProcess() m_isValid = true; m_process->removeWebPage(m_pageID); + m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID); if (m_process->context().processModel() == ProcessModelSharedSecondaryProcess) m_process = m_process->context().ensureSharedWebProcess(); @@ -650,7 +657,7 @@ bool WebPageProxy::isProcessSuppressible() const void WebPageProxy::close() { - if (!isValid()) + if (m_isClosed) return; m_isClosed = true; @@ -3133,6 +3140,8 @@ void WebPageProxy::connectionWillClose(IPC::Connection* connection) void WebPageProxy::processDidFinishLaunching() { + ASSERT(m_process->state() == WebProcessProxy::State::Running); + if (m_userContentController) m_userContentController->addProcess(m_process.get()); m_visitedLinkProvider->addProcess(m_process.get()); @@ -4459,8 +4468,6 @@ void WebPageProxy::resetStateAfterProcessExited() m_visitedLinkProvider->removeProcess(m_process.get()); } - m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID); - #if PLATFORM(IOS) m_activityToken = nullptr; #endif -- 2.36.0