<!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>[178322] releases/WebKitGTK/webkit-2.6/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/178322">178322</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-01-13 00:15:46 -0800 (Tue, 13 Jan 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/176818">r176818</a> - REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/173468">r173468</a>): Cannot step in WebInspector
https://bugs.webkit.org/show_bug.cgi?id=139260

Reviewed by Alexey Proskuryakov.

Inspector defers all loads and starts a nested runloop when it hits a breakpoint. When continuing it undefers the loads.
If the script execution was triggered from the didFinishLoading callback of the main resource then the main resource would
already be in the finished state in the network process side and setDefersLoading(false) message would end up restarting its load.
Since loads are not meant to restart the generated callbacks would assert or crash the web process when handled in the next
nested inspector runloop.

Fix by taking care that cleaned up NetworkResourceLoaders are always removed from the loader map of
the NetworkConnectionToWebProcess and so can't end up handling late messages.

No test, this requires JS debugger to trigger.

* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):

    This is now the only way to remove resource loaders.
    It is called from NetworkResourceLoader::cleanup only.

(WebKit::NetworkConnectionToWebProcess::didClose):
(WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):

    Calling abort  removes the resource loader (since it calls cleanup) so no need to do it explicitly anymore.

* NetworkProcess/NetworkConnectionToWebProcess.h:
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::start):

    We are guaranteed to be reffed by NetworkConnectionToWebProcess until cleanup so the explicit ref/deref can be removed.

(WebKit::NetworkResourceLoader::cleanup):

    Call to NetworkConnectionToWebProcess::didCleanupResourceLoader to make the loader unreachable.

