WebPageProxy::close() is a no-op for terminated processes
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Aug 2014 21:23:24 +0000 (21:23 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Aug 2014 21:23:24 +0000 (21:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136378
Related to <rdar://problem/16991213> and to <rdar://problem/17095600>

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
Source/WebKit2/UIProcess/WebInspectorProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.cpp

index 70a37d4805b8700ff69ebab824d3682ffe542a06..1a8047e20ee2de8f0c625f2fb5391ac55431072f 100644 (file)
@@ -1,3 +1,29 @@
+2014-08-29  Alexey Proskuryakov  <ap@apple.com>
+
+        WebPageProxy::close() is a no-op for terminated processes
+        https://bugs.webkit.org/show_bug.cgi?id=136378
+        Related to <rdar://problem/16991213> and to <rdar://problem/17095600>
+
+        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  <antti@apple.com>
 
         Remove NetworkResourceLoaderClient and subclasses.
index 1cd9589bd2a28d4c128d079482c3de80486f00f2..be37704cba83b2d63df6b18f494794621f2ac024 100644 (file)
@@ -163,8 +163,6 @@ void WebInspectorProxy::invalidate()
 
     m_page->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_page->pageID());
 
-    m_page->close();
-
     didClose();
 
     m_page = 0;
index bc3703365233eda8ecf2d5030a2fa0e8b8048bc0..fbce8106552e18b4fe829824e798ee282efd17f8 100644 (file)
@@ -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