- Revision
- 243142
- Author
- [email protected]
- Date
- 2019-03-19 09:48:40 -0700 (Tue, 19 Mar 2019)
Log Message
Spew: Unhandled web process message 'VisitedLinkTableController:VisitedLinkStateChanged'
https://bugs.webkit.org/show_bug.cgi?id=194787
<rdar://problem/48175520>
Reviewed by Geoffrey Garen.
The unhandled 'VisitedLinkTableController:VisitedLinkStateChanged' message logging happens
when IPC is sent to a WebProcess which does not have a VisitedLinkTableController with the
given identifier. VisitedLinkTableController are kept alive by the WebPage in the WebProcess
side so this indicates that there is no WebPage using this VisitedLinkTableController anymore.
In the UIProcess side, our tracking of who is using which VisitedLinkStore was very poor.
WebPageProxy objects would ask their process to register itself with the page's visitedLinkStore
as soon as the WebPage object has been created on the WebProcess side. This part was fine.
However, unregistration from the visitedLinkStores would only happen when either the
visitedLinkStore would get destroyed or when the WebProcess would shutdown. This means that
WebProcess could stay registered with a visitedLinkStore even after the page that was using it
has been closed, which would lead to such logging.
To address the issue, the WebProcessProxy now keeps track for which pages are using which
visitedLinkStore. When a visitedLinkStore is used by a page for the first time, the
WebProcessProxy will register itself with the visitedLinkStore. Similarly, when the last page
using a given visitedLinkStore is closed, the process unregisters itself from the
visitedLinkStore, thus avoiding the bug.
I also simplified a lot the logic for having a page telling the WebProcessProxy it started
using a visitedLinkStore. Previously, it would have to wait until the process is done launching
before notifying the WebProcessProxy. Now, the WebPageProxy merely tells the WebProcessProxy
that it is starting to use a visitedLinkStore as soon as it sent the CreateWebPage IPC to the
WebProcess (no matter if the process is still launching or not). At this point, the
WebProcessProxy registers the page as a user of the visitedLinkStore and takes care of waiting
until it is done launching before registering itself with the visitedLinkStore.
* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
(WebKit::ProvisionalPageProxy::initializeWebPage):
(WebKit::ProvisionalPageProxy::processDidFinishLaunching): Deleted.
(WebKit::ProvisionalPageProxy::finishInitializingWebPageAfterProcessLaunch): Deleted.
* UIProcess/ProvisionalPageProxy.h:
* UIProcess/VisitedLinkStore.cpp:
(WebKit::VisitedLinkStore::~VisitedLinkStore):
(WebKit::VisitedLinkStore::addProcess):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::finishAttachingToWebProcess):
(WebKit::WebPageProxy::initializeWebPage):
(WebKit::WebPageProxy::resetStateAfterProcessExited):
(WebKit::WebPageProxy::finishInitializingWebPageAfterProcessLaunch): Deleted.
(WebKit::WebPageProxy::processDidFinishLaunching): Deleted.
* UIProcess/WebPageProxy.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::addVisitedLinkStoreUser):
(WebKit::WebProcessProxy::removeVisitedLinkStoreUser):
(WebKit::WebProcessProxy::addWebUserContentControllerProxy):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::addVisitedLinkStore): Deleted.
(WebKit::WebProcessProxy::didDestroyVisitedLinkStore): Deleted.
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (243141 => 243142)
--- trunk/Source/WebKit/ChangeLog 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/ChangeLog 2019-03-19 16:48:40 UTC (rev 243142)
@@ -1,3 +1,65 @@
+2019-03-19 Chris Dumez <[email protected]>
+
+ Spew: Unhandled web process message 'VisitedLinkTableController:VisitedLinkStateChanged'
+ https://bugs.webkit.org/show_bug.cgi?id=194787
+ <rdar://problem/48175520>
+
+ Reviewed by Geoffrey Garen.
+
+ The unhandled 'VisitedLinkTableController:VisitedLinkStateChanged' message logging happens
+ when IPC is sent to a WebProcess which does not have a VisitedLinkTableController with the
+ given identifier. VisitedLinkTableController are kept alive by the WebPage in the WebProcess
+ side so this indicates that there is no WebPage using this VisitedLinkTableController anymore.
+
+ In the UIProcess side, our tracking of who is using which VisitedLinkStore was very poor.
+ WebPageProxy objects would ask their process to register itself with the page's visitedLinkStore
+ as soon as the WebPage object has been created on the WebProcess side. This part was fine.
+ However, unregistration from the visitedLinkStores would only happen when either the
+ visitedLinkStore would get destroyed or when the WebProcess would shutdown. This means that
+ WebProcess could stay registered with a visitedLinkStore even after the page that was using it
+ has been closed, which would lead to such logging.
+
+ To address the issue, the WebProcessProxy now keeps track for which pages are using which
+ visitedLinkStore. When a visitedLinkStore is used by a page for the first time, the
+ WebProcessProxy will register itself with the visitedLinkStore. Similarly, when the last page
+ using a given visitedLinkStore is closed, the process unregisters itself from the
+ visitedLinkStore, thus avoiding the bug.
+
+ I also simplified a lot the logic for having a page telling the WebProcessProxy it started
+ using a visitedLinkStore. Previously, it would have to wait until the process is done launching
+ before notifying the WebProcessProxy. Now, the WebPageProxy merely tells the WebProcessProxy
+ that it is starting to use a visitedLinkStore as soon as it sent the CreateWebPage IPC to the
+ WebProcess (no matter if the process is still launching or not). At this point, the
+ WebProcessProxy registers the page as a user of the visitedLinkStore and takes care of waiting
+ until it is done launching before registering itself with the visitedLinkStore.
+
+ * UIProcess/ProvisionalPageProxy.cpp:
+ (WebKit::ProvisionalPageProxy::~ProvisionalPageProxy):
+ (WebKit::ProvisionalPageProxy::initializeWebPage):
+ (WebKit::ProvisionalPageProxy::processDidFinishLaunching): Deleted.
+ (WebKit::ProvisionalPageProxy::finishInitializingWebPageAfterProcessLaunch): Deleted.
+ * UIProcess/ProvisionalPageProxy.h:
+ * UIProcess/VisitedLinkStore.cpp:
+ (WebKit::VisitedLinkStore::~VisitedLinkStore):
+ (WebKit::VisitedLinkStore::addProcess):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::finishAttachingToWebProcess):
+ (WebKit::WebPageProxy::initializeWebPage):
+ (WebKit::WebPageProxy::resetStateAfterProcessExited):
+ (WebKit::WebPageProxy::finishInitializingWebPageAfterProcessLaunch): Deleted.
+ (WebKit::WebPageProxy::processDidFinishLaunching): Deleted.
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::shutDown):
+ (WebKit::WebProcessProxy::removeWebPage):
+ (WebKit::WebProcessProxy::addVisitedLinkStoreUser):
+ (WebKit::WebProcessProxy::removeVisitedLinkStoreUser):
+ (WebKit::WebProcessProxy::addWebUserContentControllerProxy):
+ (WebKit::WebProcessProxy::didFinishLaunching):
+ (WebKit::WebProcessProxy::addVisitedLinkStore): Deleted.
+ (WebKit::WebProcessProxy::didDestroyVisitedLinkStore): Deleted.
+ * UIProcess/WebProcessProxy.h:
+
2019-03-19 Alex Christensen <[email protected]>
Make WTFLogChannelState and WTFLogLevel enum classes
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp 2019-03-19 16:48:40 UTC (rev 243142)
@@ -99,6 +99,7 @@
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();
@@ -136,21 +137,6 @@
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);
@@ -158,9 +144,7 @@
auto parameters = m_page.creationParameters(m_process, *m_drawingArea);
parameters.isProcessSwap = true;
m_process->send(Messages::WebProcess::CreateWebPage(m_page.pageID(), parameters), 0);
-
- if (m_process->state() == WebProcessProxy::State::Running)
- finishInitializingWebPageAfterProcessLaunch();
+ m_process->addVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.pageID());
}
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)
Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.h 2019-03-19 16:48:40 UTC (rev 243142)
@@ -79,7 +79,6 @@
void goToBackForwardItem(API::Navigation&, WebBackForwardListItem&, Optional<WebsitePoliciesData>&&);
void cancel();
- void processDidFinishLaunching();
void processDidTerminate();
void connectionWillOpen(IPC::Connection&);
@@ -115,7 +114,6 @@
#endif
void initializeWebPage();
- void finishInitializingWebPageAfterProcessLaunch();
WebPageProxy& m_page;
Ref<WebProcessProxy> m_process;
Modified: trunk/Source/WebKit/UIProcess/VisitedLinkStore.cpp (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/VisitedLinkStore.cpp 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/VisitedLinkStore.cpp 2019-03-19 16:48:40 UTC (rev 243142)
@@ -42,10 +42,7 @@
VisitedLinkStore::~VisitedLinkStore()
{
- for (WebProcessProxy* process : m_processes) {
- process->removeMessageReceiver(Messages::VisitedLinkStore::messageReceiverName(), identifier());
- process->didDestroyVisitedLinkStore(*this);
- }
+ RELEASE_ASSERT(m_processes.isEmpty());
}
VisitedLinkStore::VisitedLinkStore()
@@ -56,6 +53,7 @@
void VisitedLinkStore::addProcess(WebProcessProxy& process)
{
ASSERT(process.state() == WebProcessProxy::State::Running);
+ ASSERT(!m_processes.contains(&process));
if (!m_processes.add(&process).isNewEntry)
return;
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-03-19 16:48:40 UTC (rev 243142)
@@ -819,7 +819,6 @@
// when the process was provisional.
if (isProcessSwap != IsProcessSwap::Yes)
m_webProcessLifetimeTracker.webPageEnteringWebProcess(m_process);
- processDidFinishLaunching();
}
updateActivityState();
@@ -958,21 +957,9 @@
process().send(Messages::WebProcess::CreateWebPage(m_pageID, creationParameters(m_process, *m_drawingArea)), 0);
- m_needsToFinishInitializingWebPageAfterProcessLaunch = true;
- finishInitializingWebPageAfterProcessLaunch();
+ m_process->addVisitedLinkStoreUser(visitedLinkStore(), m_pageID);
}
-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()
{
if (m_isClosed)
@@ -5113,12 +5100,6 @@
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)
{
@@ -6924,8 +6905,6 @@
m_hasRunningProcess = false;
m_isPageSuspended = false;
- m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
-
m_editorState = EditorState();
m_cachedFontAttributesAtSelectionStart.reset();
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-03-19 16:48:40 UTC (rev 243142)
@@ -1233,8 +1233,6 @@
void connectionWillOpen(IPC::Connection&);
void webProcessWillShutDown();
- void processDidFinishLaunching();
-
void didSaveToPageCache();
void setScrollPinningBehavior(WebCore::ScrollPinningBehavior);
@@ -1974,8 +1972,6 @@
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);
@@ -2225,8 +2221,6 @@
// Whether it can run modal child web pages.
bool m_canRunModal { false };
- bool m_needsToFinishInitializingWebPageAfterProcessLaunch { false };
-
bool m_isInPrintingMode { false };
bool m_isPerformingDOMPrintOperation { false };
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-03-19 16:48:40 UTC (rev 243142)
@@ -298,9 +298,9 @@
frame->webProcessWillShutDown();
m_frameMap.clear();
- for (auto* visitedLinkStore : m_visitedLinkStores)
+ for (auto* visitedLinkStore : m_visitedLinkStoresWithUsers.keys())
visitedLinkStore->removeProcess(*this);
- m_visitedLinkStores.clear();
+ m_visitedLinkStoresWithUsers.clear();
for (auto* webUserContentControllerProxy : m_webUserContentControllerProxies)
webUserContentControllerProxy->removeProcess(*this);
@@ -392,17 +392,40 @@
if (endsUsingDataStore == EndsUsingDataStore::Yes)
m_processPool->pageEndUsingWebsiteDataStore(webPage.pageID(), webPage.websiteDataStore());
+ removeVisitedLinkStoreUser(webPage.visitedLinkStore(), webPage.pageID());
+
updateBackgroundResponsivenessTimer();
maybeShutDown();
}
-void WebProcessProxy::addVisitedLinkStore(VisitedLinkStore& store)
+void WebProcessProxy::addVisitedLinkStoreUser(VisitedLinkStore& visitedLinkStore, uint64_t pageID)
{
- m_visitedLinkStores.add(&store);
- store.addProcess(*this);
+ 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::addWebUserContentControllerProxy(WebUserContentControllerProxy& proxy, WebPageCreationParameters& parameters)
{
m_webUserContentControllerProxies.add(&proxy);
@@ -409,12 +432,6 @@
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));
@@ -739,22 +756,14 @@
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())
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (243141 => 243142)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-03-19 16:47:04 UTC (rev 243141)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-03-19 16:48:40 UTC (rev 243142)
@@ -146,9 +146,12 @@
virtual bool isServiceWorkerProcess() const { return false; }
- void addVisitedLinkStore(VisitedLinkStore&);
+ void didCreateWebPageInProcess(uint64_t pageID);
+
+ void addVisitedLinkStoreUser(VisitedLinkStore&, uint64_t pageID);
+ void removeVisitedLinkStoreUser(VisitedLinkStore&, uint64_t pageID);
+
void addWebUserContentControllerProxy(WebUserContentControllerProxy&, WebPageCreationParameters&);
- void didDestroyVisitedLinkStore(VisitedLinkStore&);
void didDestroyWebUserContentControllerProxy(WebUserContentControllerProxy&);
RefPtr<API::UserInitiatedAction> userInitiatedActivity(uint64_t);
@@ -428,7 +431,7 @@
HashSet<ProvisionalPageProxy*> m_provisionalPages;
UserInitiatedActionMap m_userInitiatedActionMap;
- HashSet<VisitedLinkStore*> m_visitedLinkStores;
+ HashMap<VisitedLinkStore*, HashSet<uint64_t/* pageID */>> m_visitedLinkStoresWithUsers;
HashSet<WebUserContentControllerProxy*> m_webUserContentControllerProxies;
int m_numberOfTimesSuddenTerminationWasDisabled;