* NetworkProcess/NetworkResourceLoader.h:
(WebKit::NetworkResourceLoader::identifier):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit26SourceWebKit2ChangeLog">releases/WebKitGTK/webkit-2.6/Source/WebKit2/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkConnectionToWebProcesscpp">releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkConnectionToWebProcessh">releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkResourceLoadercpp">releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkResourceLoaderh">releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit26SourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/WebKit2/ChangeLog (178321 => 178322)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/WebKit2/ChangeLog        2015-01-13 08:14:20 UTC (rev 178321)
+++ releases/WebKitGTK/webkit-2.6/Source/WebKit2/ChangeLog        2015-01-13 08:15:46 UTC (rev 178322)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2014-12-04  Antti Koivisto  &lt;antti@apple.com&gt;
+
+        REGRESSION (r173468): Cannot step in WebInspector
+        https://bugs.webkit.org/show_bug.cgi?id=139260
+
+        Reviewed by Alexey Proskuryakov.
+
+        Inspector defers all loads and starts a nested runloop when it hits a breakpoint. When continuing it undefers the loads.
+        If the script execution was triggered from the didFinishLoading callback of the main resource then the main resource would
+        already be in the finished state in the network process side and setDefersLoading(false) message would end up restarting its load.
+        Since loads are not meant to restart the generated callbacks would assert or crash the web process when handled in the next
+        nested inspector runloop.
+
+        Fix by taking care that cleaned up NetworkResourceLoaders are always removed from the loader map of
+        the NetworkConnectionToWebProcess and so can't end up handling late messages.
+
+        No test, this requires JS debugger to trigger.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
+
+            This is now the only way to remove resource loaders.
+            It is called from NetworkResourceLoader::cleanup only.
+
+        (WebKit::NetworkConnectionToWebProcess::didClose):
+        (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
+
+            Calling abort  removes the resource loader (since it calls cleanup) so no need to do it explicitly anymore.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::start):
+
+            We are guaranteed to be reffed by NetworkConnectionToWebProcess until cleanup so the explicit ref/deref can be removed.
+
+        (WebKit::NetworkResourceLoader::cleanup):
+
+            Call to NetworkConnectionToWebProcess::didCleanupResourceLoader to make the loader unreachable.
+
+        * NetworkProcess/NetworkResourceLoader.h:
+        (WebKit::NetworkResourceLoader::identifier):
+
</ins><span class="cx"> 2014-11-29  Anders Carlsson  &lt;andersca@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Crash when calling WKPageClose on the originated page from within createNewPage callback
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkConnectionToWebProcesscpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp (178321 => 178322)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp        2015-01-13 08:14:20 UTC (rev 178321)
+++ releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp        2015-01-13 08:15:46 UTC (rev 178322)
</span><span class="lines">@@ -62,6 +62,12 @@
</span><span class="cx"> NetworkConnectionToWebProcess::~NetworkConnectionToWebProcess()
</span><span class="cx"> {
</span><span class="cx"> }
</span><ins>+
+void NetworkConnectionToWebProcess::didCleanupResourceLoader(NetworkResourceLoader&amp; loader)
+{
+    RefPtr&lt;NetworkResourceLoader&gt; removedLoader = m_networkResourceLoaders.take(loader.identifier());
+    ASSERT(removedLoader == &amp;loader);
+}
</ins><span class="cx">     
</span><span class="cx"> void NetworkConnectionToWebProcess::didReceiveMessage(IPC::Connection* connection, IPC::MessageDecoder&amp; decoder)
</span><span class="cx"> {
</span><span class="lines">@@ -94,14 +100,13 @@
</span><span class="cx">     // Protect ourself as we might be otherwise be deleted during this function.
</span><span class="cx">     Ref&lt;NetworkConnectionToWebProcess&gt; protector(*this);
</span><span class="cx"> 
</span><del>-    HashMap&lt;ResourceLoadIdentifier, RefPtr&lt;NetworkResourceLoader&gt;&gt;::iterator end = m_networkResourceLoaders.end();
-    for (HashMap&lt;ResourceLoadIdentifier, RefPtr&lt;NetworkResourceLoader&gt;&gt;::iterator i = m_networkResourceLoaders.begin(); i != end; ++i)
-        i-&gt;value-&gt;abort();
</del><ins>+    Vector&lt;RefPtr&lt;NetworkResourceLoader&gt;&gt; loaders;
+    copyValuesToVector(m_networkResourceLoaders, loaders);
+    for (auto&amp; loader : loaders)
+        loader-&gt;abort();
+    ASSERT(m_networkResourceLoaders.isEmpty());
</ins><span class="cx"> 
</span><span class="cx">     NetworkBlobRegistry::shared().connectionToWebProcessDidClose(this);
</span><del>-
-    m_networkResourceLoaders.clear();
-    
</del><span class="cx">     NetworkProcess::shared().removeNetworkConnectionToWebProcess(this);
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -125,7 +130,7 @@
</span><span class="cx"> 
</span><span class="cx"> void NetworkConnectionToWebProcess::removeLoadIdentifier(ResourceLoadIdentifier identifier)
</span><span class="cx"> {
</span><del>-    RefPtr&lt;NetworkResourceLoader&gt; loader = m_networkResourceLoaders.take(identifier);
</del><ins>+    RefPtr&lt;NetworkResourceLoader&gt; loader = m_networkResourceLoaders.get(identifier);
</ins><span class="cx"> 
</span><span class="cx">     // It's possible we have no loader for this identifier if the NetworkProcess crashed and this was a respawned NetworkProcess.
</span><span class="cx">     if (!loader)
</span><span class="lines">@@ -134,6 +139,7 @@
</span><span class="cx">     // Abort the load now, as the WebProcess won't be able to respond to messages any more which might lead
</span><span class="cx">     // to leaked loader resources (connections, threads, etc).
</span><span class="cx">     loader-&gt;abort();
</span><ins>+    ASSERT(!m_networkResourceLoaders.contains(identifier));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void NetworkConnectionToWebProcess::setDefersLoading(ResourceLoadIdentifier identifier, bool defers)
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkConnectionToWebProcessh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h (178321 => 178322)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h        2015-01-13 08:14:20 UTC (rev 178321)
+++ releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h        2015-01-13 08:15:46 UTC (rev 178322)
</span><span class="lines">@@ -55,6 +55,8 @@
</span><span class="cx"> 
</span><span class="cx">     bool isSerialLoadingEnabled() const { return m_serialLoadingEnabled; }
</span><span class="cx"> 
</span><ins>+    void didCleanupResourceLoader(NetworkResourceLoader&amp;);
+
</ins><span class="cx"> private:
</span><span class="cx">     NetworkConnectionToWebProcess(IPC::Connection::Identifier);
</span><span class="cx"> 
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkResourceLoadercpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (178321 => 178322)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp        2015-01-13 08:14:20 UTC (rev 178321)
+++ releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp        2015-01-13 08:15:46 UTC (rev 178322)
</span><span class="lines">@@ -131,9 +131,6 @@
</span><span class="cx">     if (m_defersLoading)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    // Explicit ref() balanced by a deref() in NetworkResourceLoader::cleanup()
-    ref();
-
</del><span class="cx">     m_networkingContext = RemoteNetworkingContext::create(sessionID(), m_parameters.shouldClearReferrerOnHTTPSToHTTPRedirect);
</span><span class="cx"> 
</span><span class="cx">     consumeSandboxExtensions();
</span><span class="lines">@@ -170,12 +167,10 @@
</span><span class="cx"> 
</span><span class="cx">     NetworkProcess::shared().networkResourceLoadScheduler().removeLoader(this);
</span><span class="cx"> 
</span><del>-    if (m_handle) {
-        // Explicit deref() balanced by a ref() in NetworkResourceLoader::start()
-        // This might cause the NetworkResourceLoader to be destroyed and therefore we do it last.
-        m_handle = 0;
-        deref();
-    }
</del><ins>+    m_handle = nullptr;
+
+    // This will cause NetworkResourceLoader to be destroyed and therefore we do it last.
+    m_connection-&gt;didCleanupResourceLoader(*this);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void NetworkResourceLoader::didConvertHandleToDownload()
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceWebKit2NetworkProcessNetworkResourceLoaderh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h (178321 => 178322)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h        2015-01-13 08:14:20 UTC (rev 178321)
+++ releases/WebKitGTK/webkit-2.6/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h        2015-01-13 08:15:46 UTC (rev 178322)
</span><span class="lines">@@ -102,6 +102,7 @@
</span><span class="cx"> 
</span><span class="cx">     NetworkConnectionToWebProcess* connectionToWebProcess() const { return m_connection.get(); }
</span><span class="cx">     WebCore::SessionID sessionID() const { return m_parameters.sessionID; }
</span><ins>+    ResourceLoadIdentifier identifier() const { return m_parameters.identifier; }
</ins><span class="cx"> 
</span><span class="cx">     struct SynchronousLoadData;
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>