Don't destroy a request while it is being processed

content/browser/renderer_host/resource_dispatcher_host.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 
 #include <set>
 #include <vector>
@@ skipping 1152 matching lines @@
       GlobalRequestID(child_id, request_id));
   if (i == pending_requests_.end()) {
     // We probably want to remove this warning eventually, but I wanted to be
     // able to notice when this happens during initial development since it
     // should be rare and may indicate a bug.
     DLOG(WARNING) << "Canceling a request that wasn't found";
     return;
   }
   net::URLRequest* request = i->second;
   const bool started_before_cancel = request->is_pending();
-  CancelRequestInternal(request, from_renderer);
-  // If the request isn't in flight, then we won't get asyncronous notification,
-  // so we have to signal ourselves to finish this request.
-  if (!started_before_cancel)
+
+  if (CancelRequestInternal(request, from_renderer) &&
+      !started_before_cancel) {
+    // If the request isn't in flight, then we won't get asyncronous
+    // notification, so we have to signal ourselves to finish this
+    // request.
     OnResponseCompleted(request);
+  }
 }
 
-void ResourceDispatcherHost::CancelRequestInternal(net::URLRequest* request,
+bool ResourceDispatcherHost::CancelRequestInternal(net::URLRequest* request,
                                                    bool from_renderer) {
   VLOG(1) << "CancelRequest: " << request->url().spec();
 
   // WebKit will send us a cancel for downloads since it no longer handles them.
   // In this case, ignore the cancel since we handle downloads in the browser.
   ResourceDispatcherHostRequestInfo* info = InfoForRequest(request);
   if (!from_renderer || !info->is_download()) {
     if (info->login_handler()) {
       info->login_handler()->OnRequestCancelled();
       info->set_login_handler(NULL);
     }
     if (info->ssl_client_auth_handler()) {
       info->ssl_client_auth_handler()->OnRequestCancelled();
       info->set_ssl_client_auth_handler(NULL);
     }
     request->Cancel();

[rvargas (doing something else), 2011/03/21 22:38:10]

Did you considered moving the pending check to this code instead? I'm actually
fine either way, just wondering.

[asanka, 2011/03/22 14:52:42]

My concern was that request->is_pending() being false is not a good signal in
all cases where we end up here.

     // Our callers assume |request| is valid after we return.
     DCHECK(IsValidRequest(request));
+    return true;
   }
 
   // Do not remove from the pending requests, as the request will still
   // call AllDataReceived, and may even have more data before it does
   // that.
+  return false;
 }
 
 int ResourceDispatcherHost::IncrementOutstandingRequestsMemoryCost(
     int cost,
     int child_id) {
   // Retrieve the previous value (defaulting to 0 if not found).
   OutstandingRequestsMemoryCostMap::iterator prev_entry =
       outstanding_requests_memory_cost_map_.find(child_id);
   int new_cost = 0;
   if (prev_entry != outstanding_requests_memory_cost_map_.end())
@@ skipping 727 matching lines @@
   return is_prefetch_enabled_;
 }
 
 // static
 void ResourceDispatcherHost::set_is_prefetch_enabled(bool value) {
   is_prefetch_enabled_ = value;
 }
 
 // static
 bool ResourceDispatcherHost::is_prefetch_enabled_ = false;