ResourceScheduler: remove dependency on ResourceRequestInfo and request_id

content/browser/loader/resource_scheduler_unittest.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 "content/browser/loader/resource_scheduler.h"
 
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/mock_entropy_provider.h"
 #include "base/timer/mock_timer.h"
 #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"
 #include "content/test/test_web_contents.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/request_priority.h"
 #include "net/http/http_server_properties_impl.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/latency_info.h"
 
 using std::string;
 
 namespace content {
 
 namespace {
 
 class TestRequestFactory;
 
 const int kChildId = 30;
 const int kRouteId = 75;
 const int kChildId2 = 43;
 const int kRouteId2 = 67;
 const int kBackgroundChildId = 35;
 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 {}
+  ~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.

+  }
 
   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(); }
 
  protected:
   // ResourceController interface:
   void CancelAndIgnore() override {}
   void CancelWithError(int error_code) override {}
   void Resume() override { started_ = true; }
 
  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();
   }
 
  private:
   void Resume() override {
     TestRequest::Resume();
     request_to_cancel_.reset();
   }
 
   scoped_ptr<TestRequest> request_to_cancel_;
 };
 
 class FakeResourceContext : public ResourceContext {
  private:
   net::HostResolver* GetHostResolver() override { return NULL; }
   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();
     context_.set_http_server_properties(http_server_properties_.GetWeakPtr());
   }
 
   ~ResourceSchedulerTest() override {
     CleanupScheduler();
   }
 
@@ skipping 31 matching lines @@
     return base::FieldTrialList::CreateTrialsFromString(
         force_field_trial_argument,
         base::FieldTrialList::DONT_ACTIVATE_TRIALS,
         std::set<std::string>());
   }
 
   scoped_ptr<net::URLRequest> NewURLRequestWithChildAndRoute(
       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,
                                    net::RequestPriority priority,
                                    int route_id) {
     return NewRequestWithChildAndRoute(url, priority, route_id, kChildId);
   }
 
   TestRequest* NewRequestWithChildAndRoute(const char* url,
                                            net::RequestPriority priority,
@@ skipping 27 matching lines @@
                                                int child_id,
                                                int route_id) {
     return GetNewTestRequest(url, priority, child_id, route_id, false);
   }
 
   TestRequest* GetNewTestRequest(const char* url,
                                  net::RequestPriority priority,
                                  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;
   }
 
   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() {
     EXPECT_TRUE(mock_timer_->IsRunning());
     mock_timer_->Fire();
   }
 
   ResourceScheduler* scheduler() {
     return scheduler_.get();
   }
 
-  int next_request_id_;
   base::MessageLoopForIO message_loop_;
   BrowserThreadImpl ui_thread_;
   BrowserThreadImpl io_thread_;
   ResourceDispatcherHostImpl rdh_;
   scoped_ptr<ResourceScheduler> scheduler_;
   base::FieldTrialList field_trial_list_;
   base::MockTimer* mock_timer_;
   net::HttpServerPropertiesImpl http_server_properties_;
   net::TestURLRequestContext context_;
 };
@@ skipping 100 matching lines @@
   high2.reset();
   EXPECT_TRUE(low2->started());
 }
 
 TEST_F(ResourceSchedulerTest, CancelOtherRequestsWhileResuming) {
   scoped_ptr<TestRequest> high(NewRequest("http://host/high", net::HIGHEST));
   scoped_ptr<TestRequest> low1(NewRequest("http://host/low1", net::LOWEST));
 
   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));
   low2->set_request_to_cancel(low3.Pass());
   scoped_ptr<TestRequest> low4(NewRequest("http://host/low4", net::LOWEST));
 
   EXPECT_TRUE(high->started());
   EXPECT_FALSE(low2->started());
   high.reset();
   EXPECT_TRUE(low1->started());
@@ skipping 1903 matching lines @@
   EXPECT_FALSE(lowest->started());
   scheduler_->OnClientDeleted(kChildId2, kRouteId2);
   high.reset();
   delayable_requests.clear();
   EXPECT_TRUE(lowest->started());
 }
 
 }  // unnamed namespace
 
 }  // namespace content