<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[172660] trunk/Source/WebKit2</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/172660">172660</a></dd>
<dt>Author</dt> <dd>ap@apple.com</dd>
<dt>Date</dt> <dd>2014-08-15 16:56:57 -0700 (Fri, 15 Aug 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Improve page to process relationship tracking
https://bugs.webkit.org/show_bug.cgi?id=135996
<rdar://problem/16991213>
Reviewed by Sam Weinig.
* UIProcess/VisitedLinkProvider.cpp:
(WebKit::VisitedLinkProvider::removeAll):
(WebKit::VisitedLinkProvider::pendingVisitedLinksTimerFired):
(WebKit::VisitedLinkProvider::sendTable):
Added assertions for m_processes only having valid entries.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::reattachToWebProcess): When attaching to a new process,
tell the old process that the page is not longer associated with it, avoiding
a potential stale pointer.
If re-attached to an existing process, make sure that we perform all the same
registrations as after having launched a new process. This substantially improves
the behavior when the number of open tabs is over process limit.
(WebKit::WebPageProxy::reattachToWebProcessWithItem): Added ASSERT(!isValid())
to avoid confusion. All other calls to reattachToWebProcess() have this as a
runtime check, but reattachToWebProcessWithItem() is only called for valid pages.
(WebKit::WebPageProxy::terminateProcess): Added an assertion with a FIXME for
something that will need to be fixed another day.
* UIProcess/WebPageProxy.h: Removed an unimplemented function.
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::addExistingWebPage): Added assertions for page map sanity.
(WebKit::WebProcessProxy::removeWebPage): Added a check for page state being Terminated
already. This avoids an assertion failure that happened under the new call to
removeWebPage() in reattachToWebProcess(), as we are now calling it for terminated
processes that are not in WebContext::m_processes any more.
(WebKit::WebProcessProxy::didFinishLaunching): Added an assertion that page agrees
about using this process.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2UIProcessVisitedLinkProvidercpp">trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp</a></li>
<li><a href="#trunkSourceWebKit2UIProcessWebPageProxycpp">trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp</a></li>
<li><a href="#trunkSourceWebKit2UIProcessWebPageProxyh">trunk/Source/WebKit2/UIProcess/WebPageProxy.h</a></li>
<li><a href="#trunkSourceWebKit2UIProcessWebProcessProxycpp">trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (172659 => 172660)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2014-08-15 23:35:21 UTC (rev 172659)
+++ trunk/Source/WebKit2/ChangeLog 2014-08-15 23:56:57 UTC (rev 172660)
</span><span class="lines">@@ -1,3 +1,41 @@
</span><ins>+2014-08-15 Alexey Proskuryakov <ap@apple.com>
+
+ Improve page to process relationship tracking
+ https://bugs.webkit.org/show_bug.cgi?id=135996
+ <rdar://problem/16991213>
+
+ Reviewed by Sam Weinig.
+
+ * UIProcess/VisitedLinkProvider.cpp:
+ (WebKit::VisitedLinkProvider::removeAll):
+ (WebKit::VisitedLinkProvider::pendingVisitedLinksTimerFired):
+ (WebKit::VisitedLinkProvider::sendTable):
+ Added assertions for m_processes only having valid entries.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::reattachToWebProcess): When attaching to a new process,
+ tell the old process that the page is not longer associated with it, avoiding
+ a potential stale pointer.
+ If re-attached to an existing process, make sure that we perform all the same
+ registrations as after having launched a new process. This substantially improves
+ the behavior when the number of open tabs is over process limit.
+ (WebKit::WebPageProxy::reattachToWebProcessWithItem): Added ASSERT(!isValid())
+ to avoid confusion. All other calls to reattachToWebProcess() have this as a
+ runtime check, but reattachToWebProcessWithItem() is only called for valid pages.
+ (WebKit::WebPageProxy::terminateProcess): Added an assertion with a FIXME for
+ something that will need to be fixed another day.
+
+ * UIProcess/WebPageProxy.h: Removed an unimplemented function.
+
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::addExistingWebPage): Added assertions for page map sanity.
+ (WebKit::WebProcessProxy::removeWebPage): Added a check for page state being Terminated
+ already. This avoids an assertion failure that happened under the new call to
+ removeWebPage() in reattachToWebProcess(), as we are now calling it for terminated
+ processes that are not in WebContext::m_processes any more.
+ (WebKit::WebProcessProxy::didFinishLaunching): Added an assertion that page agrees
+ about using this process.
+
</ins><span class="cx"> 2014-08-15 Gavin Barraclough <barraclough@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix plugin visibility check.
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessVisitedLinkProvidercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp (172659 => 172660)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp 2014-08-15 23:35:21 UTC (rev 172659)
+++ trunk/Source/WebKit2/UIProcess/VisitedLinkProvider.cpp 2014-08-15 23:56:57 UTC (rev 172660)
</span><span class="lines">@@ -97,8 +97,11 @@
</span><span class="cx"> m_tableSize = 0;
</span><span class="cx"> m_table.clear();
</span><span class="cx">
</span><del>- for (auto& processAndCount : m_processes)
- processAndCount.key->connection()->send(Messages::VisitedLinkTableController::RemoveAllVisitedLinks(), m_identifier);
</del><ins>+ for (auto& processAndCount : m_processes) {
+ WebProcessProxy* process = processAndCount.key;
+ ASSERT(process->context().processes().contains(process));
+ process->connection()->send(Messages::VisitedLinkTableController::RemoveAllVisitedLinks(), m_identifier);
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void VisitedLinkProvider::addVisitedLinkHash(LinkHash linkHash)
</span><span class="lines">@@ -175,10 +178,13 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> for (auto& processAndCount : m_processes) {
</span><ins>+ WebProcessProxy* process = processAndCount.key;
+ ASSERT(process->context().processes().contains(process));
+
</ins><span class="cx"> if (addedVisitedLinks.size() > 20)
</span><del>- processAndCount.key->connection()->send(Messages::VisitedLinkTableController::AllVisitedLinkStateChanged(), m_identifier);
</del><ins>+ process->connection()->send(Messages::VisitedLinkTableController::AllVisitedLinkStateChanged(), m_identifier);
</ins><span class="cx"> else
</span><del>- processAndCount.key->connection()->send(Messages::VisitedLinkTableController::VisitedLinkStateChanged(addedVisitedLinks), m_identifier);
</del><ins>+ process->connection()->send(Messages::VisitedLinkTableController::VisitedLinkStateChanged(addedVisitedLinks), m_identifier);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -229,6 +235,8 @@
</span><span class="cx">
</span><span class="cx"> void VisitedLinkProvider::sendTable(WebProcessProxy& process)
</span><span class="cx"> {
</span><ins>+ ASSERT(process.context().processes().contains(&process));
+
</ins><span class="cx"> SharedMemory::Handle handle;
</span><span class="cx"> if (!m_table.sharedMemory()->createHandle(handle, SharedMemory::ReadOnly))
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebPageProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (172659 => 172660)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2014-08-15 23:35:21 UTC (rev 172659)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2014-08-15 23:56:57 UTC (rev 172660)
</span><span class="lines">@@ -545,11 +545,16 @@
</span><span class="cx"> ASSERT(m_process->state() == WebProcessProxy::State::Terminated);
</span><span class="cx">
</span><span class="cx"> m_isValid = true;
</span><ins>+ m_process->removeWebPage(m_pageID);
</ins><span class="cx">
</span><span class="cx"> if (m_process->context().processModel() == ProcessModelSharedSecondaryProcess)
</span><span class="cx"> m_process = m_process->context().ensureSharedWebProcess();
</span><span class="cx"> else
</span><span class="cx"> m_process = m_process->context().createNewWebProcessRespectingProcessCountLimit();
</span><ins>+
+ ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
+ if (m_process->state() == ChildProcessProxy::State::Running)
+ processDidFinishLaunching();
</ins><span class="cx"> m_process->addExistingWebPage(this, m_pageID);
</span><span class="cx"> m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
</span><span class="cx">
</span><span class="lines">@@ -579,7 +584,8 @@
</span><span class="cx">
</span><span class="cx"> if (item && item != m_backForwardList->currentItem())
</span><span class="cx"> m_backForwardList->goToItem(item);
</span><del>-
</del><ins>+
+ ASSERT(!isValid());
</ins><span class="cx"> reattachToWebProcess();
</span><span class="cx">
</span><span class="cx"> if (!item)
</span><span class="lines">@@ -1865,6 +1871,10 @@
</span><span class="cx">
</span><span class="cx"> void WebPageProxy::terminateProcess()
</span><span class="cx"> {
</span><ins>+ // requestTermination() is a no-op for launching processes, so we get into an inconsistent state by calling resetStateAfterProcessExited().
+ // FIXME: A client can terminate the page at any time, so we should do something more meaningful than assert and fall apart in release builds.
+ ASSERT(m_process->state() != WebProcessProxy::State::Launching);
+
</ins><span class="cx"> // NOTE: This uses a check of m_isValid rather than calling isValid() since
</span><span class="cx"> // we want this to run even for pages being closed or that already closed.
</span><span class="cx"> if (!m_isValid)
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebPageProxyh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (172659 => 172660)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2014-08-15 23:35:21 UTC (rev 172659)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2014-08-15 23:56:57 UTC (rev 172660)
</span><span class="lines">@@ -740,8 +740,6 @@
</span><span class="cx">
</span><span class="cx"> bool isValid() const;
</span><span class="cx">
</span><del>- PassRefPtr<API::Array> relatedPages() const;
-
</del><span class="cx"> const String& urlAtProcessExit() const { return m_urlAtProcessExit; }
</span><span class="cx"> FrameLoadState::State loadStateAtProcessExit() const { return m_loadStateAtProcessExit; }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebProcessProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (172659 => 172660)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2014-08-15 23:35:21 UTC (rev 172659)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2014-08-15 23:56:57 UTC (rev 172660)
</span><span class="lines">@@ -186,6 +186,9 @@
</span><span class="cx">
</span><span class="cx"> void WebProcessProxy::addExistingWebPage(WebPageProxy* webPage, uint64_t pageID)
</span><span class="cx"> {
</span><ins>+ ASSERT(!m_pageMap.contains(pageID));
+ ASSERT(!globalPageMap().contains(pageID));
+
</ins><span class="cx"> m_pageMap.set(pageID, webPage);
</span><span class="cx"> globalPageMap().set(pageID, webPage);
</span><span class="cx"> #if PLATFORM(COCOA)
</span><span class="lines">@@ -215,7 +218,7 @@
</span><span class="cx">
</span><span class="cx"> // If this was the last WebPage open in that web process, and we have no other reason to keep it alive, let it go.
</span><span class="cx"> // We only allow this when using a network process, as otherwise the WebProcess needs to preserve its session state.
</span><del>- if (!m_context->usesNetworkProcess() || !canTerminateChildProcess())
</del><ins>+ if (!m_context->usesNetworkProcess() || state() == State::Terminated || !canTerminateChildProcess())
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> abortProcessLaunchIfNeeded();
</span><span class="lines">@@ -471,8 +474,10 @@
</span><span class="cx"> {
</span><span class="cx"> ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
</span><span class="cx">
</span><del>- for (auto& page : m_pageMap.values())
</del><ins>+ for (WebPageProxy* page : m_pageMap.values()) {
+ ASSERT(this == &page->process());
</ins><span class="cx"> page->processDidFinishLaunching();
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> m_webConnection = WebConnectionToWebProcess::create(this);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>