Cache total_delayable_count for in_flight_requests in ResourceScheduler

content/browser/loader/resource_scheduler.cc

Index: content/browser/loader/resource_scheduler.cc
diff --git a/content/browser/loader/resource_scheduler.cc b/content/browser/loader/resource_scheduler.cc
index 4b4c832b7d5ef7d416804b6ad9cef8a65ad726b1..e2a0fe21b88bf845609e9b057621b0164cb3b27d 100644
--- a/content/browser/loader/resource_scheduler.cc
+++ b/content/browser/loader/resource_scheduler.cc
@@ -126,7 +126,8 @@ class ResourceScheduler::ScheduledResourceRequest
         deferred_(false),
         scheduler_(scheduler),
         priority_(priority),
-        fifo_ordering_(0) {
+        fifo_ordering_(0),
+        accounted_as_delayable_request_(false) {
     TRACE_EVENT_ASYNC_BEGIN1("net", "URLRequest", request_,
                              "url", request->url().spec());
   }
@@ -157,6 +158,12 @@ class ResourceScheduler::ScheduledResourceRequest
   void set_fifo_ordering(uint32 fifo_ordering) {
     fifo_ordering_ = fifo_ordering;
   }
+  bool accounted_as_delayable_request() const {
+    return accounted_as_delayable_request_;
+  }
+  void set_accounted_as_delayable_request(bool accounted) {
+    accounted_as_delayable_request_ = accounted;
+  }
 
  private:
   // ResourceMessageDelegate interface:
@@ -191,6 +198,8 @@ class ResourceScheduler::ScheduledResourceRequest
   ResourceScheduler* scheduler_;
   RequestPriorityParams priority_;
   uint32 fifo_ordering_;
+  // True if the request is delayable in |in_flight_requests_|.
+  bool accounted_as_delayable_request_;
 
   DISALLOW_COPY_AND_ASSIGN(ScheduledResourceRequest);
 };
@@ -219,14 +228,270 @@ void ResourceScheduler::RequestQueue::Insert(
 }
 
 // Each client represents a tab.
