Changeset 243154 in webkit

Timestamp:

Mar 19, 2019 11:38:59 AM (5 years ago)

Author:

Chris Dumez

Message:

Unreviewed, rolling out r243142.

Caused assertion hits in WK2 Debug

Reverted changeset:

"Spew: Unhandled web process message
'VisitedLinkTableController:VisitedLinkStateChanged'"
​https://bugs.webkit.org/show_bug.cgi?id=194787
​https://trac.webkit.org/changeset/243142

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ProvisionalPageProxy.cpp (modified) (3 diffs)
• UIProcess/ProvisionalPageProxy.h (modified) (2 diffs)
• UIProcess/VisitedLinkStore.cpp (modified) (2 diffs)
• UIProcess/WebPageProxy.cpp (modified) (4 diffs)
• UIProcess/WebPageProxy.h (modified) (3 diffs)
• UIProcess/WebProcessProxy.cpp (modified) (5 diffs)
• UIProcess/WebProcessProxy.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-03-19  Chris Dumez  <cdumez@apple.com>
+
+        Unreviewed, rolling out r243142.
+
+        Caused assertion hits in WK2 Debug
+
+        Reverted changeset:
+
+        "Spew: Unhandled web process message
+        'VisitedLinkTableController:VisitedLinkStateChanged'"
+        https://bugs.webkit.org/show_bug.cgi?id=194787
+        https://trac.webkit.org/changeset/243142
+
 2019-03-19  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

@@ -100 +100 @@
     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID());
     m_process->send(Messages::WebPage::Close(), m_page.pageID());
-    m_process->removeVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.pageID());
 
     RunLoop::main().dispatch([process = m_process.copyRef()] {
@@ -138 +137 @@
 }
 
+void ProvisionalPageProxy::processDidFinishLaunching()
+{
+    RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "processDidFinishLaunching: pageID = %" PRIu64, m_page.pageID());
+    finishInitializingWebPageAfterProcessLaunch();
+}
+
+void ProvisionalPageProxy::finishInitializingWebPageAfterProcessLaunch()
+{
+    ASSERT(m_process->state() == WebProcessProxy::State::Running);
+
+    // FIXME: The WebPageProxy delays adding the visited link store until after the process has launched
+    // so the ProvisionalPageProxy does the same. However, do we really need to?
+    m_process->addVisitedLinkStore(m_page.visitedLinkStore());
+}
+
 void ProvisionalPageProxy::initializeWebPage()
 {
@@ -145 +159 @@
     parameters.isProcessSwap = true;
     m_process->send(Messages::WebProcess::CreateWebPage(m_page.pageID(), parameters), 0);
-    m_process->addVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.pageID());
+
+    if (m_process->state() == WebProcessProxy::State::Running)
+        finishInitializingWebPageAfterProcessLaunch();
 }
 

trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h

@@ -80 +80 @@
     void cancel();
 
+    void processDidFinishLaunching();
     void processDidTerminate();
     void connectionWillOpen(IPC::Connection&);
@@ -115 +116 @@
 
     void initializeWebPage();
+    void finishInitializingWebPageAfterProcessLaunch();
 
     WebPageProxy& m_page;

trunk/Source/WebKit/UIProcess/VisitedLinkStore.cpp

@@ -43 +43 @@
 VisitedLinkStore::~VisitedLinkStore()
 {
-    RELEASE_ASSERT(m_processes.isEmpty());
+    for (WebProcessProxy* process : m_processes) {
+        process->removeMessageReceiver(Messages::VisitedLinkStore::messageReceiverName(), identifier());
+        process->didDestroyVisitedLinkStore(*this);
+    }
 }
 
@@ -54 +57 @@
 {
     ASSERT(process.state() == WebProcessProxy::State::Running);
-    ASSERT(!m_processes.contains(&process));
 
     if (!m_processes.add(&process).isNewEntry)

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -820 +820 @@
         if (isProcessSwap != IsProcessSwap::Yes)
             m_webProcessLifetimeTracker.webPageEnteringWebProcess(m_process);
+        processDidFinishLaunching();
     }
 
@@ -958 +959 @@
     process().send(Messages::WebProcess::CreateWebPage(m_pageID, creationParameters(m_process, *m_drawingArea)), 0);
 
-    m_process->addVisitedLinkStoreUser(visitedLinkStore(), m_pageID);
+    m_needsToFinishInitializingWebPageAfterProcessLaunch = true;
+    finishInitializingWebPageAfterProcessLaunch();
+}
+
+void WebPageProxy::finishInitializingWebPageAfterProcessLaunch()
+{
+    if (!m_needsToFinishInitializingWebPageAfterProcessLaunch)
+        return;
+    if (m_process->state() != WebProcessProxy::State::Running)
+        return;
+
+    m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
+    m_process->addVisitedLinkStore(m_visitedLinkStore);
 }
 
@@ -5101 +5114 @@
 }
 
+void WebPageProxy::processDidFinishLaunching()
+{
+    ASSERT(m_process->state() == WebProcessProxy::State::Running);
+    finishInitializingWebPageAfterProcessLaunch();
+}
+
 #if ENABLE(NETSCAPE_PLUGIN_API)
 void WebPageProxy::unavailablePluginButtonClicked(uint32_t opaquePluginUnavailabilityReason, const String& mimeType, const String& pluginURLString, const String& pluginspageAttributeURLString, const String& frameURLString, const String& pageURLString)
