<rdar://problem/13886443> Assertion failures in NetworkProcess in SandboxExte...
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jul 2013 20:38:41 +0000 (20:38 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jul 2013 20:38:41 +0000 (20:38 +0000)
        <rdar://problem/13826348> ASSERT(!m_useCount) fails in NetworkProcess at SandboxExtension::~SandboxExtension
        https://bugs.webkit.org/show_bug.cgi?id=118855

        Reviewed by Brady Eidson.

        * NetworkProcess/NetworkResourceLoader.cpp:
        (WebKit::NetworkResourceLoader::cleanup):
        (WebKit::NetworkResourceLoader::didFinishLoading):
        (WebKit::NetworkResourceLoader::didFail):
        Moved sandbox extension invalidation to cleanup() meaning that we won't fail to
        do this when aborting a loader that currently loading from network.

        * NetworkProcess/SchedulableLoader.cpp:
        (WebKit::SchedulableLoader::SchedulableLoader):
        (WebKit::SchedulableLoader::consumeSandboxExtensions):
        (WebKit::SchedulableLoader::invalidateSandboxExtensions):
        * NetworkProcess/SchedulableLoader.h:
        Keep track of whether sandbox extensions are consumed, we don't want to revoke
        extensions that were never consumed (as used to be the case with sync loaders,
        and would be with async ones after the above fix). Also, get rid of extensions
        immediately when invalidating, we won't need them again.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@152862 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit2/NetworkProcess/SchedulableLoader.cpp
Source/WebKit2/NetworkProcess/SchedulableLoader.h

index f429699..520f527 100644 (file)
@@ -1,3 +1,28 @@
+2013-07-18  Alexey Proskuryakov  <ap@apple.com>
+
+        <rdar://problem/13886443> Assertion failures in NetworkProcess in SandboxExtension::revoke when aborting SyncNetworkResourceLoader
+        <rdar://problem/13826348> ASSERT(!m_useCount) fails in NetworkProcess at SandboxExtension::~SandboxExtension
+        https://bugs.webkit.org/show_bug.cgi?id=118855
+
+        Reviewed by Brady Eidson.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::cleanup):
+        (WebKit::NetworkResourceLoader::didFinishLoading):
+        (WebKit::NetworkResourceLoader::didFail):
+        Moved sandbox extension invalidation to cleanup() meaning that we won't fail to
+        do this when aborting a loader that currently loading from network.
+    
+        * NetworkProcess/SchedulableLoader.cpp:
+        (WebKit::SchedulableLoader::SchedulableLoader):
+        (WebKit::SchedulableLoader::consumeSandboxExtensions):
+        (WebKit::SchedulableLoader::invalidateSandboxExtensions):
+        * NetworkProcess/SchedulableLoader.h:
+        Keep track of whether sandbox extensions are consumed, we don't want to revoke
+        extensions that were never consumed (as used to be the case with sync loaders,
+        and would be with async ones after the above fix). Also, get rid of extensions
+        immediately when invalidating, we won't need them again.
+
 2013-07-18  Tim Horton  <timothy_horton@apple.com>
 
         Remove PDFViewController and WKView "custom representations"
index be1134c..5ad12d3 100644 (file)
@@ -86,6 +86,8 @@ void NetworkResourceLoader::cleanup()
 {
     ASSERT(isMainThread());
 
+    invalidateSandboxExtensions();
+
     if (FormData* formData = request().httpBody())
         formData->removeGeneratedFilesIfNeeded();
 
@@ -181,7 +183,6 @@ void NetworkResourceLoader::didFinishLoading(ResourceHandle* handle, double fini
 
     // FIXME (NetworkProcess): For the memory cache we'll need to update the finished status of the cached resource here.
     // Such bookkeeping will need to be thread safe, as this callback is happening on a background thread.
-    invalidateSandboxExtensions();
     send(Messages::WebResourceLoader::DidFinishResourceLoad(finishTime));
     
     cleanup();
@@ -193,7 +194,6 @@ void NetworkResourceLoader::didFail(ResourceHandle* handle, const ResourceError&
 
     // FIXME (NetworkProcess): For the memory cache we'll need to update the finished status of the cached resource here.
     // Such bookkeeping will need to be thread safe, as this callback is happening on a background thread.
-    invalidateSandboxExtensions();
     send(Messages::WebResourceLoader::DidFailResourceLoad(error));
     cleanup();
 }
index 47f41f2..ae7b203 100644 (file)
@@ -49,6 +49,7 @@ SchedulableLoader::SchedulableLoader(const NetworkResourceLoadParameters& parame
     , m_inPrivateBrowsingMode(parameters.inPrivateBrowsingMode)
     , m_shouldClearReferrerOnHTTPSToHTTPRedirect(parameters.shouldClearReferrerOnHTTPSToHTTPRedirect)
     , m_isLoadingMainResource(parameters.isMainResource)
+    , m_sandboxExtensionsAreConsumed(false)
     , m_connection(connection)
 {
     // Either this loader has both a webPageID and webFrameID, or it is not allowed to ask the client for authentication credentials.
@@ -93,15 +94,23 @@ void SchedulableLoader::consumeSandboxExtensions()
 
     for (size_t i = 0, count = m_resourceSandboxExtensions.size(); i < count; ++i)
         m_resourceSandboxExtensions[i]->consume();
+
+    m_sandboxExtensionsAreConsumed = true;
 }
 
 void SchedulableLoader::invalidateSandboxExtensions()
 {
-    for (size_t i = 0, count = m_requestBodySandboxExtensions.size(); i < count; ++i)
-        m_requestBodySandboxExtensions[i]->revoke();
+    if (m_sandboxExtensionsAreConsumed) {
+        for (size_t i = 0, count = m_requestBodySandboxExtensions.size(); i < count; ++i)
+            m_requestBodySandboxExtensions[i]->revoke();
+        for (size_t i = 0, count = m_resourceSandboxExtensions.size(); i < count; ++i)
+            m_resourceSandboxExtensions[i]->revoke();
+    }
 
-    for (size_t i = 0, count = m_resourceSandboxExtensions.size(); i < count; ++i)
-        m_resourceSandboxExtensions[i]->revoke();
+    m_requestBodySandboxExtensions.clear();
+    m_resourceSandboxExtensions.clear();
+
+    m_sandboxExtensionsAreConsumed = false;
 }
 
 } // namespace WebKit
index c4206d1..81ff5e5 100644 (file)
@@ -88,6 +88,7 @@ private:
 
     Vector<RefPtr<SandboxExtension>> m_requestBodySandboxExtensions;
     Vector<RefPtr<SandboxExtension>> m_resourceSandboxExtensions;
+    bool m_sandboxExtensionsAreConsumed;
 
     RefPtr<NetworkConnectionToWebProcess> m_connection;