Diff
Modified: trunk/Source/WebKit2/ChangeLog (149388 => 149389)
--- trunk/Source/WebKit2/ChangeLog 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/ChangeLog 2013-04-30 19:21:07 UTC (rev 149389)
@@ -1,3 +1,39 @@
+2013-04-30 Brady Eidson <[email protected]>
+
+ [WK2] Threads get stuck in NetworkProcess when canceling loads in WebProcess.
+ <rdar://problem/13757687> and https://bugs.webkit.org/show_bug.cgi?id=115319
+
+ Written and reviewed by both Alexey Proskuryakov and Brady Eidson.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::didClose): Call abort() on all loaders
+ instead of connectionToWebProcessDidClose()
+ (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier): Instead of removing
+ the loader from the scheduler, call abort() which will also end up removing it.
+
+ * NetworkProcess/SchedulableLoader.h: Add a virtual abort() call.
+
+ * NetworkProcess/NetworkResourceLoader.cpp:
+ (WebKit::NetworkResourceLoader::sendAbortingOnFailure): Added a messageFlags argument so
+ callers can pass DispatchMessageEvenWhenWaitingForSyncReply.
+ (WebKit::NetworkResourceLoader::abort): Combined "abortInProgressLoad" and "cleanup" to
+ be a general purpose "cancel"
+ (WebKit::NetworkResourceLoader::didReceiveBuffer):
+ (WebKit::NetworkResourceLoader::willSendRequestAsync):
+ (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpaceAsync):
+ * NetworkProcess/NetworkResourceLoader.h:
+
+ * NetworkProcess/SyncNetworkResourceLoader.cpp:
+ (WebKit::SyncNetworkResourceLoader::abort):
+ * NetworkProcess/SyncNetworkResourceLoader.h:
+
+ * WebProcess/Network/WebResourceLoadScheduler.cpp:
+ (WebKit::WebResourceLoadScheduler::remove): Remove an obsolete/incorrect comment.
+
+ * WebProcess/Network/WebResourceLoader.cpp:
+ (WebKit::WebResourceLoader::didReceiveResponseWithCertificateInfo): Add comment stressing
+ the importance of this null check so it's not removed again.
+
2013-04-30 Carlos Garcia Campos <[email protected]>
[WK2][GTK] MiniBrowser won't play video in <embed> tag
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2013-04-30 19:21:07 UTC (rev 149389)
@@ -98,11 +98,11 @@
HashMap<ResourceLoadIdentifier, RefPtr<NetworkResourceLoader> >::iterator end = m_networkResourceLoaders.end();
for (HashMap<ResourceLoadIdentifier, RefPtr<NetworkResourceLoader> >::iterator i = m_networkResourceLoaders.begin(); i != end; ++i)
- i->value->connectionToWebProcessDidClose();
+ i->value->abort();
HashMap<ResourceLoadIdentifier, RefPtr<SyncNetworkResourceLoader> >::iterator syncEnd = m_syncNetworkResourceLoaders.end();
for (HashMap<ResourceLoadIdentifier, RefPtr<SyncNetworkResourceLoader> >::iterator i = m_syncNetworkResourceLoaders.begin(); i != syncEnd; ++i)
- i->value->connectionToWebProcessDidClose();
+ i->value->abort();
NetworkBlobRegistry::shared().connectionToWebProcessDidClose(this);
@@ -139,7 +139,9 @@
if (!loader)
return;
- NetworkProcess::shared().networkResourceLoadScheduler().removeLoader(loader.get());
+ // 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();
}
void NetworkConnectionToWebProcess::servePendingRequests(uint32_t resourceLoadPriority)
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2013-04-30 19:21:07 UTC (rev 149389)
@@ -108,40 +108,20 @@
}
}
-void NetworkResourceLoader::connectionToWebProcessDidClose()
+template<typename U> bool NetworkResourceLoader::sendAbortingOnFailure(const U& message, unsigned messageSendFlags)
{
- ASSERT(isMainThread());
-
- // If this loader already has a resource handle then it is already in progress on a background thread.
- // On that thread it will notice that its connection to its WebProcess has been invalidated and it will "gracefully" abort.
- if (m_handle)
- return;
-
- cleanup();
-}
-
-template<typename U> bool NetworkResourceLoader::sendAbortingOnFailure(const U& message)
-{
- bool result = send(message);
+ bool result = connection()->send(message, destinationID(), messageSendFlags);
if (!result)
- abortInProgressLoad();
+ abort();
return result;
}
-template<typename U> bool NetworkResourceLoader::sendSyncAbortingOnFailure(const U& message, const typename U::Reply& reply)
+void NetworkResourceLoader::abort()
{
- bool result = sendSync(message, reply);
- if (!result)
- abortInProgressLoad();
- return result;
-}
-
-void NetworkResourceLoader::abortInProgressLoad()
-{
- ASSERT(m_handle);
ASSERT(isMainThread());
- m_handle->cancel();
+ if (m_handle)
+ m_handle->cancel();
cleanup();
}
@@ -187,7 +167,7 @@
tryGetShareableHandleFromSharedBuffer(shareableResourceHandle, buffer.get());
if (!shareableResourceHandle.isNull()) {
// Since we're delivering this resource by ourselves all at once, we'll abort the resource handle since we don't need anymore callbacks from ResourceHandle.
- abortInProgressLoad();
+ abort();
send(Messages::WebResourceLoader::DidReceiveResource(shareableResourceHandle, currentTime()));
return;
}
@@ -232,7 +212,7 @@
// This message is DispatchMessageEvenWhenWaitingForSyncReply to avoid a situation where the NetworkProcess is deadlocked waiting for 6 connections
// to complete while the WebProcess is waiting for a 7th to complete.
- connection()->send(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), destinationID(), CoreIPC::DispatchMessageEvenWhenWaitingForSyncReply);
+ sendAbortingOnFailure(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), CoreIPC::DispatchMessageEvenWhenWaitingForSyncReply);
}
void NetworkResourceLoader::continueWillSendRequest(const ResourceRequest& newRequest)
@@ -325,7 +305,7 @@
// This message is DispatchMessageEvenWhenWaitingForSyncReply to avoid a situation where the NetworkProcess is deadlocked
// waiting for 6 connections to complete while the WebProcess is waiting for a 7th to complete.
- connection()->send(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(protectionSpace), destinationID(), CoreIPC::DispatchMessageEvenWhenWaitingForSyncReply);
+ sendAbortingOnFailure(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(protectionSpace), CoreIPC::DispatchMessageEvenWhenWaitingForSyncReply);
}
void NetworkResourceLoader::continueCanAuthenticateAgainstProtectionSpace(bool result)
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2013-04-30 19:21:07 UTC (rev 149389)
@@ -63,8 +63,8 @@
WebCore::ResourceHandle* handle() const { return m_handle.get(); }
virtual void start() OVERRIDE;
- virtual void connectionToWebProcessDidClose() OVERRIDE;
-
+ virtual void abort() OVERRIDE;
+
// ResourceHandleClient methods
virtual void willSendRequestAsync(WebCore::ResourceHandle*, const WebCore::ResourceRequest&, const WebCore::ResourceResponse& redirectResponse) OVERRIDE;
virtual void didSendData(WebCore::ResourceHandle*, unsigned long long bytesSent, unsigned long long totalBytesToBeSent) OVERRIDE;
@@ -116,9 +116,7 @@
void platformDidReceiveResponse(const WebCore::ResourceResponse&);
- template<typename U> bool sendAbortingOnFailure(const U& message);
- template<typename U> bool sendSyncAbortingOnFailure(const U& message, const typename U::Reply& reply);
- void abortInProgressLoad();
+ template<typename U> bool sendAbortingOnFailure(const U& message, unsigned messageSendFlags = 0);
RefPtr<RemoteNetworkingContext> m_networkingContext;
RefPtr<WebCore::ResourceHandle> m_handle;
Modified: trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h 2013-04-30 19:21:07 UTC (rev 149389)
@@ -56,10 +56,10 @@
bool isLoadingMainResource() const { return m_isLoadingMainResource; }
NetworkConnectionToWebProcess* connectionToWebProcess() const { return m_connection.get(); }
- virtual void connectionToWebProcessDidClose() = 0;
virtual void start() = 0;
-
+ virtual void abort() = 0;
+
virtual bool isSynchronous() { return false; }
void setHostRecord(HostRecord* hostRecord) { ASSERT(isMainThread()); m_hostRecord = hostRecord; }
Modified: trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp 2013-04-30 19:21:07 UTC (rev 149389)
@@ -71,10 +71,8 @@
m_delayedReply->send(error, response, CoreIPC::DataReference((uint8_t*)data.data(), data.size()));
}
-void SyncNetworkResourceLoader::connectionToWebProcessDidClose()
+void SyncNetworkResourceLoader::abort()
{
- ASSERT(isMainThread());
-
cleanup();
}
Modified: trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h (149388 => 149389)
--- trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h 2013-04-30 19:21:07 UTC (rev 149389)
@@ -42,8 +42,8 @@
}
virtual void start() OVERRIDE;
- virtual void connectionToWebProcessDidClose() OVERRIDE;
-
+ virtual void abort() OVERRIDE;
+
virtual bool isSynchronous() OVERRIDE { return true; }
private:
Modified: trunk/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp (149388 => 149389)
--- trunk/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp 2013-04-30 19:21:07 UTC (rev 149389)
@@ -169,8 +169,6 @@
if (!loader)
return;
- // FIXME (NetworkProcess): We should only tell the NetworkProcess to remove load identifiers for ResourceLoaders that were never started.
- // If a resource load was actually started within the NetworkProcess then the NetworkProcess handles clearing out the identifier.
WebProcess::shared().networkConnection()->connection()->send(Messages::NetworkConnectionToWebProcess::RemoveLoadIdentifier(identifier), 0);
// It's possible that this WebResourceLoader might be just about to message back to the NetworkProcess (e.g. ContinueWillSendRequest)
Modified: trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp (149388 => 149389)
--- trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp 2013-04-30 19:04:34 UTC (rev 149388)
+++ trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp 2013-04-30 19:21:07 UTC (rev 149389)
@@ -109,6 +109,7 @@
responseCopy.setCertificateChain(certificateInfo.certificateChain());
m_coreLoader->didReceiveResponse(responseCopy);
+ // If m_coreLoader becomes null as a result of the didReceiveResponse callback, we can't use the send function().
if (!m_coreLoader)
return;
