Diff
Modified: trunk/Source/WebKit2/ChangeLog (145484 => 145485)
--- trunk/Source/WebKit2/ChangeLog 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/ChangeLog 2013-03-12 06:12:25 UTC (rev 145485)
@@ -1,3 +1,45 @@
+2013-03-11 Brady Eidson <beid...@apple.com>
+
+ Loads are never canceled in the NetworkProcess
+ <rdar://problem/12890500> and https://bugs.webkit.org/show_bug.cgi?id=112103
+
+ Reviewed by Alexey Proskuryakov.
+
+ If a connection to a WebProcess is closed (gracefully or by crashing) then:
+ - All scheduled loads for that connection should be forgotten.
+ - All in-progress loads for that connection should be aborted asap.
+
+ * NetworkProcess/NetworkResourceLoader.cpp:
+ (WebKit::requestsToCleanupMutex): The "request to stop" mechanism is now called "request to cleanup"
+ (WebKit::requestsToCleanup):
+ (WebKit::NetworkResourceLoader::scheduleCleanupOnMainThread):
+ (WebKit::NetworkResourceLoader::performCleanups):
+ (WebKit::NetworkResourceLoader::cleanup):
+ (WebKit::NetworkResourceLoader::didFinishLoading):
+ (WebKit::NetworkResourceLoader::didFail):
+
+ (WebKit::NetworkResourceLoader::connectionToWebProcessDidClose): If there is a resource handle, let
+ the loading thread notice the connection is invalid. Otherwise request to cleanup the loader.
+ (WebKit::NetworkResourceLoader::sendAbortingOnFailure):
+ (WebKit::NetworkResourceLoader::sendSyncAbortingOnFailure):
+ (WebKit::NetworkResourceLoader::abortInProgressLoad): Cancel the resource handle and schedule main thread cleanup.
+ (WebKit::NetworkResourceLoader::didReceiveResponse): Use sendAbortingOnFailure instead of send.
+ (WebKit::NetworkResourceLoader::didReceiveData): Ditto.
+
+ (WebKit::NetworkResourceLoader::willSendRequest): Call abortInProgressLoad if the sync message failed.
+ (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpace): Ditto.
+ * NetworkProcess/NetworkResourceLoader.h:
+
+ Make connectionToWebProcessDidClose() pure virtual, moving its implementation to the subclasses:
+ * NetworkProcess/SchedulableLoader.cpp:
+ * NetworkProcess/SchedulableLoader.h:
+
+ * NetworkProcess/SyncNetworkResourceLoader.cpp:
+ (WebKit::SyncNetworkResourceLoader::start): Call cleanup().
+ (WebKit::SyncNetworkResourceLoader::connectionToWebProcessDidClose): Call cleanup().
+ (WebKit::SyncNetworkResourceLoader::cleanup): Factor out the sync loader cleanup code.
+ * NetworkProcess/SyncNetworkResourceLoader.h:
+
2013-03-11 Tim Horton <timothy_hor...@apple.com>
PDFPlugin: Return PDFKit's data instead of the original resource data for save/etc.
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2013-03-12 06:12:25 UTC (rev 145485)
@@ -86,70 +86,117 @@
m_handle = ResourceHandle::create(m_networkingContext.get(), request(), this, false /* defersLoading */, contentSniffingPolicy() == SniffContent);
}
-static bool stopRequestsCalled = false;
+static bool performCleanupsCalled = false;
-static Mutex& requestsToStopMutex()
+static Mutex& requestsToCleanupMutex()
{
DEFINE_STATIC_LOCAL(Mutex, mutex, ());
return mutex;
}
-static HashSet<NetworkResourceLoader*>& requestsToStop()
+static HashSet<NetworkResourceLoader*>& requestsToCleanup()
{
DEFINE_STATIC_LOCAL(HashSet<NetworkResourceLoader*>, requests, ());
return requests;
}
-void NetworkResourceLoader::scheduleStopOnMainThread()
+void NetworkResourceLoader::scheduleCleanupOnMainThread()
{
- MutexLocker locker(requestsToStopMutex());
+ MutexLocker locker(requestsToCleanupMutex());
- requestsToStop().add(this);
- if (!stopRequestsCalled) {
- stopRequestsCalled = true;
- callOnMainThread(NetworkResourceLoader::performStops, 0);
+ requestsToCleanup().add(this);
+ if (!performCleanupsCalled) {
+ performCleanupsCalled = true;
+ callOnMainThread(NetworkResourceLoader::performCleanups, 0);
}
}
-void NetworkResourceLoader::performStops(void*)
+void NetworkResourceLoader::performCleanups(void*)
{
- ASSERT(stopRequestsCalled);
+ ASSERT(performCleanupsCalled);
Vector<NetworkResourceLoader*> requests;
{
- MutexLocker locker(requestsToStopMutex());
- copyToVector(requestsToStop(), requests);
- requestsToStop().clear();
- stopRequestsCalled = false;
+ MutexLocker locker(requestsToCleanupMutex());
+ copyToVector(requestsToCleanup(), requests);
+ requestsToCleanup().clear();
+ performCleanupsCalled = false;
}
for (size_t i = 0; i < requests.size(); ++i)
- requests[i]->resourceHandleStopped();
+ requests[i]->cleanup();
}
-void NetworkResourceLoader::resourceHandleStopped()
+void NetworkResourceLoader::cleanup()
{
ASSERT(isMainThread());
if (FormData* formData = request().httpBody())
formData->removeGeneratedFilesIfNeeded();
- m_handle = 0;
-
// Tell the scheduler about this finished loader soon so it can start more network requests.
NetworkProcess::shared().networkResourceLoadScheduler().scheduleRemoveLoader(this);
- // Explicit deref() balanced by a ref() in NetworkResourceLoader::start()
- // This might cause the NetworkResourceLoader to be destroyed and therefore we do it last.
- deref();
+ 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();
+ }
}
+void NetworkResourceLoader::connectionToWebProcessDidClose()
+{
+ 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;
+
+#if !ASSERT_DISABLED
+ // Since there's no handle, this loader should never have been started, and therefore it should never be in the
+ // set of loaders to cleanup on the main thread.
+ // Let's make sure that's true.
+ {
+ MutexLocker locker(requestsToCleanupMutex());
+ ASSERT(!requestsToCleanup().contains(this));
+ }
+#endif
+
+ cleanup();
+}
+
+template<typename U> void NetworkResourceLoader::sendAbortingOnFailure(const U& message)
+{
+ if (!send(message))
+ abortInProgressLoad();
+}
+
+template<typename U> bool NetworkResourceLoader::sendSyncAbortingOnFailure(const U& message, const typename U::Reply& reply)
+{
+ bool result = sendSync(message, reply);
+ if (!result)
+ abortInProgressLoad();
+ return result;
+}
+
+void NetworkResourceLoader::abortInProgressLoad()
+{
+ ASSERT(m_handle);
+ ASSERT(!isMainThread());
+
+ m_handle->cancel();
+
+ scheduleCleanupOnMainThread();
+}
+
void NetworkResourceLoader::didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
{
// FIXME (NetworkProcess): Cache the response.
if (FormData* formData = request().httpBody())
formData->removeGeneratedFilesIfNeeded();
- send(Messages::WebResourceLoader::DidReceiveResponseWithCertificateInfo(response, PlatformCertificateInfo(response)));
+ sendAbortingOnFailure(Messages::WebResourceLoader::DidReceiveResponseWithCertificateInfo(response, PlatformCertificateInfo(response)));
}
void NetworkResourceLoader::didReceiveData(ResourceHandle*, const char* data, int length, int encodedDataLength)
@@ -158,7 +205,7 @@
// Such buffering will need to be thread safe, as this callback is happening on a background thread.
CoreIPC::DataReference dataReference(reinterpret_cast<const uint8_t*>(data), length);
- send(Messages::WebResourceLoader::DidReceiveData(dataReference, encodedDataLength, false));
+ sendAbortingOnFailure(Messages::WebResourceLoader::DidReceiveData(dataReference, encodedDataLength, false));
}
void NetworkResourceLoader::didFinishLoading(ResourceHandle*, double finishTime)
@@ -167,7 +214,7 @@
// Such bookkeeping will need to be thread safe, as this callback is happening on a background thread.
invalidateSandboxExtensions();
send(Messages::WebResourceLoader::DidFinishResourceLoad(finishTime));
- scheduleStopOnMainThread();
+ scheduleCleanupOnMainThread();
}
void NetworkResourceLoader::didFail(ResourceHandle*, const ResourceError& error)
@@ -176,7 +223,7 @@
// Such bookkeeping will need to be thread safe, as this callback is happening on a background thread.
invalidateSandboxExtensions();
send(Messages::WebResourceLoader::DidFailResourceLoad(error));
- scheduleStopOnMainThread();
+ scheduleCleanupOnMainThread();
}
void NetworkResourceLoader::willSendRequest(ResourceHandle*, ResourceRequest& request, const ResourceResponse& redirectResponse)
@@ -190,10 +237,10 @@
// to complete while the WebProcess is waiting for a 7th to complete.
// If we ever change this message to be asynchronous we have to include safeguards to make sure the new design interacts well with sync XHR.
ResourceRequest returnedRequest;
- if (sendSync(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), Messages::WebResourceLoader::WillSendRequest::Reply(returnedRequest)))
- request.updateFromDelegatePreservingOldHTTPBody(returnedRequest.nsURLRequest(DoNotUpdateHTTPBody));
- else
- request = ResourceRequest();
+ if (!sendSyncAbortingOnFailure(Messages::WebResourceLoader::WillSendRequest(request, redirectResponse), Messages::WebResourceLoader::WillSendRequest::Reply(returnedRequest)))
+ return;
+
+ request.updateFromDelegatePreservingOldHTTPBody(returnedRequest.nsURLRequest(DoNotUpdateHTTPBody));
RunLoop::main()->dispatch(bind(&NetworkResourceLoadScheduler::receivedRedirect, &NetworkProcess::shared().networkResourceLoadScheduler(), this, request.url()));
}
@@ -249,7 +296,7 @@
// to complete while the WebProcess is waiting for a 7th to complete.
// If we ever change this message to be asynchronous we have to include safeguards to make sure the new design interacts well with sync XHR.
bool result;
- if (!sendSync(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(protectionSpace), Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace::Reply(result)))
+ if (!sendSyncAbortingOnFailure(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(protectionSpace), Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace::Reply(result)))
return false;
return result;
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.h 2013-03-12 06:12:25 UTC (rev 145485)
@@ -57,7 +57,8 @@
CoreIPC::Connection* connection() const;
uint64_t destinationID() const;
- virtual void start();
+ virtual void start() OVERRIDE;
+ virtual void connectionToWebProcessDidClose() OVERRIDE;
// ResourceHandleClient methods
virtual void willSendRequest(WebCore::ResourceHandle*, WebCore::ResourceRequest&, const WebCore::ResourceResponse& /*redirectResponse*/) OVERRIDE;
@@ -94,10 +95,13 @@
private:
NetworkResourceLoader(const NetworkResourceLoadParameters&, NetworkConnectionToWebProcess*);
- void scheduleStopOnMainThread();
- static void performStops(void*);
+ void scheduleCleanupOnMainThread();
+ static void performCleanups(void*);
+ void cleanup();
- void resourceHandleStopped();
+ template<typename U> void sendAbortingOnFailure(const U& message);
+ template<typename U> bool sendSyncAbortingOnFailure(const U& message, const typename U::Reply& reply);
+ void abortInProgressLoad();
RefPtr<RemoteNetworkingContext> m_networkingContext;
RefPtr<WebCore::ResourceHandle> m_handle;
Modified: trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.cpp (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.cpp 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.cpp 2013-03-12 06:12:25 UTC (rev 145485)
@@ -79,11 +79,6 @@
ASSERT(!m_hostRecord);
}
-void SchedulableLoader::connectionToWebProcessDidClose()
-{
- // FIXME (NetworkProcess): <rdar://problem/12890500>: Cancel the load. The request may be long-living, so we don't want it to linger around after all clients are gone.
-}
-
void SchedulableLoader::consumeSandboxExtensions()
{
for (size_t i = 0, count = m_requestBodySandboxExtensions.size(); i < count; ++i)
Modified: trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/SchedulableLoader.h 2013-03-12 06:12:25 UTC (rev 145485)
@@ -54,7 +54,7 @@
bool inPrivateBrowsingMode() const { return m_inPrivateBrowsingMode; }
NetworkConnectionToWebProcess* connectionToWebProcess() const { return m_connection.get(); }
- void connectionToWebProcessDidClose();
+ virtual void connectionToWebProcessDidClose() = 0;
virtual void start() = 0;
Modified: trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.cpp 2013-03-12 06:12:25 UTC (rev 145485)
@@ -66,13 +66,24 @@
ResourceHandle::loadResourceSynchronously(networkingContext.get(), request(), allowStoredCredentials(), error, response, data);
- invalidateSandboxExtensions();
+ cleanup();
m_delayedReply->send(error, response, CoreIPC::DataReference((uint8_t*)data.data(), data.size()));
+}
+
+void SyncNetworkResourceLoader::connectionToWebProcessDidClose()
+{
+ ASSERT(isMainThread());
- NetworkProcess::shared().networkResourceLoadScheduler().removeLoader(this);
+ cleanup();
}
+void SyncNetworkResourceLoader::cleanup()
+{
+ invalidateSandboxExtensions();
+ NetworkProcess::shared().networkResourceLoadScheduler().scheduleRemoveLoader(this);
+}
+
} // namespace WebKit
#endif // ENABLE(NETWORK_PROCESS)
Modified: trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h (145484 => 145485)
--- trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h 2013-03-12 06:10:21 UTC (rev 145484)
+++ trunk/Source/WebKit2/NetworkProcess/SyncNetworkResourceLoader.h 2013-03-12 06:12:25 UTC (rev 145485)
@@ -41,13 +41,16 @@
return adoptRef(new SyncNetworkResourceLoader(parameters, connection, reply));
}
- virtual void start();
+ virtual void start() OVERRIDE;
+ virtual void connectionToWebProcessDidClose() OVERRIDE;
- virtual bool isSynchronous() { return true; }
+ virtual bool isSynchronous() OVERRIDE { return true; }
private:
SyncNetworkResourceLoader(const NetworkResourceLoadParameters&, NetworkConnectionToWebProcess*, PassRefPtr<Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply>);
+ void cleanup();
+
RefPtr<Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply> m_delayedReply;
};