@@ -6905 +6924 @@
     m_hasRunningProcess = false;
     m_isPageSuspended = false;
+
+    m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
 
     m_editorState = EditorState();

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1234 +1234 @@
     void webProcessWillShutDown();
 
+    void processDidFinishLaunching();
+
     void didSaveToPageCache();
         
@@ -1973 +1975 @@
     void didResignInputElementStrongPasswordAppearance(const UserData&);
 
+    void finishInitializingWebPageAfterProcessLaunch();
+
     void handleMessage(IPC::Connection&, const String& messageName, const UserData& messageBody);
     void handleSynchronousMessage(IPC::Connection&, const String& messageName, const UserData& messageBody, UserData& returnUserData);
@@ -2222 +2226 @@
     bool m_canRunModal { false };
 
+    bool m_needsToFinishInitializingWebPageAfterProcessLaunch { false };
+
     bool m_isInPrintingMode { false };
     bool m_isPerformingDOMPrintOperation { false };

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -299 +299 @@
     m_frameMap.clear();
 
-    for (auto* visitedLinkStore : m_visitedLinkStoresWithUsers.keys())
+    for (auto* visitedLinkStore : m_visitedLinkStores)
         visitedLinkStore->removeProcess(*this);
-    m_visitedLinkStoresWithUsers.clear();
+    m_visitedLinkStores.clear();
 
     for (auto* webUserContentControllerProxy : m_webUserContentControllerProxies)
@@ -393 +393 @@
         m_processPool->pageEndUsingWebsiteDataStore(webPage.pageID(), webPage.websiteDataStore());
 
-    removeVisitedLinkStoreUser(webPage.visitedLinkStore(), webPage.pageID());
-
     updateBackgroundResponsivenessTimer();
 
@@ -400 +398 @@
 }
 
-void WebProcessProxy::addVisitedLinkStoreUser(VisitedLinkStore& visitedLinkStore, uint64_t pageID)
-{
-    auto users = m_visitedLinkStoresWithUsers.ensure(&visitedLinkStore, [] {
-        return HashSet<uint64_t> { };
-    }).iterator->value;
-
-    ASSERT(!users.contains(pageID));
-    users.add(pageID);
-
-    if (users.size() == 1 && state() == State::Running)
-        visitedLinkStore.addProcess(*this);
-}
-
-void WebProcessProxy::removeVisitedLinkStoreUser(VisitedLinkStore& visitedLinkStore, uint64_t pageID)
-{
-    auto it = m_visitedLinkStoresWithUsers.find(&visitedLinkStore);
-    if (it == m_visitedLinkStoresWithUsers.end())
-        return;
-
-    auto& users = it->value;
-    users.remove(pageID);
-    if (users.isEmpty()) {
-        m_visitedLinkStoresWithUsers.remove(it);
-        visitedLinkStore.removeProcess(*this);
-    }
+void WebProcessProxy::addVisitedLinkStore(VisitedLinkStore& store)
+{
+    m_visitedLinkStores.add(&store);
+    store.addProcess(*this);
 }
 
@@ -431 +408 @@
     m_webUserContentControllerProxies.add(&proxy);
     proxy.addProcess(*this, parameters);
+}
+
+void WebProcessProxy::didDestroyVisitedLinkStore(VisitedLinkStore& store)
+{
+    ASSERT(m_visitedLinkStores.contains(&store));
+    m_visitedLinkStores.remove(&store);
 }
 
@@ -757 +740 @@
     }
 
+    for (WebPageProxy* page : m_pageMap.values()) {
+        ASSERT(this == &page->process());
+        page->processDidFinishLaunching();
+    }
+
+    for (auto* provisionalPage : m_provisionalPages) {
+        ASSERT(this == &provisionalPage->process());
+        provisionalPage->processDidFinishLaunching();
+    }
+
+
     RELEASE_ASSERT(!m_webConnection);
     m_webConnection = WebConnectionToWebProcess::create(this);
 
     m_processPool->processDidFinishLaunching(this);
-
-    for (auto* visitedLinkStore : m_visitedLinkStoresWithUsers.keys())
-        visitedLinkStore->addProcess(*this);
 
 #if PLATFORM(IOS_FAMILY)

trunk/Source/WebKit/UIProcess/WebProcessProxy.h

@@ -147 +147 @@
     virtual bool isServiceWorkerProcess() const { return false; }
 
-    void didCreateWebPageInProcess(uint64_t pageID);
-
-    void addVisitedLinkStoreUser(VisitedLinkStore&, uint64_t pageID);
-    void removeVisitedLinkStoreUser(VisitedLinkStore&, uint64_t pageID);
-
+    void addVisitedLinkStore(VisitedLinkStore&);
     void addWebUserContentControllerProxy(WebUserContentControllerProxy&, WebPageCreationParameters&);
+    void didDestroyVisitedLinkStore(VisitedLinkStore&);
     void didDestroyWebUserContentControllerProxy(WebUserContentControllerProxy&);
 
@@ -432 +429 @@
     UserInitiatedActionMap m_userInitiatedActionMap;
 
-    HashMap<VisitedLinkStore*, HashSet<uint64_t/* pageID */>> m_visitedLinkStoresWithUsers;
+    HashSet<VisitedLinkStore*> m_visitedLinkStores;
     HashSet<WebUserContentControllerProxy*> m_webUserContentControllerProxies;