Modified: trunk/Source/WebKit2/ChangeLog (127613 => 127614)
--- trunk/Source/WebKit2/ChangeLog 2012-09-05 18:16:58 UTC (rev 127613)
+++ trunk/Source/WebKit2/ChangeLog 2012-09-05 18:21:49 UTC (rev 127614)
@@ -1,3 +1,27 @@
+2012-09-05 Alexey Proskuryakov <[email protected]>
+
+ [WK2] Make visited link tracking work in multi-process mode
+ https://bugs.webkit.org/show_bug.cgi?id=95869
+
+ Reviewed by Dan Bernstein.
+
+ * UIProcess/VisitedLinkProvider.h:
+ * UIProcess/VisitedLinkProvider.cpp:
+ (WebKit::VisitedLinkProvider::VisitedLinkProvider): m_webProcessHasVisitedLinkState
+ was making no sense in multi-process world, so it was let go.
+ (WebKit::VisitedLinkProvider::processDidFinishLaunching): Track new processes.
+ (WebKit::VisitedLinkProvider::processDidClose): Clean up pointers that are going
+ to become stale.
+ (WebKit::VisitedLinkProvider::pendingVisitedLinksTimerFired): Added comments. Fixed
+ a bug where we would churn table size in some cases. Added debug logging in failure
+ case. Re-implemented messaging code to work with multiple web processes.
+
+ * UIProcess/WebContext.cpp:
+ (WebKit::WebContext::processDidFinishLaunching): Pass process proxy pointer to
+ m_visitedLinkProvider, as it now needs to track processes.
+ (WebKit::WebContext::disconnectProcess): Ditto. Also re-enabled visited link provider
+ cleanup in multi-process mode.
+
2012-09-05 Brady Eidson <[email protected]>
Frequent crashes in PluginView::scriptObject under runtimeObjectCustomGetOwnPropertySlot
Modified: trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp (127613 => 127614)
--- trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp 2012-09-05 18:16:58 UTC (rev 127613)
+++ trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp 2012-09-05 18:21:49 UTC (rev 127614)
@@ -40,16 +40,15 @@
VisitedLinkProvider::VisitedLinkProvider(WebContext* context)
: m_context(context)
, m_visitedLinksPopulated(false)
- , m_webProcessHasVisitedLinkState(false)
, m_keyCount(0)
, m_tableSize(0)
, m_pendingVisitedLinksTimer(RunLoop::main(), this, &VisitedLinkProvider::pendingVisitedLinksTimerFired)
{
}
-void VisitedLinkProvider::processDidFinishLaunching()
+void VisitedLinkProvider::processDidFinishLaunching(WebProcessProxy* process)
{
- m_webProcessHasVisitedLinkState = false;
+ m_processesWithoutVisitedLinkState.add(process);
if (m_keyCount)
m_pendingVisitedLinksTimer.startOneShot(0);
@@ -70,9 +69,10 @@
m_pendingVisitedLinksTimer.startOneShot(0);
}
-void VisitedLinkProvider::processDidClose()
+void VisitedLinkProvider::processDidClose(WebProcessProxy* process)
{
- m_pendingVisitedLinksTimer.stop();
+ m_processesWithVisitedLinkState.remove(process);
+ m_processesWithoutVisitedLinkState.remove(process);
}
static unsigned nextPowerOf2(unsigned v)
@@ -111,18 +111,26 @@
m_pendingVisitedLinks.clear();
unsigned currentTableSize = m_tableSize;
+
+ // Upper bound on needed size - some of the links may be duplicates, in which case we could have done with less.
unsigned newTableSize = tableSizeForKeyCount(m_keyCount + pendingVisitedLinks.size());
+ // Never decrease table size when adding to it, to avoid unneeded churn.
+ newTableSize = std::max(currentTableSize, newTableSize);
+
// Links that were added.
Vector<WebCore::LinkHash> addedVisitedLinks;
+ // VisitedLinkTable remains internally consistent when adding, so it's OK to modify it in place
+ // even if a web process is accessing it at the same time.
if (currentTableSize != newTableSize) {
- // Create a new table.
RefPtr<SharedMemory> newTableMemory = SharedMemory::create(newTableSize * sizeof(LinkHash));
// We failed to create the shared memory.
- if (!newTableMemory)
+ if (!newTableMemory) {
+ LOG_ERROR("Could not allocate shared memory for visited link table");
return;
+ }
memset(newTableMemory->data(), 0, newTableMemory->size());
@@ -156,27 +164,36 @@
m_keyCount += pendingVisitedLinks.size();
- if (!m_webProcessHasVisitedLinkState || currentTableSize != newTableSize) {
- // Send the new visited link table.
-
+
+ for (HashSet<WebProcessProxy*>::iterator iter = m_processesWithVisitedLinkState.begin(); iter != m_processesWithVisitedLinkState.end(); ++iter) {
+ WebProcessProxy* process = *iter;
+ if (currentTableSize != newTableSize) {
+ // In the rare case of needing to resize the table, we'll bypass the VisitedLinkStateChanged optimization,
+ // and unconditionally use AllVisitedLinkStateChanged for the process.
+ m_processesWithoutVisitedLinkState.add(process);
+ continue;
+ }
+
+ if (addedVisitedLinks.size() <= 20)
+ process->send(Messages::WebProcess::VisitedLinkStateChanged(addedVisitedLinks), 0);
+ else
+ process->send(Messages::WebProcess::AllVisitedLinkStateChanged(), 0);
+ }
+
+ for (HashSet<WebProcessProxy*>::iterator iter = m_processesWithoutVisitedLinkState.begin(); iter != m_processesWithoutVisitedLinkState.end(); ++iter) {
+ WebProcessProxy* process = *iter;
+
SharedMemory::Handle handle;
if (!m_table.sharedMemory()->createHandle(handle, SharedMemory::ReadOnly))
return;
- // FIXME (Multi-WebProcess): Encoding a handle will null it out so we need to create a new
- // handle for every process. Maybe the ArgumentEncoder should handle this.
- m_context->sendToAllProcesses(Messages::WebProcess::SetVisitedLinkTable(handle));
+ process->send(Messages::WebProcess::SetVisitedLinkTable(handle), 0);
+ process->send(Messages::WebProcess::AllVisitedLinkStateChanged(), 0);
+
+ m_processesWithVisitedLinkState.add(process);
}
- // We now need to let the web process know that we've added links.
- if (m_webProcessHasVisitedLinkState && addedVisitedLinks.size() <= 20) {
- m_context->sendToAllProcesses(Messages::WebProcess::VisitedLinkStateChanged(addedVisitedLinks));
- return;
- }
-
- // Just recalculate all the visited links.
- m_context->sendToAllProcesses(Messages::WebProcess::AllVisitedLinkStateChanged());
- m_webProcessHasVisitedLinkState = true;
+ m_processesWithoutVisitedLinkState.clear();
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.h (127613 => 127614)
--- trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.h 2012-09-05 18:16:58 UTC (rev 127613)
+++ trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.h 2012-09-05 18:21:49 UTC (rev 127614)
@@ -35,6 +35,7 @@
namespace WebKit {
class WebContext;
+class WebProcessProxy;
class VisitedLinkProvider {
WTF_MAKE_NONCOPYABLE(VisitedLinkProvider);
@@ -43,15 +44,16 @@
void addVisitedLink(WebCore::LinkHash);
- void processDidFinishLaunching();
- void processDidClose();
+ void processDidFinishLaunching(WebProcessProxy*);
+ void processDidClose(WebProcessProxy*);
private:
void pendingVisitedLinksTimerFired();
WebContext* m_context;
bool m_visitedLinksPopulated;
- bool m_webProcessHasVisitedLinkState;
+ HashSet<WebProcessProxy*> m_processesWithVisitedLinkState;
+ HashSet<WebProcessProxy*> m_processesWithoutVisitedLinkState;
unsigned m_keyCount;
unsigned m_tableSize;
Modified: trunk/Source/WebKit2/UIProcess/WebContext.cpp (127613 => 127614)
--- trunk/Source/WebKit2/UIProcess/WebContext.cpp 2012-09-05 18:16:58 UTC (rev 127613)
+++ trunk/Source/WebKit2/UIProcess/WebContext.cpp 2012-09-05 18:21:49 UTC (rev 127614)
@@ -421,7 +421,7 @@
{
ASSERT(m_processes.contains(process));
- m_visitedLinkProvider.processDidFinishLaunching();
+ m_visitedLinkProvider.processDidFinishLaunching(process);
// Sometimes the memorySampler gets initialized after process initialization has happened but before the process has finished launching
// so check if it needs to be started here
@@ -441,6 +441,8 @@
{
ASSERT(m_processes.contains(process));
+ m_visitedLinkProvider.processDidClose(process);
+
// FIXME (Multi-WebProcess): All the invalidation calls below are still necessary in multi-process mode, but they should only affect data structures pertaining to the process being disconnected.
// Clearing everything causes assertion failures, so it's less trouble to skip that for now.
if (m_processModel != ProcessModelSharedSecondaryProcess) {
@@ -449,8 +451,6 @@
return;
}
- m_visitedLinkProvider.processDidClose();
-
// Invalidate all outstanding downloads.
for (HashMap<uint64_t, RefPtr<DownloadProxy> >::iterator::Values it = m_downloads.begin().values(), end = m_downloads.end().values(); it != end; ++it) {
(*it)->processDidClose();
