ResourceScheduler: remove dependency on ResourceRequestInfo and request_id

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 1d118f0ffbc2ae3be243e75f8eb227a1073edfcf..e90aa59e71a25aa33e2d9770be0cdf72e1fa6f0b 100644
--- a/content/browser/loader/resource_scheduler.cc
+++ b/content/browser/loader/resource_scheduler.cc
@@ -11,13 +11,12 @@
 #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"
@@ -110,17 +109,17 @@ struct ResourceScheduler::RequestPriorityParams {
 
 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())
@@ -138,7 +137,7 @@ class ResourceScheduler::RequestQueue {
   }
 
   // Returns true if |request| is queued.
-  bool IsQueued(ScheduledResourceRequest* request) const {
+  bool IsQueued(ScheduledResourceRequestImpl* request) const {
     return ContainsKey(pointers_, request);
   }
 
@@ -146,7 +145,8 @@ class ResourceScheduler::RequestQueue {
   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;
@@ -163,27 +163,41 @@ class ResourceScheduler::RequestQueue {
 
 // 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;
@@ -226,6 +240,7 @@ class ResourceScheduler::ScheduledResourceRequest
   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;
@@ -238,15 +253,16 @@ class ResourceScheduler::ScheduledResourceRequest
   }
 
  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) {}

[davidben, 2015/08/13 23:08:52]

Nit: Maybe a newline here? Not sure. Was a bit hard to tell what was going on
with the two close together like that.

[Adam Rice, 2015/08/13 23:42:28]

Done.

+    ScheduledResourceRequestImpl* const pointer;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(UnownedPointer);
+  };
+
+  static const void* const kUserDataKey;
 
   // ResourceThrottle interface:
   void WillStartRequest(bool* defer) override {
@@ -256,28 +272,36 @@ class ResourceScheduler::ScheduledResourceRequest
 
   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;

[davidben, 2015/08/13 23:08:53]

[Ah the sweet music of C++. You can tell it's music if it repeats itself a lot.
:-) ]

[Adam Rice, 2015/08/13 23:42:28]

I have to admit I borrowed this trick from someone else. If you absolutely,
positively have to have a constant unique void pointer, accept no substitute!

+
+ResourceScheduler::ScheduledResourceRequest::ScheduledResourceRequest() {}
+ResourceScheduler::ScheduledResourceRequest::~ScheduledResourceRequest() {}
+ResourceScheduler::ScheduledResourceRequest*
+ResourceScheduler::ScheduledResourceRequest::ForRequest(
+    net::URLRequest* request) {
+  return ResourceScheduler::ScheduledResourceRequestImpl::ForRequest(request);
+}
+
 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)
@@ -292,7 +316,7 @@ bool ResourceScheduler::ScheduledResourceSorter::operator()(
 }
 
 void ResourceScheduler::RequestQueue::Insert(
-    ScheduledResourceRequest* request) {
+    ScheduledResourceRequestImpl* request) {
   DCHECK(!ContainsKey(pointers_, request));
   request->set_fifo_ordering(MakeFifoOrderingId());
   pointers_[request] = queue_.insert(request);
@@ -325,9 +349,8 @@ class ResourceScheduler::Client {
     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
@@ -335,7 +358,7 @@ class ResourceScheduler::Client {
     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));
@@ -355,7 +378,7 @@ class ResourceScheduler::Client {
     // 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?
@@ -486,7 +509,7 @@ class ResourceScheduler::Client {
     }
   }
 
-  void ReprioritizeRequest(ScheduledResourceRequest* request,
+  void ReprioritizeRequest(ScheduledResourceRequestImpl* request,
                            RequestPriorityParams old_priority_params,
                            RequestPriorityParams new_priority_params) {
     request->url_request()->SetPriority(new_priority_params.priority);
@@ -560,12 +583,12 @@ class ResourceScheduler::Client {
     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.
@@ -597,7 +620,7 @@ class ResourceScheduler::Client {
     return classification_request_count;
   }
 
-  void SetRequestClassification(ScheduledResourceRequest* request,
+  void SetRequestClassification(ScheduledResourceRequestImpl* request,
                                 RequestClassification classification) {
     RequestClassification old_classification = request->classification();
     if (old_classification == classification)
@@ -621,7 +644,7 @@ class ResourceScheduler::Client {
               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 &&
@@ -661,7 +684,7 @@ class ResourceScheduler::Client {
     return false;
   }
 
-  void StartRequest(ScheduledResourceRequest* request) {
+  void StartRequest(ScheduledResourceRequestImpl* request) {
     InsertInFlightRequest(request);
     request->Start();
   }
@@ -719,11 +742,11 @@ class ResourceScheduler::Client {
   //   * 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;
     }
 
@@ -810,7 +833,7 @@ class ResourceScheduler::Client {
         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) {
@@ -905,17 +928,17 @@ ResourceScheduler::GetClientStateForTesting(int child_id, int route_id) {
   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()) {
@@ -933,7 +956,7 @@ scoped_ptr<ResourceThrottle> ResourceScheduler::ScheduleRequest(
   return request.Pass();
 }
 
-void ResourceScheduler::RemoveRequest(ScheduledResourceRequest* request) {
+void ResourceScheduler::RemoveRequest(ScheduledResourceRequestImpl* request) {
   DCHECK(CalledOnValidThread());
   if (ContainsKey(unowned_requests_, request)) {
     unowned_requests_.erase(request);
@@ -1171,9 +1194,10 @@ ResourceScheduler::ClientState ResourceScheduler::GetClientState(
   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) {
   if (request->url_request()->load_flags() & net::LOAD_IGNORE_LIMITS) {
     // We should not be re-prioritizing requests with the
     // IGNORE_LIMITS flag.