Title: [176818] trunk/Source/WebKit2
- Revision
- 176818
- Author
- [email protected]
- Date
- 2014-12-04 14:48:24 -0800 (Thu, 04 Dec 2014)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (176817 => 176818)
--- trunk/Source/WebKit2/ChangeLog 2014-12-04 22:20:08 UTC (rev 176817)
+++ trunk/Source/WebKit2/ChangeLog 2014-12-04 22:48:24 UTC (rev 176818)
@@ -1,3 +1,45 @@
+2014-12-04 Antti Koivisto <[email protected]>
+
+ 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):
+
2014-12-04 Timothy Horton <[email protected]>
Fix the 32-bit build.
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp (176817 => 176818)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2014-12-04 22:20:08 UTC (rev 176817)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2014-12-04 22:48:24 UTC (rev 176818)
@@ -62,6 +62,12 @@
NetworkConnectionToWebProcess::~NetworkConnectionToWebProcess()
{
}
+
+void NetworkConnectionToWebProcess::didCleanupResourceLoader(NetworkResourceLoader& loader)
+{
+ RefPtr<NetworkResourceLoader> removedLoader = m_networkResourceLoaders.take(loader.identifier());
+ ASSERT(removedLoader == &loader);
+}
void NetworkConnectionToWebProcess::didReceiveMessage(IPC::Connection* connection, IPC::MessageDecoder& decoder)
{
@@ -94,14 +100,13 @@
// Protect ourself as we might be otherwise be deleted during this function.
Ref<NetworkConnectionToWebProcess> protector(*this);
- HashMap<ResourceLoadIdentifier, RefPtr<NetworkResourceLoader>>::iterator end = m_networkResourceLoaders.end();
- for (HashMap<ResourceLoadIdentifier, RefPtr<NetworkResourceLoader>>::iterator i = m_networkResourceLoaders.begin(); i != end; ++i)
- i->value->abort();
+ Vector<RefPtr<NetworkResourceLoader>> loaders;
+ copyValuesToVector(m_networkResourceLoaders, loaders);
+ for (auto& loader : loaders)
+ loader->abort();
+ ASSERT(m_networkResourceLoaders.isEmpty());
NetworkBlobRegistry::shared().connectionToWebProcessDidClose(this);
-
- m_networkResourceLoaders.clear();
-
NetworkProcess::shared().removeNetworkConnectionToWebProcess(this);
}
@@ -125,7 +130,7 @@
void NetworkConnectionToWebProcess::removeLoadIdentifier(ResourceLoadIdentifier identifier)
{
- RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.take(identifier);
+ RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
// It's possible we have no loader for this identifier if the NetworkProcess crashed and this was a respawned NetworkProcess.
if (!loader)
@@ -134,6 +139,7 @@
// Abort the load now, as the WebProcess won't be able to respond to messages any more which might lead
// to leaked loader resources (connections, threads, etc).
loader->abort();
+ ASSERT(!m_networkResourceLoaders.contains(identifier));
}
void NetworkConnectionToWebProcess::setDefersLoading(ResourceLoadIdentifier identifier, bool defers)
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h (176817 => 176818)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2014-12-04 22:20:08 UTC (rev 176817)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2014-12-04 22:48:24 UTC (rev 176818)
@@ -55,6 +55,8 @@
bool isSerialLoadingEnabled() const { return m_serialLoadingEnabled; }
+ void didCleanupResourceLoader(NetworkResourceLoader&);
+
private:
NetworkConnectionToWebProcess(IPC::Connection::Identifier);
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (176817 => 176818)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2014-12-04 22:20:08 UTC (rev 176817)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2014-12-04 22:48:24 UTC (rev 176818)
@@ -130,9 +130,6 @@
if (m_defersLoading)
return;
- // Explicit ref() balanced by a deref() in NetworkResourceLoader::cleanup()
- ref();
-
m_networkingContext = RemoteNetworkingContext::create(sessionID(), m_parameters.shouldClearReferrerOnHTTPSToHTTPRedirect);
consumeSandboxExtensions();
@@ -169,12 +166,10 @@
NetworkProcess::shared().networkResourceLoadScheduler().removeLoader(this);
- 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();
- }
+ m_handle = nullptr;
+
+ // This will cause NetworkResourceLoader to be destroyed and therefore we do it last.
+ m_connection->didCleanupResourceLoader(*this);
}
void NetworkResourceLoader::didConvertHandleToDownload()
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h (176817 => 176818)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2014-12-04 22:20:08 UTC (rev 176817)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2014-12-04 22:48:24 UTC (rev 176818)
@@ -101,6 +101,7 @@
NetworkConnectionToWebProcess* connectionToWebProcess() const { return m_connection.get(); }
WebCore::SessionID sessionID() const { return m_parameters.sessionID; }
+ ResourceLoadIdentifier identifier() const { return m_parameters.identifier; }
struct SynchronousLoadData;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
