Changed resource scheduler to block until all critical resources actually load.

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 85adbdb30e275631b53af88286e75b658e73a16c..4ff46abef379fa50ab4875a9b398fb21bff7b3dd 100644
--- a/content/browser/loader/resource_scheduler.cc
+++ b/content/browser/loader/resource_scheduler.cc
@@ -128,7 +128,7 @@ class ResourceScheduler::ScheduledResourceRequest
         scheduler_(scheduler),
         priority_(priority),
         fifo_ordering_(0),
-        accounted_as_delayable_request_(false) {
+        category_(NORMAL_REQUEST) {
     TRACE_EVENT_ASYNC_BEGIN1("net", "URLRequest", request_,
                              "url", request->url().spec());
   }
@@ -159,11 +159,11 @@ 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_;
+  RequestCategory category() const {
+    return category_;
   }
-  void set_accounted_as_delayable_request(bool accounted) {
-    accounted_as_delayable_request_ = accounted;
+  void set_category(RequestCategory category) {
+    category_ = category;
   }
 
  private:
@@ -195,11 +195,10 @@ class ResourceScheduler::ScheduledResourceRequest
   net::URLRequest* request_;
   bool ready_;
   bool deferred_;
+  RequestCategory category_;
   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);
 };
@@ -237,7 +236,8 @@ class ResourceScheduler::Client {
         is_paused_(false),
         has_body_(false),
         using_spdy_proxy_(false),
-        total_delayable_count_(0),
+        in_flight_delayable_count_(0),
+        total_layout_blocking_count_(0),
         throttle_state_(ResourceScheduler::THROTTLED) {
     scheduler_ = scheduler;
   }
@@ -258,6 +258,13 @@ class ResourceScheduler::Client {
       StartRequest(request);
     } else {
       pending_requests_.Insert(request);
+      // Keep track of all layout-blocking requests, not just ones that are
+      // in-flight.  StartRequest() updates the accounting for ones that are
+      // in-flight and this catches the ones that are queued. The accounting
+      // needs to be done after it is in the queue so the DCHECK to validate
+      // the counts is accurate.
+      if (GetRequestCategory(request) == LAYOUT_BLOCKING_REQUEST)
+        SetRequestCategory(request, LAYOUT_BLOCKING_REQUEST);
     }
   }
 
@@ -278,7 +285,7 @@ class ResourceScheduler::Client {
     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);
+      (*it)->set_category(NORMAL_REQUEST);
     }
     ClearInFlightRequests();
     return unowned_requests;
@@ -374,7 +381,7 @@ class ResourceScheduler::Client {
       DCHECK(ContainsKey(in_flight_requests_, request));
       // The priority and SPDY support may have changed, so update the
       // delayable count.
-      SetRequestDelayable(request, IsDelayableRequest(request));
+      SetRequestCategory(request, GetRequestCategory(request));
       // Request has already started.
       return;
     }
@@ -441,44 +448,84 @@ class ResourceScheduler::Client {
 
   void InsertInFlightRequest(ScheduledResourceRequest* request) {
     in_flight_requests_.insert(request);
-    if (IsDelayableRequest(request))
-      SetRequestDelayable(request, true);
+    SetRequestCategory(request, GetRequestCategory(request));
   }
 
   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());
+    // Clear any special state that we were tracking for this request.
+    SetRequestCategory(request, NORMAL_REQUEST);
   }
 
   void ClearInFlightRequests() {
     in_flight_requests_.clear();
-    total_delayable_count_ = 0;
+    in_flight_delayable_count_ = 0;
+    total_layout_blocking_count_ = 0;
   }
 
