Fixed CHECKs for use of canceled ResourceContexts in ResourceDispatcherHostImpl.

Checks weren't taking into account the possibility of a later allocation re-using a
deleted pointer.

TBR=willchan@chromium.org
BUG=90971
BUG=100566
TEST="browser_tests --gtest_filter=BrowserCloseTest.DISABLED_DownloadsCloseCheck_1 --gtest_repeat=10 --gtest_also_run_disabled_tests" with no crash.


Review URL: https://chromiumcodereview.appspot.com/10389030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@137163 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 32 insertions, 7 deletions

diff --git a/content/browser/renderer_host/resource_dispatcher_host_impl.cc b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
index ce87cd6..67362fd 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_impl.cc
+++ b/content/browser/renderer_host/resource_dispatcher_host_impl.cc
@@ -370,12 +370,22 @@ void ResourceDispatcherHostImpl::SetAllowCrossOriginAuthPrompt(bool value) {
   allow_cross_origin_auth_prompt_ = value;
 }
 
+void ResourceDispatcherHostImpl::AddResourceContext(ResourceContext* context) {
+  active_resource_contexts_.insert(context);
+}
+
+void ResourceDispatcherHostImpl::RemoveResourceContext(
+    ResourceContext* context) {
+  CHECK(ContainsKey(active_resource_contexts_, context));
+  active_resource_contexts_.erase(context);
+}
+
 void ResourceDispatcherHostImpl::CancelRequestsForContext(
     ResourceContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(context);
 
-  canceled_resource_contexts_.insert(context);
+  CHECK(ContainsKey(active_resource_contexts_, context));
 
   // Note that request cancellation has side effects. Therefore, we gather all
   // the requests to cancel first, and then we start cancelling. We assert at
@@ -481,7 +491,7 @@ net::Error ResourceDispatcherHostImpl::BeginDownload(
   char url_buf[128];
   base::strlcpy(url_buf, url.spec().c_str(), arraysize(url_buf));
   base::debug::Alias(url_buf);
-  CHECK(!ContainsKey(canceled_resource_contexts_, context));
+  CHECK(ContainsKey(active_resource_contexts_, context));
 
   const net::URLRequestContext* request_context = context->GetRequestContext();
   request->set_referrer(MaybeStripReferrer(GURL(request->referrer())).spec());
@@ -756,7 +766,7 @@ void ResourceDispatcherHostImpl::BeginRequest(
 
   ResourceContext* resource_context = filter_->resource_context();
   // http://crbug.com/90971
-  CHECK(!ContainsKey(canceled_resource_contexts_, resource_context));
+  CHECK(ContainsKey(active_resource_contexts_, resource_context));
 
   // Might need to resolve the blob references in the upload data.
   if (request_data.upload_data) {
@@ -1152,7 +1162,7 @@ void ResourceDispatcherHostImpl::BeginSaveFile(
   char url_buf[128];
   base::strlcpy(url_buf, url.spec().c_str(), arraysize(url_buf));
   base::debug::Alias(url_buf);
-  CHECK(!ContainsKey(canceled_resource_contexts_, context));
+  CHECK(ContainsKey(active_resource_contexts_, context));
 
   scoped_refptr<ResourceHandler> handler(
       new SaveFileResourceHandler(child_id,
diff --git a/content/browser/renderer_host/resource_dispatcher_host_impl.h b/content/browser/renderer_host/resource_dispatcher_host_impl.h
index 463fffa..1cb899a 100644
--- a/content/browser/renderer_host/resource_dispatcher_host_impl.h
+++ b/content/browser/renderer_host/resource_dispatcher_host_impl.h
@@ -90,6 +90,12 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // new requests).  Cancels all pending requests.
   void Shutdown();
 
+  // Notify the ResourceDispatcherHostImpl of a new resource context.
+  void AddResourceContext(ResourceContext* context);
+
+  // Notify the ResourceDispatcherHostImpl of a resource context destruction.
+  void RemoveResourceContext(ResourceContext* context);
+
   // Force cancels any pending requests for the given |context|. This is
   // necessary to ensure that before |context| goes away, all requests
   // for it are dead.
@@ -508,7 +514,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
 
   // http://crbug.com/90971 - Assists in tracking down use-after-frees on
   // shutdown.
-  std::set<const ResourceContext*> canceled_resource_contexts_;
+  std::set<const ResourceContext*> active_resource_contexts_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceDispatcherHostImpl);
 };
diff --git a/content/browser/resource_context_impl.cc b/content/browser/resource_context_impl.cc
index dc90129..f47824d 100644
--- a/content/browser/resource_context_impl.cc
+++ b/content/browser/resource_context_impl.cc
@@ -179,9 +179,17 @@ AppCacheService* ResourceContext::GetAppCacheService(ResourceContext* context) {
       context, kAppCacheServicKeyName);
 }
 
-ResourceContext::~ResourceContext() {
+ResourceContext::ResourceContext() {
   if (ResourceDispatcherHostImpl::Get())
-    ResourceDispatcherHostImpl::Get()->CancelRequestsForContext(this);
+    ResourceDispatcherHostImpl::Get()->AddResourceContext(this);
+}
+
+ResourceContext::~ResourceContext() {
+  ResourceDispatcherHostImpl* rdhi = ResourceDispatcherHostImpl::Get();
+  if (rdhi) {
+    rdhi->CancelRequestsForContext(this);
+    rdhi->RemoveResourceContext(this);
+  }
 }
 
 BlobStorageController* GetBlobStorageControllerForResourceContext(
diff --git a/content/public/browser/resource_context.h b/content/public/browser/resource_context.h
index 5b4bf5be..0bdcff8 100644
--- a/content/public/browser/resource_context.h
+++ b/content/public/browser/resource_context.h
@@ -27,6 +27,7 @@ class CONTENT_EXPORT ResourceContext : public base::SupportsUserData {
   static appcache::AppCacheService* GetAppCacheService(
       ResourceContext* resource_context);
 
+  ResourceContext();
   virtual ~ResourceContext();
   virtual net::HostResolver* GetHostResolver() = 0;
   virtual net::URLRequestContext* GetRequestContext() = 0;