Fix outstanding request stats for multiple transfer navigations.

content/browser/loader/resource_dispatcher_host_impl.cc

 // Copyright (c) 2012 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/loader/resource_dispatcher_host_impl.h"
 
 #include <algorithm>
 #include <set>
@@ skipping 986 matching lines @@
     const linked_ptr<ResourceLoader>& loader) {
   ResourceRequestInfoImpl* info = loader->GetRequestInfo();
   GlobalRoutingID old_routing_id(
       request_data.transferred_request_child_id, info->GetRouteID());
   GlobalRequestID old_request_id(request_data.transferred_request_child_id,
                                  request_data.transferred_request_request_id);
   GlobalRoutingID new_routing_id(child_id, route_id);
   GlobalRequestID new_request_id(child_id, request_id);
 
   // Clear out data that depends on |info| before updating it.
+  // We always need to move the memory stats to the new process.  In contrast,
+  // stats.num_requests is only tracked for some requests (those that require
+  // file descriptors for their shared memory buffer).
   IncrementOutstandingRequestsMemory(-1, *info);
-  OustandingRequestsStats empty_stats = { 0, 0 };
-  OustandingRequestsStats old_stats = GetOutstandingRequestsStats(*info);
-  UpdateOutstandingRequestsStats(*info, empty_stats);
+  bool should_update_count = info->counted_as_in_flight_request();
+  if (should_update_count)
+    IncrementOutstandingRequestsCount(-1, *info);
   pending_loaders_.erase(old_request_id);
 
   // ResourceHandlers should always get state related to the request from the
   // ResourceRequestInfo rather than caching it locally.  This lets us update
   // the info object when a transfer occurs.
   info->UpdateForTransfer(child_id, route_id, request_data.origin_pid,
                           request_id, request_data.parent_render_frame_id,
                           filter_->GetWeakPtr());
 
   // Update maps that used the old IDs, if necessary.  Some transfers in tests
   // do not actually use a different ID, so not all maps need to be updated.
   pending_loaders_[new_request_id] = loader;
-  UpdateOutstandingRequestsStats(*info, old_stats);
   IncrementOutstandingRequestsMemory(1, *info);
+  if (should_update_count)
+    IncrementOutstandingRequestsCount(1, *info);
   if (old_routing_id != new_routing_id) {
     if (blocked_loaders_map_.find(old_routing_id) !=
             blocked_loaders_map_.end()) {
       blocked_loaders_map_[new_routing_id] =
           blocked_loaders_map_[old_routing_id];
       blocked_loaders_map_.erase(old_routing_id);
     }
   }
   if (old_request_id != new_request_id) {
     DelegateMap::iterator it = delegate_map_.find(old_request_id);
@@ skipping 685 matching lines @@
   stats.memory_cost += count * info.memory_cost();
   DCHECK_GE(stats.memory_cost, 0);
   UpdateOutstandingRequestsStats(info, stats);
 
   return stats;
 }
 
 ResourceDispatcherHostImpl::OustandingRequestsStats
 ResourceDispatcherHostImpl::IncrementOutstandingRequestsCount(
     int count,
-    const ResourceRequestInfoImpl& info) {
+    ResourceRequestInfoImpl& info) {

[davidben, 2014/10/15 19:15:12]

Nit: probably should be passed in as pointer.

[Charlie Reis, 2014/10/15 19:48:00]

Yep.

   DCHECK_EQ(1, abs(count));
   num_in_flight_requests_ += count;
 
+  // Keep track of whether this request is counting toward the number of
+  // in-flight requests for this process, in case we need to transfer it to
+  // another process. This should be a toggle.
+  DCHECK_NE(info.counted_as_in_flight_request(), count > 0);
+  info.set_counted_as_in_flight_request(count > 0);
+
   OustandingRequestsStats stats = GetOutstandingRequestsStats(info);
   stats.num_requests += count;
   DCHECK_GE(stats.num_requests, 0);
   UpdateOutstandingRequestsStats(info, stats);
 
   return stats;
 }
 
 bool ResourceDispatcherHostImpl::HasSufficientResourcesForRequest(
-    const net::URLRequest* request_) {
-  const ResourceRequestInfoImpl* info =
-      ResourceRequestInfoImpl::ForRequest(request_);
+    net::URLRequest* request) {
+  ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest(request);
   OustandingRequestsStats stats = IncrementOutstandingRequestsCount(1, *info);
 
   if (stats.num_requests > max_num_in_flight_requests_per_process_)
     return false;
   if (num_in_flight_requests_ > max_num_in_flight_requests_)
     return false;
 
   return true;
 }
 
 void ResourceDispatcherHostImpl::FinishedWithResourcesForRequest(
-    const net::URLRequest* request_) {
-  const ResourceRequestInfoImpl* info =
-      ResourceRequestInfoImpl::ForRequest(request_);
+    net::URLRequest* request) {
+  ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest(request);
   IncrementOutstandingRequestsCount(-1, *info);
 }
 
 void ResourceDispatcherHostImpl::StartNavigationRequest(
     const CommonNavigationParams& params,
     const NavigationRequestInfo& info,
     scoped_refptr<ResourceRequestBody> request_body,
     int64 navigation_request_id,
     int64 frame_node_id) {
   NOTIMPLEMENTED();
@@ skipping 379 matching lines @@
 
   // Add a flag to selectively bypass the data reduction proxy if the resource
   // type is not an image.
   if (request_data.resource_type != RESOURCE_TYPE_IMAGE)
     load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   return load_flags;
 }
 
 }  // namespace content