ResourceScheduler: remove dependency on ResourceRequestInfo and request_id

content/browser/loader/resource_scheduler.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.
 
 #include <set>
 
 #include "content/browser/loader/resource_scheduler.h"
 
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/supports_user_data.h"
 #include "base/time/time.h"
 #include "content/common/resource_messages.h"
-#include "content/browser/loader/resource_message_delegate.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/resource_throttle.h"
-#include "ipc/ipc_message_macros.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
 #include "net/base/request_priority.h"
 #include "net/http/http_server_properties.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 
 namespace content {
 
 namespace {
@@ skipping 72 matching lines @@
       return priority > other.priority;
     return intra_priority > other.intra_priority;
   }
 
   net::RequestPriority priority;
   int intra_priority;
 };
 
 class ResourceScheduler::RequestQueue {
  public:
-  typedef std::multiset<ScheduledResourceRequest*, ScheduledResourceSorter>
+  typedef std::multiset<ScheduledResourceRequestImpl*, ScheduledResourceSorter>
       NetQueue;
 
   RequestQueue() : fifo_ordering_ids_(0) {}
   ~RequestQueue() {}
 
   // Adds |request| to the queue with given |priority|.
-  void Insert(ScheduledResourceRequest* request);
+  void Insert(ScheduledResourceRequestImpl* request);
 
   // Removes |request| from the queue.
-  void Erase(ScheduledResourceRequest* request) {
+  void Erase(ScheduledResourceRequestImpl* request) {
     PointerMap::iterator it = pointers_.find(request);
     DCHECK(it != pointers_.end());
     if (it == pointers_.end())
       return;
     queue_.erase(it->second);
     pointers_.erase(it);
   }
 
   NetQueue::iterator GetNextHighestIterator() {
     return queue_.begin();
   }
 
   NetQueue::iterator End() {
     return queue_.end();
   }
 
   // Returns true if |request| is queued.
-  bool IsQueued(ScheduledResourceRequest* request) const {
+  bool IsQueued(ScheduledResourceRequestImpl* request) const {
     return ContainsKey(pointers_, request);
   }
 
   // Returns true if no requests are queued.
   bool IsEmpty() const { return queue_.size() == 0; }
 
  private:
-  typedef std::map<ScheduledResourceRequest*, NetQueue::iterator> PointerMap;
+  typedef std::map<ScheduledResourceRequestImpl*, NetQueue::iterator>
+      PointerMap;
 
   uint32 MakeFifoOrderingId() {
     fifo_ordering_ids_ += 1;
     return fifo_ordering_ids_;
   }
 
   // Used to create an ordering ID for scheduled resources so that resources
   // with same priority/intra_priority stay in fifo order.
   uint32 fifo_ordering_ids_;
 
   NetQueue queue_;
   PointerMap pointers_;
 };
 
 // This is the handle we return to the ResourceDispatcherHostImpl so it can
 // interact with the request.
-class ResourceScheduler::ScheduledResourceRequest
-    : public ResourceMessageDelegate,
-      public ResourceThrottle {
+class ResourceScheduler::ScheduledResourceRequestImpl
+    : public ScheduledResourceRequest {
  public:
-  ScheduledResourceRequest(const ClientId& client_id,
-                           net::URLRequest* request,
-                           ResourceScheduler* scheduler,
-                           const RequestPriorityParams& priority)
-      : ResourceMessageDelegate(request),
-        client_id_(client_id),
+  ScheduledResourceRequestImpl(const ClientId& client_id,
+                               net::URLRequest* request,
+                               ResourceScheduler* scheduler,
+                               const RequestPriorityParams& priority,
+                               bool is_async)
+      : client_id_(client_id),
         client_state_on_creation_(scheduler->GetClientState(client_id_)),
         request_(request),
         ready_(false),
         deferred_(false),
+        is_async_(is_async),
         classification_(NORMAL_REQUEST),
         scheduler_(scheduler),
         priority_(priority),
         fifo_ordering_(0) {
+    request_->SetUserData(kUserDataKey, new UnownedPointer(this));
   }
 
-  ~ScheduledResourceRequest() override { scheduler_->RemoveRequest(this); }
+  ~ScheduledResourceRequestImpl() override {
+    request_->RemoveUserData(kUserDataKey);
+    scheduler_->RemoveRequest(this);
+  }
+
+  void ChangePriority(net::RequestPriority new_priority,
+                      int intra_priority_value) override {
+    scheduler_->ReprioritizeRequest(this, new_priority, intra_priority_value);
+  }
+
+  static ScheduledResourceRequest* ForRequest(net::URLRequest* request) {
+    return static_cast<UnownedPointer*>(request->GetUserData(kUserDataKey))
+        ->pointer;
+  }
 
   void Start() {
     ready_ = true;
     if (!request_->status().is_success())
       return;
     base::TimeTicks time = base::TimeTicks::Now();
     ClientState current_state = scheduler_->GetClientState(client_id_);
     // Note: the client state isn't perfectly accurate since it won't capture
     // tabs which have switched between active and background multiple times.
     // Ex: A tab with the following transitions Active -> Background -> Active
@@ skipping 22 matching lines @@
 
   void set_request_priority_params(const RequestPriorityParams& priority) {
     priority_ = priority;
   }
   const RequestPriorityParams& get_request_priority_params() const {
     return priority_;
   }
   const ClientId& client_id() const { return client_id_; }
   net::URLRequest* url_request() { return request_; }
   const net::URLRequest* url_request() const { return request_; }
+  bool is_async() const { return is_async_; }
   uint32 fifo_ordering() const { return fifo_ordering_; }
   void set_fifo_ordering(uint32 fifo_ordering) {
     fifo_ordering_ = fifo_ordering;
   }
   RequestClassification classification() const {
     return classification_;
   }
   void set_classification(RequestClassification classification) {
     classification_ = classification;
   }
 
  private:
-  // ResourceMessageDelegate interface:
-  bool OnMessageReceived(const IPC::Message& message) override {
-    bool handled = true;
-    IPC_BEGIN_MESSAGE_MAP(ScheduledResourceRequest, message)
-      IPC_MESSAGE_HANDLER(ResourceHostMsg_DidChangePriority, DidChangePriority)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_END_MESSAGE_MAP()
-    return handled;
-  }
+  class UnownedPointer : public base::SupportsUserData::Data {
+   public:
+    UnownedPointer(ScheduledResourceRequestImpl* pointer) : pointer(pointer) {}

[mmenke, 2015/08/14 15:23:14]

explicit

[Adam Rice, 2015/08/14 17:17:22]

Done.

+
+    ScheduledResourceRequestImpl* const pointer;

[mmenke, 2015/08/14 15:23:15]

This should be private, with an accessor, per Google style guide.

[Adam Rice, 2015/08/14 17:17:23]

Done.

+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(UnownedPointer);
+  };
+
+  static const void* const kUserDataKey;
 
   // ResourceThrottle interface:
   void WillStartRequest(bool* defer) override {
     deferred_ = *defer = !ready_;
     time_deferred_ = base::TimeTicks::Now();
   }
 
   const char* GetNameForLogging() const override { return "ResourceScheduler"; }
 
-  void DidChangePriority(int request_id, net::RequestPriority new_priority,
-                         int intra_priority_value) {
-    scheduler_->ReprioritizeRequest(this, new_priority, intra_priority_value);
-  }
-
   const ClientId client_id_;
   const ResourceScheduler::ClientState client_state_on_creation_;
   net::URLRequest* request_;
   bool ready_;
   bool deferred_;
+  bool is_async_;
   RequestClassification classification_;
   ResourceScheduler* scheduler_;
   RequestPriorityParams priority_;
   uint32 fifo_ordering_;
   base::TimeTicks time_deferred_;
 
-  DISALLOW_COPY_AND_ASSIGN(ScheduledResourceRequest);
+  DISALLOW_COPY_AND_ASSIGN(ScheduledResourceRequestImpl);
 };
 
+const void* const
+    ResourceScheduler::ScheduledResourceRequestImpl::kUserDataKey =
+        &ResourceScheduler::ScheduledResourceRequestImpl::kUserDataKey;
+
+ResourceScheduler::ScheduledResourceRequest::ScheduledResourceRequest() {}
+ResourceScheduler::ScheduledResourceRequest::~ScheduledResourceRequest() {}
+ResourceScheduler::ScheduledResourceRequest*
+ResourceScheduler::ScheduledResourceRequest::ForRequest(
+    net::URLRequest* request) {
+  return ResourceScheduler::ScheduledResourceRequestImpl::ForRequest(request);
+}

[mmenke, 2015/08/14 15:23:14]

nit:  believe there should be a blank line between these method definitions.

[Adam Rice, 2015/08/14 17:17:23]

Removed.

+
 bool ResourceScheduler::ScheduledResourceSorter::operator()(
-    const ScheduledResourceRequest* a,
-    const ScheduledResourceRequest* b) const {
+    const ScheduledResourceRequestImpl* a,
+    const ScheduledResourceRequestImpl* b) const {
   // Want the set to be ordered first by decreasing priority, then by
   // decreasing intra_priority.
   // ie. with (priority, intra_priority)
   // [(1, 0), (1, 0), (0, 100), (0, 0)]
   if (a->get_request_priority_params() != b->get_request_priority_params())
     return a->get_request_priority_params().GreaterThan(
         b->get_request_priority_params());
 
   // If priority/intra_priority is the same, fall back to fifo ordering.
   // std::multiset doesn't guarantee this until c++11.
   return a->fifo_ordering() < b->fifo_ordering();
 }
 
 void ResourceScheduler::RequestQueue::Insert(
-    ScheduledResourceRequest* request) {
+    ScheduledResourceRequestImpl* request) {
   DCHECK(!ContainsKey(pointers_, request));
   request->set_fifo_ordering(MakeFifoOrderingId());
   pointers_[request] = queue_.insert(request);
 }
 
 // Each client represents a tab.
 class ResourceScheduler::Client {
  public:
   explicit Client(ResourceScheduler* scheduler,
                   bool is_visible,
@@ skipping 12 matching lines @@
 
   ~Client() {
     // Update to default state and pause to ensure the scheduler has a
     // correct count of relevant types of clients.
     is_visible_ = false;
     is_audible_ = false;
     is_paused_ = true;
     UpdateThrottleState();
   }
 
-  void ScheduleRequest(
-      net::URLRequest* url_request,
-      ScheduledResourceRequest* request) {
+  void ScheduleRequest(net::URLRequest* url_request,
+                       ScheduledResourceRequestImpl* request) {
     if (ShouldStartRequest(request) == START_REQUEST)
       StartRequest(request);
     else
       pending_requests_.Insert(request);
     SetRequestClassification(request, ClassifyRequest(request));
   }
 
-  void RemoveRequest(ScheduledResourceRequest* request) {
+  void RemoveRequest(ScheduledResourceRequestImpl* 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 StartAndRemoveAllRequests() {
     // First start any pending requests so that they will be moved into
     // in_flight_requests_. This may exceed the limits
     // kMaxNumDelayableRequestsPerClient, kMaxNumDelayableRequestsPerHost and
     // kMaxNumThrottledRequestsPerClient, so this method must not do anything
     // that depends on those limits before calling ClearInFlightRequests()
     // below.
     while (!pending_requests_.IsEmpty()) {
-      ScheduledResourceRequest* request =
+      ScheduledResourceRequestImpl* request =
           *pending_requests_.GetNextHighestIterator();
       pending_requests_.Erase(request);
       // StartRequest() may modify pending_requests_. TODO(ricea): Does it?
       StartRequest(request);
     }
     RequestSet unowned_requests;
     for (RequestSet::iterator it = in_flight_requests_.begin();
          it != in_flight_requests_.end(); ++it) {
       unowned_requests.insert(*it);
       (*it)->set_classification(NORMAL_REQUEST);
@@ skipping 110 matching lines @@
     LoadAnyStartablePendingRequests();
   }
 
   void OnReceivedSpdyProxiedHttpResponse() {
     if (!using_spdy_proxy_) {
       using_spdy_proxy_ = true;
       LoadAnyStartablePendingRequests();
     }
   }
 
-  void ReprioritizeRequest(ScheduledResourceRequest* request,
+  void ReprioritizeRequest(ScheduledResourceRequestImpl* 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 of the request and priority support of the server may
       // have changed, so update the delayable count.
       SetRequestClassification(request, ClassifyRequest(request));
       // Request has already started.
@@ skipping 53 matching lines @@
     SetThrottleState(COALESCED);
   }
 
  private:
   enum ShouldStartReqResult {
     DO_NOT_START_REQUEST_AND_STOP_SEARCHING,
     DO_NOT_START_REQUEST_AND_KEEP_SEARCHING,
     START_REQUEST,
   };
 
-  void InsertInFlightRequest(ScheduledResourceRequest* request) {
+  void InsertInFlightRequest(ScheduledResourceRequestImpl* request) {
     in_flight_requests_.insert(request);
     SetRequestClassification(request, ClassifyRequest(request));
   }
 
-  void EraseInFlightRequest(ScheduledResourceRequest* request) {
+  void EraseInFlightRequest(ScheduledResourceRequestImpl* request) {
     size_t erased = in_flight_requests_.erase(request);
     DCHECK_EQ(1u, erased);
     // Clear any special state that we were tracking for this request.
     SetRequestClassification(request, NORMAL_REQUEST);
   }
 
   void ClearInFlightRequests() {
     in_flight_requests_.clear();
     in_flight_delayable_count_ = 0;
     total_layout_blocking_count_ = 0;
@@ skipping 11 matching lines @@
       for (RequestQueue::NetQueue::const_iterator
            it = pending_requests_.GetNextHighestIterator();
            it != pending_requests_.End(); ++it) {
         if ((*it)->classification() == classification)
           classification_request_count++;
       }
     }
     return classification_request_count;
   }
 
-  void SetRequestClassification(ScheduledResourceRequest* request,
+  void SetRequestClassification(ScheduledResourceRequestImpl* request,
                                 RequestClassification classification) {
     RequestClassification old_classification = request->classification();
     if (old_classification == classification)
       return;
 
     if (old_classification == IN_FLIGHT_DELAYABLE_REQUEST)
       in_flight_delayable_count_--;
     if (old_classification == LAYOUT_BLOCKING_REQUEST)
       total_layout_blocking_count_--;
 
     if (classification == IN_FLIGHT_DELAYABLE_REQUEST)
       in_flight_delayable_count_++;
     if (classification == LAYOUT_BLOCKING_REQUEST)
       total_layout_blocking_count_++;
 
     request->set_classification(classification);
     DCHECK_EQ(
         CountRequestsWithClassification(IN_FLIGHT_DELAYABLE_REQUEST, false),
         in_flight_delayable_count_);
     DCHECK_EQ(CountRequestsWithClassification(LAYOUT_BLOCKING_REQUEST, true),
               total_layout_blocking_count_);
   }
 
-  RequestClassification ClassifyRequest(ScheduledResourceRequest* request) {
+  RequestClassification ClassifyRequest(ScheduledResourceRequestImpl* request) {
     // If a request is already marked as layout-blocking make sure to keep the
     // classification across redirects unless the priority was lowered.
     if (request->classification() == 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;
 
@@ skipping 19 matching lines @@
           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) {
+  void StartRequest(ScheduledResourceRequestImpl* request) {
     InsertInFlightRequest(request);
     request->Start();
   }
 
   // ShouldStartRequest is the main scheduling algorithm.
   //
   // Requests are evaluated on five attributes:
   //
   // 1. Non-delayable requests:
   //   * Synchronous requests.
@@ skipping 37 matching lines @@
   //   * If no high priority requests are in flight, start loading low priority
   //     requests.
   //
   //  COALESCED Clients never load requests, with the following exceptions:
   //   * Non-delayable requests are issued imediately.
   //   * On a (currently 5 second) heart beat, they load all requests as an
   //     UNTHROTTLED Client, and then return to the COALESCED state.
   //   * When an active Client makes a request, they are THROTTLED until the
   //     active Client finishes loading.
   ShouldStartReqResult ShouldStartRequest(
-      ScheduledResourceRequest* request) const {
+      ScheduledResourceRequestImpl* request) const {
     const net::URLRequest& url_request = *request->url_request();
     // Syncronous requests could block the entire render, which could impact
     // user-observable Clients.
-    if (!ResourceRequestInfo::ForRequest(&url_request)->IsAsync()) {
+    if (!request->is_async()) {
       return START_REQUEST;
     }
 
     // TODO(simonjam): This may end up causing disk contention. We should
     // experiment with throttling if that happens.
     // TODO(aiolos): We probably want to Coalesce these as well to avoid
     // waking the disk.
     if (!url_request.url().SchemeIsHTTPOrHTTPS()) {
       return START_REQUEST;
     }
@@ skipping 66 matching lines @@
     // 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;
+      ScheduledResourceRequestImpl* 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() ==
@@ skipping 74 matching lines @@
   OnLoadingActiveClientsStateChangedForAllClients();
 }
 
 ResourceScheduler::ClientThrottleState
 ResourceScheduler::GetClientStateForTesting(int child_id, int route_id) {
   Client* client = GetClient(child_id, route_id);
   DCHECK(client);
   return client->throttle_state();
 }
 
-scoped_ptr<ResourceThrottle> ResourceScheduler::ScheduleRequest(
-    int child_id,
-    int route_id,
-    net::URLRequest* url_request) {
+scoped_ptr<ResourceScheduler::ScheduledResourceRequest>
+ResourceScheduler::ScheduleRequest(int child_id,
+                                   int route_id,
+                                   bool is_async,
+                                   net::URLRequest* url_request) {
   DCHECK(CalledOnValidThread());
   ClientId client_id = MakeClientId(child_id, route_id);
-  scoped_ptr<ScheduledResourceRequest> request(new ScheduledResourceRequest(
-      client_id,
-      url_request,
-      this,
-      RequestPriorityParams(url_request->priority(), 0)));
+  scoped_ptr<ScheduledResourceRequestImpl> request(
+      new ScheduledResourceRequestImpl(
+          client_id, url_request, this,
+          RequestPriorityParams(url_request->priority(), 0), is_async));
 
   ClientMap::iterator it = client_map_.find(client_id);
   if (it == client_map_.end()) {
     // There are several ways this could happen:
     // 1. <a ping> requests don't have a route_id.
     // 2. Most unittests don't send the IPCs needed to register Clients.
     // 3. The tab is closed while a RequestResource IPC is in flight.
     unowned_requests_.insert(request.get());
     request->Start();
     return request.Pass();
   }
 
   Client* client = it->second;
   client->ScheduleRequest(url_request, request.get());
   return request.Pass();
 }
 
-void ResourceScheduler::RemoveRequest(ScheduledResourceRequest* request) {
+void ResourceScheduler::RemoveRequest(ScheduledResourceRequestImpl* request) {
   DCHECK(CalledOnValidThread());
   if (ContainsKey(unowned_requests_, request)) {
     unowned_requests_.erase(request);
     return;
   }
 
   ClientMap::iterator client_it = client_map_.find(request->client_id());
   if (client_it == client_map_.end()) {
     return;
   }
@@ skipping 217 matching lines @@
 }
 
 ResourceScheduler::ClientState ResourceScheduler::GetClientState(
     ClientId client_id) const {
   ClientMap::const_iterator client_it = client_map_.find(client_id);
   if (client_it == client_map_.end())
     return UNKNOWN;
   return client_it->second->is_active() ? ACTIVE : BACKGROUND;
 }
 
-void ResourceScheduler::ReprioritizeRequest(ScheduledResourceRequest* request,
-                                            net::RequestPriority new_priority,
-                                            int new_intra_priority_value) {
+void ResourceScheduler::ReprioritizeRequest(
+    ScheduledResourceRequestImpl* request,
+    net::RequestPriority new_priority,
+    int new_intra_priority_value) {

[mmenke, 2015/08/14 15:23:14]

Why not have the RDH call this method with the URLRequest*, and have this method
look up the ScheduledResourceRequest from the URLRequest*?  Then you hide all
the details of the lookup from the RDH.

[Adam Rice, 2015/08/14 17:17:22]

Oh. Thank you. Done.

   if (request->url_request()->load_flags() & net::LOAD_IGNORE_LIMITS) {
     // We should not be re-prioritizing requests with the
     // IGNORE_LIMITS flag.
     NOTREACHED();
     return;
   }
   RequestPriorityParams new_priority_params(new_priority,
       new_intra_priority_value);
   RequestPriorityParams old_priority_params =
       request->get_request_priority_params();
@@ skipping 15 matching lines @@
   client->ReprioritizeRequest(
       request, old_priority_params, new_priority_params);
 }
 
 ResourceScheduler::ClientId ResourceScheduler::MakeClientId(
     int child_id, int route_id) {
   return (static_cast<ResourceScheduler::ClientId>(child_id) << 32) | route_id;
 }
 
 }  // namespace content