ResourceScheduler: remove dependency on ResourceRequestInfo and request_id

content/browser/loader/resource_scheduler_unittest.cc

Index: content/browser/loader/resource_scheduler_unittest.cc
diff --git a/content/browser/loader/resource_scheduler_unittest.cc b/content/browser/loader/resource_scheduler_unittest.cc
index 3ac956667bfdafb138962616a723766662ad8c10..a98c48af8e57a0b85c488f71f527eecf829a079e 100644
--- a/content/browser/loader/resource_scheduler_unittest.cc
+++ b/content/browser/loader/resource_scheduler_unittest.cc
@@ -14,15 +14,10 @@
 #include "base/timer/timer.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
-#include "content/browser/loader/resource_message_filter.h"
-#include "content/browser/loader/resource_request_info_impl.h"
-#include "content/common/resource_messages.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_throttle.h"

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

No longer needed?

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

Done.

 #include "content/public/common/content_switches.h"
-#include "content/public/common/process_type.h"
-#include "content/public/common/resource_type.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/test/test_render_view_host_factory.h"
@@ -52,28 +47,38 @@ const int kBackgroundRouteId = 43;
 const int kBackgroundChildId2 = 54;
 const int kBackgroundRouteId2 = 82;
 
+typedef ResourceScheduler::ScheduledResourceRequest ScheduledResourceRequest;
+
 class TestRequest : public ResourceController {
  public:
-  TestRequest(scoped_ptr<ResourceThrottle> throttle,
+  TestRequest(scoped_ptr<ScheduledResourceRequest> scheduled_resource_request,
               scoped_ptr<net::URLRequest> url_request)
       : started_(false),
-        throttle_(throttle.Pass()),
+        scheduled_resource_request_(scheduled_resource_request.Pass()),
         url_request_(url_request.Pass()) {
-    throttle_->set_controller_for_testing(this);
+    scheduled_resource_request_->set_controller_for_testing(this);
+  }
+  ~TestRequest() override {
+    // The URLRequest must still be valid when the ScheduledResourceRequest is
+    // destroyed, so that it can unregister itself.
+    scheduled_resource_request_.reset();

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

Perhaps just change the order of url_request_ and scheduled_resource_request_?
The constraint is that the URLRequest must outlive the SRR, right?

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

Yes. But I have kept the comment & reset() just to make it explicit what is
going on. I was confused when the tests started throwing memory errors, so I'm
being extra cautious.

   }
-  ~TestRequest() override {}
 
   bool started() const { return started_; }
 
   void Start() {
     bool deferred = false;
-    throttle_->WillStartRequest(&deferred);
+    scheduled_resource_request_->WillStartRequest(&deferred);
     started_ = !deferred;
   }
 
+  void ChangePriority(net::RequestPriority new_priority, int intra_priority) {
+    scheduled_resource_request_->ChangePriority(new_priority, intra_priority);
+  }
+
   void Cancel() override {
     // Alert the scheduler that the request can be deleted.
-    throttle_.reset(0);
+    scheduled_resource_request_.reset();
   }
 
   const net::URLRequest* url_request() const { return url_request_.get(); }
@@ -86,15 +91,16 @@ class TestRequest : public ResourceController {
 
  private:
   bool started_;
-  scoped_ptr<ResourceThrottle> throttle_;
+  scoped_ptr<ScheduledResourceRequest> scheduled_resource_request_;
   scoped_ptr<net::URLRequest> url_request_;
 };
 
 class CancelingTestRequest : public TestRequest {
  public:
-  CancelingTestRequest(scoped_ptr<ResourceThrottle> throttle,
-                       scoped_ptr<net::URLRequest> url_request)
-      : TestRequest(throttle.Pass(), url_request.Pass()) {}
+  CancelingTestRequest(
+      scoped_ptr<ScheduledResourceRequest> scheduled_resource_request,
+      scoped_ptr<net::URLRequest> url_request)
+      : TestRequest(scheduled_resource_request.Pass(), url_request.Pass()) {}
 
   void set_request_to_cancel(scoped_ptr<TestRequest> request_to_cancel) {
     request_to_cancel_ = request_to_cancel.Pass();
@@ -115,39 +121,10 @@ class FakeResourceContext : public ResourceContext {
   net::URLRequestContext* GetRequestContext() override { return NULL; }
 };
 
-class FakeResourceMessageFilter : public ResourceMessageFilter {
- public:
-  FakeResourceMessageFilter(int child_id)
-      : ResourceMessageFilter(
-          child_id,
-          PROCESS_TYPE_RENDERER,
-          NULL  /* appcache_service */,
-          NULL  /* blob_storage_context */,
-          NULL  /* file_system_context */,
-          NULL  /* service_worker_context */,
-          NULL  /* host_zoom_level_context */,
-          base::Bind(&FakeResourceMessageFilter::GetContexts,
-                     base::Unretained(this))) {
-  }
-
- private:
-  ~FakeResourceMessageFilter() override {}
-
-  void GetContexts(const ResourceHostMsg_Request& request,
-                   ResourceContext** resource_context,
-                   net::URLRequestContext** request_context) {
-    *resource_context = &context_;
-    *request_context = NULL;
-  }
-
-  FakeResourceContext context_;
-};
-
 class ResourceSchedulerTest : public testing::Test {
  protected:
   ResourceSchedulerTest()
-      : next_request_id_(0),
-        ui_thread_(BrowserThread::UI, &message_loop_),
+      : ui_thread_(BrowserThread::UI, &message_loop_),
         io_thread_(BrowserThread::IO, &message_loop_),
         field_trial_list_(new base::MockEntropyProvider()) {
     InitializeScheduler();
@@ -199,44 +176,15 @@ class ResourceSchedulerTest : public testing::Test {
       const char* url,
       net::RequestPriority priority,
       int child_id,
-      int route_id,
-      bool is_async) {
+      int route_id) {
     scoped_ptr<net::URLRequest> url_request(
         context_.CreateRequest(GURL(url), priority, NULL));
-    ResourceRequestInfoImpl* info = new ResourceRequestInfoImpl(
-        PROCESS_TYPE_RENDERER,                   // process_type
-        child_id,                                // child_id
-        route_id,                                // route_id
-        -1,                                      // frame_tree_node_id
-        0,                                       // origin_pid
-        ++next_request_id_,                      // request_id
-        MSG_ROUTING_NONE,                        // render_frame_id
-        false,                                   // is_main_frame
-        false,                                   // parent_is_main_frame
-        0,                                       // parent_render_frame_id
-        RESOURCE_TYPE_SUB_RESOURCE,              // resource_type
-        ui::PAGE_TRANSITION_LINK,                // transition_type
-        false,                                   // should_replace_current_entry
-        false,                                   // is_download
-        false,                                   // is_stream
-        true,                                    // allow_download
-        false,                                   // has_user_gesture
-        false,                                   // enable_load_timing
-        false,                                   // enable_upload_progress
-        false,                                   // do_not_prompt_for_login
-        blink::WebReferrerPolicyDefault,         // referrer_policy
-        blink::WebPageVisibilityStateVisible,    // visibility_state
-        NULL,                                    // context
-        base::WeakPtr<ResourceMessageFilter>(),  // filter
-        is_async);                               // is_async
-    info->AssociateWithRequest(url_request.get());
     return url_request.Pass();
   }
 
   scoped_ptr<net::URLRequest> NewURLRequest(const char* url,
                                             net::RequestPriority priority) {
-    return NewURLRequestWithChildAndRoute(
-        url, priority, kChildId, kRouteId, true);
+    return NewURLRequestWithChildAndRoute(url, priority, kChildId, kRouteId);
   }
 
   TestRequest* NewRequestWithRoute(const char* url,
@@ -284,11 +232,13 @@ class ResourceSchedulerTest : public testing::Test {
                                  int child_id,
                                  int route_id,
                                  bool is_async) {
-    scoped_ptr<net::URLRequest> url_request(NewURLRequestWithChildAndRoute(
-        url, priority, child_id, route_id, is_async));
-    scoped_ptr<ResourceThrottle> throttle(
-        scheduler_->ScheduleRequest(child_id, route_id, url_request.get()));
-    TestRequest* request = new TestRequest(throttle.Pass(), url_request.Pass());
+    scoped_ptr<net::URLRequest> url_request(
+        NewURLRequestWithChildAndRoute(url, priority, child_id, route_id));
+    scoped_ptr<ScheduledResourceRequest> scheduled_resource_request(
+        scheduler_->ScheduleRequest(child_id, route_id, is_async,
+                                    url_request.get()));
+    TestRequest* request =
+        new TestRequest(scheduled_resource_request.Pass(), url_request.Pass());
     request->Start();
     return request;
   }
@@ -296,14 +246,7 @@ class ResourceSchedulerTest : public testing::Test {
   void ChangeRequestPriority(TestRequest* request,
                              net::RequestPriority new_priority,
                              int intra_priority = 0) {
-    scoped_refptr<FakeResourceMessageFilter> filter(
-        new FakeResourceMessageFilter(kChildId));
-    const ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest(
-        request->url_request());
-    const GlobalRequestID& id = info->GetGlobalRequestID();
-    ResourceHostMsg_DidChangePriority msg(id.request_id, new_priority,
-                                          intra_priority);
-    rdh_.OnMessageReceived(msg, filter.get());
+    request->ChangePriority(new_priority, intra_priority);
   }
 
   void FireCoalescingTimer() {
@@ -315,7 +258,6 @@ class ResourceSchedulerTest : public testing::Test {
     return scheduler_.get();
   }
 
-  int next_request_id_;
   base::MessageLoopForIO message_loop_;
   BrowserThreadImpl ui_thread_;
   BrowserThreadImpl io_thread_;
@@ -436,10 +378,11 @@ TEST_F(ResourceSchedulerTest, CancelOtherRequestsWhileResuming) {
 
   scoped_ptr<net::URLRequest> url_request(
       NewURLRequest("http://host/low2", net::LOWEST));
-  scoped_ptr<ResourceThrottle> throttle(
-      scheduler()->ScheduleRequest(kChildId, kRouteId, url_request.get()));
+  scoped_ptr<ScheduledResourceRequest> scheduled_resource_request(
+      scheduler()->ScheduleRequest(kChildId, kRouteId, true,
+                                   url_request.get()));
   scoped_ptr<CancelingTestRequest> low2(new CancelingTestRequest(
-      throttle.Pass(), url_request.Pass()));
+      scheduled_resource_request.Pass(), url_request.Pass()));
   low2->Start();
 
   scoped_ptr<TestRequest> low3(NewRequest("http://host/low3", net::LOWEST));