WebPageProxy::reattachToWebProcess cleanups
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 16:15:04 +0000 (16:15 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 16:15:04 +0000 (16:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189881

Reviewed by Chris Dumez.

Change the factoring to separate swap and crash code paths into different functions.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::reattachToWebProcess):
(WebKit::WebPageProxy::swapToWebProcess):

Separate function for the swap case.

(WebKit::WebPageProxy::finishAttachingToWebProcess):

Factor the common parts here.

(WebKit::WebPageProxy::continueNavigationInNewProcess):
* UIProcess/WebPageProxy.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h

index e977c89..1425dc9 100644 (file)
@@ -1,3 +1,25 @@
+2018-10-02  Antti Koivisto  <antti@apple.com>
+
+        WebPageProxy::reattachToWebProcess cleanups
+        https://bugs.webkit.org/show_bug.cgi?id=189881
+
+        Reviewed by Chris Dumez.
+
+        Change the factoring to separate swap and crash code paths into different functions.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::reattachToWebProcess):
+        (WebKit::WebPageProxy::swapToWebProcess):
+
+        Separate function for the swap case.
+
+        (WebKit::WebPageProxy::finishAttachingToWebProcess):
+
+        Factor the common parts here.
+
+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
+        * UIProcess/WebPageProxy.h:
+
 2018-10-02  Adrian Perez de Castro  <aperez@igalia.com>
 
         [GTK] Theming of authentication dialog breaks with themes other than Adwaita
index 5d91e1a..4b7a0f7 100644 (file)
@@ -694,8 +694,18 @@ void WebPageProxy::handleSynchronousMessage(IPC::Connection& connection, const S
 
 void WebPageProxy::reattachToWebProcess()
 {
-    auto process = makeRef(m_process->processPool().createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get()));
-    reattachToWebProcess(WTFMove(process), nullptr);
+    ASSERT(!m_isClosed);
+    ASSERT(!isValid());
+    RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::reattachToWebProcess\n", this);
+
+    m_process->removeWebPage(*this, m_pageID);
+    m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
+
+    auto& processPool = m_process->processPool();
+    m_process = processPool.createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get());
+    m_isValid = true;
+
+    finishAttachingToWebProcess();
 }
 
 SuspendedPageProxy* WebPageProxy::maybeCreateSuspendedPage(WebProcessProxy& process, API::Navigation& navigation)
@@ -721,27 +731,18 @@ void WebPageProxy::suspendedPageClosed(SuspendedPageProxy& page)
     m_suspendedPage = nullptr;
 }
 
-void WebPageProxy::reattachToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation* navigation)
+void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, API::Navigation& navigation)
 {
     ASSERT(!m_isClosed);
-    ASSERT(!isValid());
-
-    m_isValid = true;
+    RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::swapToWebProcess\n", this);
 
     // If the process we're attaching to is kept alive solely by our current suspended page,
     // we need to maintain that by temporarily keeping the suspended page alive.
-    std::unique_ptr<SuspendedPageProxy> currentSuspendedPage;
-    if (!navigation) {
-        m_process->removeWebPage(*this, m_pageID);
-        m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
-    } else {
-        currentSuspendedPage = WTFMove(m_suspendedPage);
-        m_process->suspendWebPageProxy(*this, *navigation);
-    }
+    auto currentSuspendedPage = WTFMove(m_suspendedPage);
+    m_process->suspendWebPageProxy(*this, navigation);
 
     m_process = WTFMove(process);
-    if (m_process->state() == WebProcessProxy::State::Running)
-        m_webProcessLifetimeTracker.webPageEnteringWebProcess();
+    m_isValid = true;
 
     // If we are reattaching to a SuspendedPage, then the WebProcess' WebPage already exists and
     // WebPageProxy::didCreateMainFrame() will not be called to initialize m_mainFrame. In such
@@ -754,11 +755,18 @@ void WebPageProxy::reattachToWebProcess(Ref<WebProcessProxy>&& process, API::Nav
         m_process->frameCreated(*m_mainFrameID, *m_mainFrame);
     }
 
-    RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::reattachToWebProcess\n", this);
+    finishAttachingToWebProcess();
+}
 
+void WebPageProxy::finishAttachingToWebProcess()
+{
     ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
-    if (m_process->state() == ChildProcessProxy::State::Running)
+
+    if (m_process->state() == ChildProcessProxy::State::Running) {
+        m_webProcessLifetimeTracker.webPageEnteringWebProcess();
         processDidFinishLaunching();
+    }
+
     m_process->addExistingWebPage(*this, m_pageID);
     m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
 
@@ -2517,9 +2525,7 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
 
     processDidTerminate(ProcessTerminationReason::NavigationSwap);
 
-    // FIXME: this is to fix the ASSERT(isValid()) inside reattachToWebProcess, some other way to fix this is needed.
-    m_isValid = false;
-    reattachToWebProcess(WTFMove(process), &navigation);
+    swapToWebProcess(WTFMove(process), navigation);
 
     if (auto* item = navigation.targetItem()) {
         LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
index 68f6c82..e66c54a 100644 (file)
@@ -1521,7 +1521,8 @@ private:
     void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
     void reattachToWebProcess();
-    void reattachToWebProcess(Ref<WebProcessProxy>&&, API::Navigation*);
+    void swapToWebProcess(Ref<WebProcessProxy>&&, API::Navigation&);
+    void finishAttachingToWebProcess();
 
     RefPtr<API::Navigation> reattachToWebProcessForReload();
     RefPtr<API::Navigation> reattachToWebProcessWithItem(WebBackForwardListItem&);