-  bool IsDelayableRequest(ScheduledResourceRequest* request) {
+  size_t CountRequestsInCategory(const RequestCategory category,
+                                 const bool include_pending) {
+    size_t category_request_count = 0;
+    for (RequestSet::const_iterator it = in_flight_requests_.begin();
+         it != in_flight_requests_.end(); ++it) {
+      if ((*it)->category() == category)
+        category_request_count++;
+    }
+    if (include_pending) {
+      for (RequestQueue::NetQueue::const_iterator
+           it = pending_requests_.GetNextHighestIterator();
+           it != pending_requests_.End(); ++it) {
+        if ((*it)->category() == category)
+          category_request_count++;
+      }
+    }
+    return category_request_count;
+  }
+
+  void SetRequestCategory(ScheduledResourceRequest* request,
+                          RequestCategory category) {
+    RequestCategory old_category = request->category();
+    if (old_category == category)
+      return;
+
+    if (old_category == DELAYABLE_REQUEST)
+      in_flight_delayable_count_--;
+    if (old_category == LAYOUT_BLOCKING_REQUEST)
+      total_layout_blocking_count_--;
+
+    if (category == DELAYABLE_REQUEST)
+      in_flight_delayable_count_++;
+    if (category == LAYOUT_BLOCKING_REQUEST)
+      total_layout_blocking_count_++;
+
+    request->set_category(category);
+    DCHECK_EQ(CountRequestsInCategory(DELAYABLE_REQUEST, false),
+              in_flight_delayable_count_);
+    DCHECK_EQ(CountRequestsInCategory(LAYOUT_BLOCKING_REQUEST, true),
+              total_layout_blocking_count_);
+  }
+
+  RequestCategory GetRequestCategory(ScheduledResourceRequest* request) {
+    // If a request is already marked as layout-blocking make sure to keep the
+    // category across redirects unless the priority was lowered.
+    if (request->category() == LAYOUT_BLOCKING_REQUEST &&
+        request->url_request()->priority() >= net::LOW) {
+      return LAYOUT_BLOCKING_REQUEST;
+    }
+
+    if (!has_body_ && request->url_request()->priority() >= net::LOW)
+      return LAYOUT_BLOCKING_REQUEST;
     if (request->url_request()->priority() < net::LOW) {
       net::HostPortPair host_port_pair =
           net::HostPortPair::FromURL(request->url_request()->url());
       net::HttpServerProperties& http_server_properties =
           *request->url_request()->context()->http_server_properties();
       if (!http_server_properties.SupportsSpdy(host_port_pair)) {
-        return true;
+        return DELAYABLE_REQUEST;
       }
     }
-    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);
+    return NORMAL_REQUEST;
   }
 
   bool ShouldKeepSearching(
@@ -504,7 +551,7 @@ class ResourceScheduler::Client {
 
   // ShouldStartRequest is the main scheduling algorithm.
   //
-  // Requests are categorized into three categories:
+  // Requests are categorized into five categories:

[mmenke, 2014/08/18 16:30:20]

Think it's worth noting these are not mutually exclusive categories.  For a
throttle renderer, layout-blocking SPDY requests are treated different from
layout-blocking non-SPDY requests, and non-layout-blocking SPDY requests, for
instance.

[Pat Meenan, 2014/08/18 18:29:57]

Done. (changed the language to say it evaluates requests based on 5 attributes).

   //
   // 1. Non-delayable requests:
   //   * Synchronous requests.
@@ -515,20 +562,25 @@ class ResourceScheduler::Client {
   // 3. High-priority requests:
   //   * Higher priority requests (>= net::LOW).
   //
-  // 4. Low priority requests
+  // 4. Layout-blocking requests:
+  //   * High-priority requests initiated before the renderer has a <body>.
+  //
+  // 5. Low priority requests
   //
   //  The following rules are followed:
   //
   //  ACTIVE_AND_LOADING and UNTHROTTLED Clients follow these rules:
-  //   * Non-delayable, High-priority and SDPY capable requests are issued
-  //     immediately
+  //   * Non-delayable, High-priority and SPDY capable requests are issued
+  //     immediately.
   //   * If no high priority requests are in flight, start loading low priority
   //     requests.

[mmenke, 2014/08/18 16:30:20]

This seems redundant with the two you added/modified below.  Suggest just
getting rid of it, and maybe updating the "once all..." line to include the case
there are no layout-blocking requests.

[Pat Meenan, 2014/08/18 18:29:57]

Done.

   //   * Low priority requests are delayable.
-  //   * Once the renderer has a <body>, start loading delayable requests.
+  //   * Allow one delayable request to load at a time while layout-blocking
+  //     requests are loading.
+  //   * Once all layout-blocking requests have finished loading, 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.
   //
   //  THROTTLED Clients follow these rules:
   //   * Non-delayable and SPDY-capable requests are issued immediately.
@@ -585,12 +637,12 @@ class ResourceScheduler::Client {
       return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
     }
 
+    // High-priority and layout-blocking requests.
     if (url_request.priority() >= net::LOW) {
       return START_REQUEST;
     }
 
-    size_t num_delayable_requests_in_flight = total_delayable_count_;
-    if (num_delayable_requests_in_flight >= kMaxNumDelayableRequestsPerClient) {
+    if (in_flight_delayable_count_ >= kMaxNumDelayableRequestsPerClient) {
       return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
     }
 
@@ -601,9 +653,10 @@ class ResourceScheduler::Client {
     }
 
     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) {
+        in_flight_requests_.size() > in_flight_delayable_count_;
+    if (have_immediate_requests_in_flight &&
+        total_layout_blocking_count_ != 0 &&
+        in_flight_delayable_count_ != 0) {
       return DO_NOT_START_REQUEST_AND_STOP_SEARCHING;
     }
 
@@ -657,7 +710,9 @@ class ResourceScheduler::Client {
   RequestSet in_flight_requests_;
   ResourceScheduler* scheduler_;
   // The number of delayable in-flight requests.
-  size_t total_delayable_count_;
+  size_t in_flight_delayable_count_;
+  // The number of layout-blocking in-flight requests.
+  size_t total_layout_blocking_count_;
   ResourceScheduler::ClientThrottleState throttle_state_;
 };