-struct ResourceScheduler::Client {
-  Client() : has_body(false), using_spdy_proxy(false) {}
+class ResourceScheduler::Client {
+ public:
+  Client()
+      : has_body_(false),
+        using_spdy_proxy_(false),
+        total_delayable_count_(0) {}
   ~Client() {}
 
-  bool has_body;
-  bool using_spdy_proxy;
-  RequestQueue pending_requests;
-  RequestSet in_flight_requests;
+  void ScheduleRequest(
+      net::URLRequest* url_request,
+      ScheduledResourceRequest* request) {
+    if (ShouldStartRequest(request) == START_REQUEST) {
+      StartRequest(request);
+    } else {
+      pending_requests_.Insert(request);
+    }
+  }
+
+  void RemoveRequest(ScheduledResourceRequest* request) {
+    if (pending_requests_.IsQueued(request)) {
+      pending_requests_.Erase(request);
+      DCHECK(!ContainsKey(in_flight_requests_, request));
+    } else {
+      EraseInFlightRequest(request);
+
+      // Removing this request may have freed up another to load.
+      LoadAnyStartablePendingRequests();
+    }
+  }
+
+  RequestSet RemoveAllRequests() {
+    RequestSet unowned_requests;
+    for (RequestSet::iterator it = in_flight_requests_.begin();
+         it != in_flight_requests_.end(); ++it) {
+      unowned_requests.insert(*it);
+      (*it)->set_accounted_as_delayable_request(false);
+    }
+    ClearInFlightRequests();
+    return unowned_requests;
+  }
+
+  void OnNavigate() {
+    has_body_ = false;
+  }
+
+  void OnWillInsertBody() {
+    has_body_ = true;
+    LoadAnyStartablePendingRequests();
+  }
+
+  void OnReceivedSpdyProxiedHttpResponse() {
+    if (!using_spdy_proxy_) {
+      using_spdy_proxy_ = true;
+      LoadAnyStartablePendingRequests();
+    }
+  }
+
+  void ReprioritizeRequest(ScheduledResourceRequest* request,
+                           RequestPriorityParams old_priority_params,
+                           RequestPriorityParams new_priority_params) {
+    request->url_request()->SetPriority(new_priority_params.priority);
+    request->set_request_priority_params(new_priority_params);
+    if (!pending_requests_.IsQueued(request)) {
+      DCHECK(ContainsKey(in_flight_requests_, request));
+      // The priority and SPDY support may have changed, so update the
+      // delayable count.
+      SetRequestDelayable(request, IsDelayableRequest(request));
+      // Request has already started.
+      return;
+    }
+
+    pending_requests_.Erase(request);
+    pending_requests_.Insert(request);
+
+    if (new_priority_params.priority > old_priority_params.priority) {
+      // Check if this request is now able to load at its new priority.
+      LoadAnyStartablePendingRequests();
+    }
+  }
+
+ private:
+  enum ShouldStartReqResult {
+      DO_NOT_START_REQUEST_AND_STOP_SEARCHING = -2,
+      DO_NOT_START_REQUEST_AND_KEEP_SEARCHING = -1,
+      START_REQUEST = 1,
+  };
+
+  void InsertInFlightRequest(ScheduledResourceRequest* request) {
+    in_flight_requests_.insert(request);
+    if (IsDelayableRequest(request))
+      SetRequestDelayable(request, true);
+  }
+
+  void EraseInFlightRequest(ScheduledResourceRequest* request) {
+    size_t erased = in_flight_requests_.erase(request);
+    DCHECK_EQ(1u, erased);
+    SetRequestDelayable(request, false);
+    DCHECK_LE(total_delayable_count_, in_flight_requests_.size());
+  }
+
+  void ClearInFlightRequests() {
+    in_flight_requests_.clear();
+    total_delayable_count_ = 0;
+  }
+
+  bool IsDelayableRequest(ScheduledResourceRequest* request) {
+    if (request->url_request()->priority() < net::LOW) {
+      net::HostPortPair host_port_pair =
+          net::HostPortPair::FromURL(request->url_request()->url());
+      const net::HttpServerProperties& http_server_properties =
+          *request->url_request()->context()->http_server_properties();
+      if (!http_server_properties.SupportsSpdy(host_port_pair)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  void SetRequestDelayable(ScheduledResourceRequest* request,
+                           bool delayable) {
+    if (request->accounted_as_delayable_request() == delayable)
+      return;
+    if (delayable)
+      total_delayable_count_++;
+    else
+      total_delayable_count_--;
+    request->set_accounted_as_delayable_request(delayable);
+  }
+
+  bool ShouldKeepSearching(
+      const net::HostPortPair& active_request_host) const {
+    size_t same_host_count = 0;
+    for (RequestSet::const_iterator it = in_flight_requests_.begin();
+         it != in_flight_requests_.end(); ++it) {
+      net::HostPortPair host_port_pair =
+          net::HostPortPair::FromURL((*it)->url_request()->url());
+      if (active_request_host.Equals(host_port_pair)) {
+        same_host_count++;
+        if (same_host_count >= kMaxNumDelayableRequestsPerHost)
+          return true;
+      }
+    }
+    return false;
+  }
+
+  void StartRequest(ScheduledResourceRequest* request) {
+    InsertInFlightRequest(request);
+    request->Start();
+  }
+
+  // ShouldStartRequest is the main scheduling algorithm.
+  //
+  // Requests are categorized into two categories:
+  //
+  // 1. Immediately issued requests, which are:
+  //
+  //   * Higher priority requests (>= net::LOW).
+  //   * Synchronous requests.
+  //   * Requests to SPDY-capable origin servers.
+  //   * Non-HTTP[S] requests.
+  //
+  // 2. The remainder are delayable requests, which follow these rules:
+  //
+  //   * If no high priority requests are in flight, start loading low priority
+  //      requests.
+  //   * Once the renderer has a <body>, start loading delayable requests.
+  //   * Never exceed 10 delayable requests in flight per client.
+  //   * Never exceed 6 delayable requests for a given host.
+  //   * Prior to <body>, allow one delayable request to load at a time.
+  ShouldStartReqResult ShouldStartRequest(
+      ScheduledResourceRequest* request) const {
+    const net::URLRequest& url_request = *request->url_request();
+    // TODO(simonjam): This may end up causing disk contention. We should
+    // experiment with throttling if that happens.
+    if (!url_request.url().SchemeIsHTTPOrHTTPS()) {
+      return START_REQUEST;
+    }
+
+    if (using_spdy_proxy_ && url_request.url().SchemeIs("http")) {
+      return START_REQUEST;
+    }
+
+    const net::HttpServerProperties& http_server_properties =
+        *url_request.context()->http_server_properties();
+
+    if (url_request.priority() >= net::LOW ||
+        !ResourceRequestInfo::ForRequest(&url_request)->IsAsync()) {
+      return START_REQUEST;
+    }
+
+    net::HostPortPair host_port_pair =
+        net::HostPortPair::FromURL(url_request.url());
+
+    // TODO(willchan): We should really improve this algorithm as described in
+    // crbug.com/164101. Also, theoretically we should not count a SPDY request
+    // against the delayable requests limit.
+    if (http_server_properties.SupportsSpdy(host_port_pair)) {
+      return START_REQUEST;
+    }
+
+    size_t num_delayable_requests_in_flight = total_delayable_count_;
+    if (num_delayable_requests_in_flight >= kMaxNumDelayableRequestsPerClient) {
+      return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+    }
+
+    if (ShouldKeepSearching(host_port_pair)) {
+      // There may be other requests for other hosts we'd allow,
+      // so keep checking.
+      return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
+    }
+
+    bool have_immediate_requests_in_flight =
+        in_flight_requests_.size() > num_delayable_requests_in_flight;
+    if (have_immediate_requests_in_flight && !has_body_ &&
+        num_delayable_requests_in_flight != 0) {
+      return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
+    }
+
+    return START_REQUEST;
+  }
+
+  void LoadAnyStartablePendingRequests() {
+    // We iterate through all the pending requests, starting with the highest
+    // priority one. For each entry, one of three things can happen:
+    // 1) We start the request, remove it from the list, and keep checking.
+    // 2) We do NOT start the request, but ShouldStartRequest() signals us that
+    //     there may be room for other requests, so we keep checking and leave
+    //     the previous request still in the list.
+    // 3) We do not start the request, same as above, but StartRequest() tells
+    //     us there's no point in checking any further requests.
+    RequestQueue::NetQueue::iterator request_iter =
+        pending_requests_.GetNextHighestIterator();
+
+    while (request_iter != pending_requests_.End()) {
+      ScheduledResourceRequest* request = *request_iter;
+      ShouldStartReqResult query_result = ShouldStartRequest(request);
+
+      if (query_result == START_REQUEST) {
+        pending_requests_.Erase(request);
+        StartRequest(request);
+
+        // StartRequest can modify the pending list, so we (re)start evaluation
+        // from the currently highest priority request. Avoid copying a singular
+        // iterator, which would trigger undefined behavior.
+        if (pending_requests_.GetNextHighestIterator() ==
+            pending_requests_.End())
+          break;
+        request_iter = pending_requests_.GetNextHighestIterator();
+      } else if (query_result == DO_NOT_START_REQUEST_AND_KEEP_SEARCHING) {
+        ++request_iter;
+        continue;
+      } else {
+        DCHECK(query_result == DO_NOT_START_REQUEST_AND_STOP_SEARCHING);
+        break;
+      }
+    }
+  }
+
+  bool has_body_;
+  bool using_spdy_proxy_;
+  RequestQueue pending_requests_;
+  RequestSet in_flight_requests_;
+  // The number of delayable in-flight requests.
+  size_t total_delayable_count_;
 };
 
 ResourceScheduler::ResourceScheduler() {
@@ -259,11 +524,7 @@ scoped_ptr<ResourceThrottle> ResourceScheduler::ScheduleRequest(
   }
 
   Client* client = it->second;
-  if (ShouldStartRequest(request.get(), client) == START_REQUEST) {
-    StartRequest(request.get(), client);
-  } else {
-    client->pending_requests.Insert(request.get());
-  }
+  client->ScheduleRequest(url_request, request.get());
   return request.PassAs<ResourceThrottle>();
 }
 
@@ -280,17 +541,7 @@ void ResourceScheduler::RemoveRequest(ScheduledResourceRequest* request) {
   }
 
   Client* client = client_it->second;
-
-  if (client->pending_requests.IsQueued(request)) {
-    client->pending_requests.Erase(request);
-    DCHECK(!ContainsKey(client->in_flight_requests, request));
-  } else {
-    size_t erased = client->in_flight_requests.erase(request);
-    DCHECK(erased);
-
-    // Removing this request may have freed up another to load.
-    LoadAnyStartablePendingRequests(client);
-  }
+  client->RemoveRequest(request);
 }
 
 void ResourceScheduler::OnClientCreated(int child_id, int route_id) {
@@ -314,11 +565,11 @@ void ResourceScheduler::OnClientDeleted(int child_id, int route_id) {
   // FYI, ResourceDispatcherHost cancels all of the requests after this function
   // is called. It should end up canceling all of the requests except for a
   // cross-renderer navigation.
-  for (RequestSet::iterator it = client->in_flight_requests.begin();
-       it != client->in_flight_requests.end(); ++it) {
+  RequestSet client_unowned_requests = client->RemoveAllRequests();
+  for (RequestSet::iterator it = client_unowned_requests.begin();
+       it != client_unowned_requests.end(); ++it) {
     unowned_requests_.insert(*it);
   }
-  client->in_flight_requests.clear();
 
   delete client;
   client_map_.erase(it);
@@ -335,7 +586,7 @@ void ResourceScheduler::OnNavigate(int child_id, int route_id) {
   }
 
   Client* client = it->second;
-  client->has_body = false;
+  client->OnNavigate();
 }
 
 void ResourceScheduler::OnWillInsertBody(int child_id, int route_id) {
@@ -349,8 +600,7 @@ void ResourceScheduler::OnWillInsertBody(int child_id, int route_id) {
   }
 
   Client* client = it->second;
-  client->has_body = true;
-  LoadAnyStartablePendingRequests(client);
+  client->OnWillInsertBody();
 }
 
 void ResourceScheduler::OnReceivedSpdyProxiedHttpResponse(
@@ -365,17 +615,7 @@ void ResourceScheduler::OnReceivedSpdyProxiedHttpResponse(
   }
 
   Client* client = client_it->second;
-
-  if (!client->using_spdy_proxy) {
-    client->using_spdy_proxy = true;
-    LoadAnyStartablePendingRequests(client);
-  }
-}
-
-void ResourceScheduler::StartRequest(ScheduledResourceRequest* request,
-                                     Client* client) {
-  client->in_flight_requests.insert(request);
-  request->Start();
+  client->OnReceivedSpdyProxiedHttpResponse();
 }
 
 void ResourceScheduler::ReprioritizeRequest(ScheduledResourceRequest* request,
@@ -387,7 +627,6 @@ void ResourceScheduler::ReprioritizeRequest(ScheduledResourceRequest* request,
     NOTREACHED();
     return;
   }
-
   RequestPriorityParams new_priority_params(new_priority,
       new_intra_priority_value);
   RequestPriorityParams old_priority_params =
@@ -395,11 +634,11 @@ void ResourceScheduler::ReprioritizeRequest(ScheduledResourceRequest* request,
 
   DCHECK(old_priority_params != new_priority_params);
 
-  request->url_request()->SetPriority(new_priority_params.priority);
-  request->set_request_priority_params(new_priority_params);
   ClientMap::iterator client_it = client_map_.find(request->client_id());
   if (client_it == client_map_.end()) {
     // The client was likely deleted shortly before we received this IPC.
+    request->url_request()->SetPriority(new_priority_params.priority);
+    request->set_request_priority_params(new_priority_params);
     return;
   }
 
@@ -407,164 +646,8 @@ void ResourceScheduler::ReprioritizeRequest(ScheduledResourceRequest* request,
     return;
 
   Client *client = client_it->second;
-  if (!client->pending_requests.IsQueued(request)) {
-    DCHECK(ContainsKey(client->in_flight_requests, request));
-    // Request has already started.
-    return;
-  }
-
-  client->pending_requests.Erase(request);
-  client->pending_requests.Insert(request);
-
-  if (new_priority_params.priority > old_priority_params.priority) {
-    // Check if this request is now able to load at its new priority.
-    LoadAnyStartablePendingRequests(client);
-  }
-}
-
-void ResourceScheduler::LoadAnyStartablePendingRequests(Client* client) {
-  // We iterate through all the pending requests, starting with the highest
-  // priority one. For each entry, one of three things can happen:
-  // 1) We start the request, remove it from the list, and keep checking.
-  // 2) We do NOT start the request, but ShouldStartRequest() signals us that
-  //    there may be room for other requests, so we keep checking and leave
-  //    the previous request still in the list.
-  // 3) We do not start the request, same as above, but StartRequest() tells
-  //    us there's no point in checking any further requests.
-  RequestQueue::NetQueue::iterator request_iter =
-      client->pending_requests.GetNextHighestIterator();
-  while (request_iter != client->pending_requests.End()) {
-    ScheduledResourceRequest* request = *request_iter;
-    ShouldStartReqResult query_result = ShouldStartRequest(request, client);
-
-    if (query_result == START_REQUEST) {
-      client->pending_requests.Erase(request);
-      StartRequest(request, client);
-
-      // StartRequest can modify the pending list, so we (re)start evaluation
-      // from the currently highest priority request. Avoid copying a singular
-      // iterator, which would trigger undefined behavior.
-      if (client->pending_requests.GetNextHighestIterator() ==
-          client->pending_requests.End())
-        break;
-      request_iter = client->pending_requests.GetNextHighestIterator();
-    } else if (query_result == DO_NOT_START_REQUEST_AND_KEEP_SEARCHING) {
-      ++request_iter;
-      continue;
-    } else {
-      DCHECK(query_result == DO_NOT_START_REQUEST_AND_STOP_SEARCHING);
-      break;
-    }
-  }
-}
-
-void ResourceScheduler::GetNumDelayableRequestsInFlight(
-    Client* client,
-    const net::HostPortPair& active_request_host,
-    size_t* total_delayable,
-    size_t* total_for_active_host) const {
-  DCHECK(client != NULL && total_delayable != NULL &&
-         total_for_active_host != NULL);
-
-  size_t total_delayable_count = 0;
-  size_t same_host_count = 0;
-  for (RequestSet::iterator it = client->in_flight_requests.begin();
-       it != client->in_flight_requests.end(); ++it) {
-    net::HostPortPair host_port_pair =
-        net::HostPortPair::FromURL((*it)->url_request()->url());
-
-    if (active_request_host.Equals(host_port_pair)) {
-      same_host_count++;
-    }
-
-    if ((*it)->url_request()->priority() < net::LOW) {
-      const net::HttpServerProperties& http_server_properties =
-          *(*it)->url_request()->context()->http_server_properties();
-
-      if (!http_server_properties.SupportsSpdy(host_port_pair)) {
-        ++total_delayable_count;
-      }
-    }
-  }
-  *total_delayable = total_delayable_count;
-  *total_for_active_host = same_host_count;
-}
-
-// ShouldStartRequest is the main scheduling algorithm.
-//
-// Requests are categorized into two categories:
-//
-// 1. Immediately issued requests, which are:
-//
-//   * Higher priority requests (>= net::LOW).
-//   * Synchronous requests.
-//   * Requests to SPDY-capable origin servers.
-//   * Non-HTTP[S] requests.
-//
-// 2. The remainder are delayable requests, which follow these rules:
-//
-//   * If no high priority requests are in flight, start loading low priority
-//     requests.
-//   * Once the renderer has a <body>, start loading delayable requests.
-//   * Never exceed 10 delayable requests in flight per client.
-//   * Never exceed 6 delayable requests for a given host.
-//   * Prior to <body>, allow one delayable request to load at a time.
-ResourceScheduler::ShouldStartReqResult ResourceScheduler::ShouldStartRequest(
-    ScheduledResourceRequest* request,
-    Client* client) const {
-  const net::URLRequest& url_request = *request->url_request();
-
-  // TODO(simonjam): This may end up causing disk contention. We should
-  // experiment with throttling if that happens.
-  if (!url_request.url().SchemeIsHTTPOrHTTPS()) {
-    return START_REQUEST;
-  }
-
-  if (client->using_spdy_proxy && url_request.url().SchemeIs("http")) {
-    return START_REQUEST;
-  }
-
-  const net::HttpServerProperties& http_server_properties =
-      *url_request.context()->http_server_properties();
-
-  if (url_request.priority() >= net::LOW ||
-      !ResourceRequestInfo::ForRequest(&url_request)->IsAsync()) {
-    return START_REQUEST;
-  }
-
-  net::HostPortPair host_port_pair =
-      net::HostPortPair::FromURL(url_request.url());
-
-  // TODO(willchan): We should really improve this algorithm as described in
-  // crbug.com/164101. Also, theoretically we should not count a SPDY request
-  // against the delayable requests limit.
-  if (http_server_properties.SupportsSpdy(host_port_pair)) {
-    return START_REQUEST;
-  }
-
-  size_t num_delayable_requests_in_flight = 0;
-  size_t num_requests_in_flight_for_host = 0;
-  GetNumDelayableRequestsInFlight(client, host_port_pair,
-                                  &num_delayable_requests_in_flight,
-                                  &num_requests_in_flight_for_host);
-
-  if (num_delayable_requests_in_flight >= kMaxNumDelayableRequestsPerClient) {
-    return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
-  }
-
-  if (num_requests_in_flight_for_host >= kMaxNumDelayableRequestsPerHost) {
-    // There may be other requests for other hosts we'd allow, so keep checking.
-    return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
-  }
-
-  bool have_immediate_requests_in_flight =
-      client->in_flight_requests.size() > num_delayable_requests_in_flight;
-  if (have_immediate_requests_in_flight && !client->has_body &&
-      num_delayable_requests_in_flight != 0) {
-    return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
-  }
-
-  return START_REQUEST;
+  client->ReprioritizeRequest(
+      request, old_priority_params, new_priority_params);
 }
 
 ResourceScheduler::ClientId ResourceScheduler::MakeClientId(