Fix for 1498134, possible crash when cancelling a url request. The

crash happened because OnReadCompleted would invoke CompleteRead which
might invoke CancelRequest and delete the request. If this happened
when control returned back to OnReadCompleted the request was deleted,
yet ResourceDispatcherHost assumed it wasn't.

BUG=1498134
TEST=none

Review URL: http://codereview.chromium.org/12602

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

2 files changed, 29 insertions, 2 deletions

diff --git a/chrome/browser/resource_dispatcher_host.cc b/chrome/browser/resource_dispatcher_host.cc
index c68c5da..41e841e 100644
--- a/chrome/browser/resource_dispatcher_host.cc
+++ b/chrome/browser/resource_dispatcher_host.cc
@@ -1729,6 +1729,13 @@ void ResourceDispatcherHost::BeginSaveFile(const GURL& url,
 void ResourceDispatcherHost::CancelRequest(int render_process_host_id,
                                            int request_id,
                                            bool from_renderer) {
+  CancelRequest(render_process_host_id, request_id, from_renderer, true);
+}
+
+void ResourceDispatcherHost::CancelRequest(int render_process_host_id,
+                                           int request_id,
+                                           bool from_renderer,
+                                           bool allow_delete) {
   PendingRequestList::iterator i = pending_requests_.find(
       GlobalRequestID(render_process_host_id, request_id));
   if (i == pending_requests_.end()) {
@@ -1747,7 +1754,16 @@ void ResourceDispatcherHost::CancelRequest(int render_process_host_id,
       info->login_handler->OnRequestCancelled();
       info->login_handler = NULL;
     }
-    i->second->Cancel();
+    if (!i->second->is_pending() && allow_delete) {
+      // No io is pending, canceling the request won't notify us of anything,
+      // so we explicitly remove it.
+      // TODO: removing the request in this manner means we're not notifying
+      // anyone. We need make sure the event handlers and others are notified
+      // so that everything is cleaned up properly.
+      RemovePendingRequest(info->render_process_host_id, info->request_id);
+    } else {
+      i->second->Cancel();
+    }
   }
 
   // Do not remove from the pending requests, as the request will still
@@ -2234,7 +2250,10 @@ bool ResourceDispatcherHost::CompleteRead(URLRequest* request,
   ExtraRequestInfo* info = ExtraInfoForRequest(request);
 
   if (!info->event_handler->OnReadCompleted(info->request_id, bytes_read)) {
-    CancelRequest(info->render_process_host_id, info->request_id, false);
+    // Pass in false as the last arg to indicate we don't want |request|
+    // deleted. We do this as callers of us assume |request| is valid after we
+    // return.
+    CancelRequest(info->render_process_host_id, info->request_id, false, false);
     return false;
   }
 
diff --git a/chrome/browser/resource_dispatcher_host.h b/chrome/browser/resource_dispatcher_host.h
index fc21c7e..bffc136 100644
--- a/chrome/browser/resource_dispatcher_host.h
+++ b/chrome/browser/resource_dispatcher_host.h
@@ -396,6 +396,14 @@ class ResourceDispatcherHost : public URLRequest::Delegate {
   // true on success.
   bool CompleteResponseStarted(URLRequest* request);
 
+  // Cancels the given request if it still exists. We ignore cancels from the
+  // renderer in the event of a download. If |allow_delete| is true and no IO
+  // is pending, the request is removed and deleted.
+  void CancelRequest(int render_process_host_id,
+                     int request_id,
+                     bool from_renderer,
+                     bool allow_delete);
+
   // Helper function for regular and download requests.
   void BeginRequestInternal(URLRequest* request, bool mixed_content);