Unreviewed, rolling out r243142.
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 18:38:59 +0000 (18:38 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 18:38:59 +0000 (18:38 +0000)
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

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
Source/WebKit/UIProcess/ProvisionalPageProxy.h
Source/WebKit/UIProcess/VisitedLinkStore.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.h

index 831dda3..c129712 100644 (file)
@@ -1,3 +1,16 @@
+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>
 
         [iOS] Focus not preserved when switching between tabs
index bbff68b..09a28b0 100644 (file)
@@ -99,7 +99,6 @@ ProvisionalPageProxy::~ProvisionalPageProxy()
 
     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()] {
         process->maybeShutDown();
@@ -137,6 +136,21 @@ void ProvisionalPageProxy::cancel()
     didFailProvisionalLoadForFrame(m_mainFrame->frameID(), { }, m_navigationID, m_provisionalLoadURL, error, UserData { }); // Will delete |this|.
 }
 
+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()
 {
     m_drawingArea = m_page.pageClient().createDrawingAreaProxy(m_process);
@@ -144,7 +158,9 @@ void ProvisionalPageProxy::initializeWebPage()
     auto parameters = m_page.creationParameters(m_process, *m_drawingArea);
     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();
 }
 
 void ProvisionalPageProxy::loadData(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, Optional<WebsitePoliciesData>&& websitePolicies)
index 1ce9780..16bd5cd 100644 (file)
@@ -79,6 +79,7 @@ public:
     void goToBackForwardItem(API::Navigation&, WebBackForwardListItem&, Optional<WebsitePoliciesData>&&);
     void cancel();
 
+    void processDidFinishLaunching();
     void processDidTerminate();
     void connectionWillOpen(IPC::Connection&);
 
@@ -114,6 +115,7 @@ private:
 #endif
 
     void initializeWebPage();
+    void finishInitializingWebPageAfterProcessLaunch();
 
     WebPageProxy& m_page;
     Ref<WebProcessProxy> m_process;
index fac3cdd..29bf944 100644 (file)
@@ -42,7 +42,10 @@ Ref<VisitedLinkStore> VisitedLinkStore::create()
 
 VisitedLinkStore::~VisitedLinkStore()
 {
-    RELEASE_ASSERT(m_processes.isEmpty());
+    for (WebProcessProxy* process : m_processes) {
+        process->removeMessageReceiver(Messages::VisitedLinkStore::messageReceiverName(), identifier());
+        process->didDestroyVisitedLinkStore(*this);
+    }
 }
 
 VisitedLinkStore::VisitedLinkStore()
@@ -53,7 +56,6 @@ VisitedLinkStore::VisitedLinkStore()
 void VisitedLinkStore::addProcess(WebProcessProxy& process)
 {
     ASSERT(process.state() == WebProcessProxy::State::Running);
-    ASSERT(!m_processes.contains(&process));
 
     if (!m_processes.add(&process).isNewEntry)
         return;
index cb56e4f..4bb0141 100644 (file)
@@ -819,6 +819,7 @@ void WebPageProxy::finishAttachingToWebProcess(IsProcessSwap isProcessSwap)
         // when the process was provisional.
         if (isProcessSwap != IsProcessSwap::Yes)
             m_webProcessLifetimeTracker.webPageEnteringWebProcess(m_process);
+        processDidFinishLaunching();
     }
 
     updateActivityState();
@@ -957,7 +958,19 @@ void WebPageProxy::initializeWebPage()
 
     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);
 }
 
 void WebPageProxy::close()
@@ -5100,6 +5113,12 @@ void WebPageProxy::webProcessWillShutDown()
     m_webProcessLifetimeTracker.webPageLeavingWebProcess(m_process);
 }
 
+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,6 +6924,8 @@ void WebPageProxy::resetStateAfterProcessExited(ProcessTerminationReason termina
     m_hasRunningProcess = false;
     m_isPageSuspended = false;
 
+    m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
+
     m_editorState = EditorState();
     m_cachedFontAttributesAtSelectionStart.reset();
 
index 4c05c24..4c6cebf 100644 (file)
@@ -1233,6 +1233,8 @@ public:
     void connectionWillOpen(IPC::Connection&);
     void webProcessWillShutDown();
 
+    void processDidFinishLaunching();
+
     void didSaveToPageCache();
         
     void setScrollPinningBehavior(WebCore::ScrollPinningBehavior);
@@ -1972,6 +1974,8 @@ private:
 
     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);
 
@@ -2221,6 +2225,8 @@ private:
     // Whether it can run modal child web pages.
     bool m_canRunModal { false };
 
+    bool m_needsToFinishInitializingWebPageAfterProcessLaunch { false };
+
     bool m_isInPrintingMode { false };
     bool m_isPerformingDOMPrintOperation { false };
 
index 9a37395..46e8316 100644 (file)
@@ -298,9 +298,9 @@ void WebProcessProxy::shutDown()
         frame->webProcessWillShutDown();
     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)
         webUserContentControllerProxy->removeProcess(*this);
@@ -392,38 +392,15 @@ void WebProcessProxy::removeWebPage(WebPageProxy& webPage, EndsUsingDataStore en
     if (endsUsingDataStore == EndsUsingDataStore::Yes)
         m_processPool->pageEndUsingWebsiteDataStore(webPage.pageID(), webPage.websiteDataStore());
 
-    removeVisitedLinkStoreUser(webPage.visitedLinkStore(), webPage.pageID());
-
     updateBackgroundResponsivenessTimer();
 
     maybeShutDown();
 }
 
-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)
+void WebProcessProxy::addVisitedLinkStore(VisitedLinkStore& store)
 {
-    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);
-    }
+    m_visitedLinkStores.add(&store);
+    store.addProcess(*this);
 }
 
 void WebProcessProxy::addWebUserContentControllerProxy(WebUserContentControllerProxy& proxy, WebPageCreationParameters& parameters)
@@ -432,6 +409,12 @@ void WebProcessProxy::addWebUserContentControllerProxy(WebUserContentControllerP
     proxy.addProcess(*this, parameters);
 }
 
+void WebProcessProxy::didDestroyVisitedLinkStore(VisitedLinkStore& store)
+{
+    ASSERT(m_visitedLinkStores.contains(&store));
+    m_visitedLinkStores.remove(&store);
+}
+
 void WebProcessProxy::didDestroyWebUserContentControllerProxy(WebUserContentControllerProxy& proxy)
 {
     ASSERT(m_webUserContentControllerProxies.contains(&proxy));
@@ -756,14 +739,22 @@ void WebProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connect
         return;
     }
 
+    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)
     if (connection()) {
         if (xpc_connection_t xpcConnection = connection()->xpcConnection())
index 276490d..ff6a9ec 100644 (file)
@@ -146,12 +146,9 @@ public:
 
     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&);
 
     RefPtr<API::UserInitiatedAction> userInitiatedActivity(uint64_t);
@@ -431,7 +428,7 @@ private:
     HashSet<ProvisionalPageProxy*> m_provisionalPages;
     UserInitiatedActionMap m_userInitiatedActionMap;
 
-    HashMap<VisitedLinkStore*, HashSet<uint64_t/* pageID */>> m_visitedLinkStoresWithUsers;
+    HashSet<VisitedLinkStore*> m_visitedLinkStores;
     HashSet<WebUserContentControllerProxy*> m_webUserContentControllerProxies;
 
     int m_numberOfTimesSuddenTerminationWasDisabled;