Add MaybeInterceptRedirect/Response to URLRequestInterceptor

net/url_request/url_request_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 "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 54 matching lines @@
 #include "net/proxy/proxy_service.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/test/cert_test_util.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "net/url_request/data_protocol_handler.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_http_job.h"
+#include "net/url_request/url_request_intercepting_job_factory.h"
+#include "net/url_request/url_request_interceptor.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_redirect_job.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 #if !defined(DISABLE_FILE_SUPPORT)
 #include "net/base/filename_util.h"
 #include "net/url_request/file_protocol_handler.h"
@@ skipping 543 matching lines @@
     return protocol_handler_;
   }
 
  protected:
   CapturingNetLog net_log_;
   TestNetworkDelegate default_network_delegate_;  // Must outlive URLRequest.
   URLRequestJobFactoryImpl job_factory_;
   TestURLRequestContext default_context_;
 };
 
+

[mmenke, 2014/11/05 18:43:44]

nit:  Remove extra line break.  Looks like you tried to be systematic in where
you added them, but doesn't seem to get us much, and the prevailing style is not
to use them.

[bengr, 2014/11/05 22:51:39]

Done.

 TEST_F(URLRequestTest, AboutBlankTest) {
   TestDelegate d;
   {
     scoped_ptr<URLRequest> r(default_context_.CreateRequest(
         GURL("about:blank"), DEFAULT_PRIORITY, &d, NULL));
 
     r->Start();
     EXPECT_TRUE(r->is_pending());
 
     base::RunLoop().Run();
@@ skipping 919 matching lines @@
 
   // Check the interceptor got called as expected
   EXPECT_TRUE(interceptor.did_cancel_then_restart_main_);
   EXPECT_FALSE(interceptor.did_intercept_final_);
 
   // Check we see a canceled request
   EXPECT_FALSE(req->status().is_success());
   EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
 }
 
+// An Interceptor for use with interceptor tests

[mmenke, 2014/11/05 18:43:44]

nit:  +.

[bengr, 2014/11/05 22:51:39]

Done.

+class MockURLRequestInterceptor : public URLRequestInterceptor {
+ public:
+  MockURLRequestInterceptor()
+      : intercept_main_request_(false), restart_main_request_(false),
+        cancel_main_request_(false), cancel_then_restart_main_request_(false),
+        simulate_main_network_error_(false),
+        intercept_redirect_(false), cancel_redirect_request_(false),
+        intercept_final_response_(false), cancel_final_request_(false),
+        use_url_request_http_job_(false),
+        did_intercept_main_(false), did_restart_main_(false),
+        did_cancel_main_(false), did_cancel_then_restart_main_(false),
+        did_simulate_error_main_(false),
+        did_intercept_redirect_(false), did_cancel_redirect_(false),
+        did_intercept_final_(false), did_cancel_final_(false) {
+  }
+
+  ~MockURLRequestInterceptor() override {
+  }
+

[mmenke, 2014/11/05 18:43:44]

nit:  // URLRequestInterceptor implementation:

[bengr, 2014/11/05 22:51:37]

Done.

+  virtual URLRequestJob* MaybeInterceptRequest(

[mmenke, 2014/11/05 18:43:45]

nit:  -virtual

[bengr, 2014/11/05 22:51:37]

Done.

+      URLRequest* request,
+      NetworkDelegate* network_delegate) override {
+    if (restart_main_request_) {
+      restart_main_request_ = false;
+      did_restart_main_ = true;
+      return new RestartTestJob(request, network_delegate);
+    }
+    if (cancel_main_request_) {
+      cancel_main_request_ = false;
+      did_cancel_main_ = true;
+      return new CancelTestJob(request, network_delegate);
+    }
+    if (cancel_then_restart_main_request_) {
+      cancel_then_restart_main_request_ = false;
+      did_cancel_then_restart_main_ = true;
+      return new CancelThenRestartTestJob(request, network_delegate);
+    }
+    if (simulate_main_network_error_) {
+      simulate_main_network_error_ = false;
+      did_simulate_error_main_ = true;
+      // will error since the requeted url is not one of its canned urls

[mmenke, 2014/11/05 18:43:44]

nit:  Comments should all be complete sentences (Capitalize + period).

[bengr, 2014/11/05 22:51:38]

Done.

+      if (use_url_request_http_job_) {
+        return URLRequestHttpJob::Factory(request, network_delegate, "http");
+      }
+      return new URLRequestTestJob(request, network_delegate, true);
+    }
+    if (!intercept_main_request_)
+      return NULL;
+    intercept_main_request_ = false;
+    did_intercept_main_ = true;
+    URLRequestTestJob* job =  new URLRequestTestJob(request,
+                                                    network_delegate,
+                                                    main_headers_,
+                                                    main_data_,
+                                                    true);
+    job->set_load_timing_info(main_request_load_timing_info_);
+    return job;
+  }
+
+  URLRequestJob* MaybeInterceptRedirect(URLRequest* request,
+                                        NetworkDelegate* network_delegate,
+                                        const GURL& location) override {
+    if (cancel_redirect_request_) {
+      cancel_redirect_request_ = false;
+      did_cancel_redirect_ = true;
+      return new CancelTestJob(request, network_delegate);
+    }
+    if (!intercept_redirect_)
+      return NULL;
+    intercept_redirect_ = false;
+    did_intercept_redirect_ = true;
+    if (use_url_request_http_job_) {
+      return URLRequestHttpJob::Factory(request, network_delegate, "http");
+    }
+    return new URLRequestTestJob(request,
+                                 network_delegate,
+                                 redirect_headers_,
+                                 redirect_data_,
+                                 true);
+  }
+
+  URLRequestJob* MaybeInterceptResponse(
+      URLRequest* request,
+      NetworkDelegate* network_delegate) override {
+    if (cancel_final_request_) {
+      cancel_final_request_ = false;
+      did_cancel_final_ = true;
+      return new CancelTestJob(request, network_delegate);
+    }
+    if (!intercept_final_response_)
+      return NULL;
+    intercept_final_response_ = false;
+    did_intercept_final_ = true;
+    if (use_url_request_http_job_) {
+      return URLRequestHttpJob::Factory(request, network_delegate, "http");
+    }
+    return new URLRequestTestJob(request,
+                                 network_delegate,
+                                 final_headers_,
+                                 final_data_,
+                                 true);
+  }
+
+  // Whether to intercept the main request, and if so the response to return and
+  // the LoadTimingInfo to use.
+  bool intercept_main_request_;

[mmenke, 2014/11/05 18:43:44]

Per google style guide, these should all be private.

[bengr, 2014/11/05 22:51:37]

Done.

+  std::string main_headers_;
+  std::string main_data_;
+  LoadTimingInfo main_request_load_timing_info_;
+
+  // Other actions we take at MaybeIntercept time

[mmenke, 2014/11/05 18:43:44]

Comments should all be capitalized, end with periods.

[bengr, 2014/11/05 22:51:38]

Done.

+  bool restart_main_request_;
+  bool cancel_main_request_;
+  bool cancel_then_restart_main_request_;
+  bool simulate_main_network_error_;
+
+  // Whether to intercept redirects, and if so the response to return.
+  bool intercept_redirect_;
+  std::string redirect_headers_;
+  std::string redirect_data_;
+
+  // Other actions we can take at MaybeInterceptRedirect time
+  bool cancel_redirect_request_;
+
+  // Whether to intercept final response, and if so the response to return.
+  bool intercept_final_response_;
+  std::string final_headers_;
+  std::string final_data_;
+
+  // Other actions we can take at MaybeInterceptResponse time
+  bool cancel_final_request_;
+
+  bool use_url_request_http_job_;
+
+  // If we did something or not
+  bool did_intercept_main_;
+  bool did_restart_main_;
+  bool did_cancel_main_;
+  bool did_cancel_then_restart_main_;
+  bool did_simulate_error_main_;
+  bool did_intercept_redirect_;
+  bool did_cancel_redirect_;
+  bool did_intercept_final_;
+  bool did_cancel_final_;
+
+  // Static getters for canned response header and data strings
+
+  static std::string ok_data() {
+    return URLRequestTestJob::test_data_1();
+  }
+
+  static std::string ok_headers() {
+    return URLRequestTestJob::test_headers();
+  }
+
+  static std::string redirect_data() {
+    return std::string();
+  }
+
+  static std::string redirect_headers() {
+    return URLRequestTestJob::test_redirect_headers();
+  }
+
+  static std::string error_data() {
+    return std::string("ohhh nooooo mr. bill!");
+  }
+
+  static std::string error_headers() {
+    return URLRequestTestJob::test_error_headers();
+  }
+};
+
+// Inherit PlatformTest since we require the autorelease pool on Mac OS X.
+class URLRequestInterceptorTest : public PlatformTest {

[mmenke, 2014/11/05 18:43:44]

Could you add a TODO about merging this with the URLRequestInterceptorHTTPTest
fixture?  I'm not a big fan of URLRequestTestJob, so would like to remove the
dependency on it at some point (And maybe move these tests into the factory
tests - they're fine as-is for the moment, though).

[bengr, 2014/11/05 22:51:38]

Done.

+ public:
+  URLRequestInterceptorTest() : default_context_(true) {
+    default_context_.set_network_delegate(&default_network_delegate_);
+    default_context_.set_net_log(&net_log_);
+    job_factory_impl_ = new URLRequestJobFactoryImpl();
+    job_factory_.reset(job_factory_impl_);
+    job_factory_impl_->SetProtocolHandler("data", new DataProtocolHandler);
+#if !defined(DISABLE_FILE_SUPPORT)
+    job_factory_impl_->SetProtocolHandler(
+        "file", new FileProtocolHandler(base::MessageLoopProxy::current()));
+#endif

[mmenke, 2014/11/05 18:43:44]

Know you're just copying here, but is this SetProtocolHandler call needed?  I
don't see any tests ifdef-ed out, so I don't think it's used?

Or better, can we just inherit from URLRequestTest, and add a method:

virtual scoped_ptr<URLRequestFactory> SetUpFactory(scoped_ptr<URLRequestFactory
base_factory)?

And have the other tests inherit from URLRequestHTTP test.

[bengr, 2014/11/05 22:51:38]

Done.

+    interceptor_ = new MockURLRequestInterceptor();
+    job_factory_.reset(new URLRequestInterceptingJobFactory(
+        job_factory_.Pass(), make_scoped_ptr(interceptor_)));
+    default_context_.set_job_factory(job_factory_.get());
+    default_context_.Init();
+  }
+  ~URLRequestInterceptorTest() override {

[mmenke, 2014/11/05 18:43:44]

nit:  Blank line before destructor definition.

[bengr, 2014/11/05 22:51:38]

Done.

+    // URLRequestJobs may post clean-up tasks on destruction.
+    base::RunLoop().RunUntilIdle();
+  }
+
+ protected:

[mmenke, 2014/11/05 18:43:44]

These should all be private.

[bengr, 2014/11/05 22:51:39]

Done.

+  CapturingNetLog net_log_;
+  TestNetworkDelegate default_network_delegate_;  // Must outlive URLRequest.
+  URLRequestJobFactoryImpl* job_factory_impl_;
+  scoped_ptr<URLRequestJobFactory> job_factory_;
+  TestURLRequestContext default_context_;
+  MockURLRequestInterceptor* interceptor_;
+};
+
+

[mmenke, 2014/11/05 18:43:44]

nit:  Remove blank line.

[bengr, 2014/11/05 22:51:38]

Done.

+TEST_F(URLRequestInterceptorTest, Intercept) {
+  // intercept the main request and respond with a simple response

[mmenke, 2014/11/05 18:43:44]

These should all be sentences (Start with capital, end with period).

[bengr, 2014/11/05 22:51:37]

Done.

+  interceptor_->intercept_main_request_ = true;
+  interceptor_->main_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->main_data_ = MockURLRequestInterceptor::ok_data();
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  base::SupportsUserData::Data* user_data0 = new base::SupportsUserData::Data();
+  base::SupportsUserData::Data* user_data1 = new base::SupportsUserData::Data();
+  base::SupportsUserData::Data* user_data2 = new base::SupportsUserData::Data();
+  req->SetUserData(NULL, user_data0);
+  req->SetUserData(&user_data1, user_data1);
+  req->SetUserData(&user_data2, user_data2);
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Make sure we can retrieve our specific user data
+  EXPECT_EQ(user_data0, req->GetUserData(NULL));
+  EXPECT_EQ(user_data1, req->GetUserData(&user_data1));
+  EXPECT_EQ(user_data2, req->GetUserData(&user_data2));
+
+  // Check we got one good response
+  EXPECT_TRUE(req->status().is_success());
+  EXPECT_EQ(200, req->response_headers()->response_code());
+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRedirect) {
+  // intercept the main request and respond with a redirect
+  interceptor_->intercept_main_request_ = true;
+  interceptor_->main_headers_ = MockURLRequestInterceptor::redirect_headers();
+  interceptor_->main_data_ = MockURLRequestInterceptor::redirect_data();
+
+  // intercept that redirect and respond a final OK response
+  interceptor_->intercept_redirect_ = true;
+  interceptor_->redirect_headers_ =  MockURLRequestInterceptor::ok_headers();
+  interceptor_->redirect_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_intercept_main_);
+  EXPECT_TRUE(interceptor_->did_intercept_redirect_);
+
+  // Check we got one good response
+  EXPECT_TRUE(req->status().is_success());
+  if (req->status().is_success()) {
+    EXPECT_EQ(200, req->response_headers()->response_code());
+  }

[mmenke, 2014/11/05 18:43:44]

nit:  Don't use braces with one line ifs.

[bengr, 2014/11/05 22:51:38]

Done.

+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptServerError) {
+  // intercept the main request to generate a server error response
+  interceptor_->intercept_main_request_ = true;
+  interceptor_->main_headers_ = MockURLRequestInterceptor::error_headers();
+  interceptor_->main_data_ = MockURLRequestInterceptor::error_data();
+
+  // intercept that error and respond with an OK response
+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->final_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_intercept_main_);
+  EXPECT_TRUE(interceptor_->did_intercept_final_);
+
+  // Check we got one good response
+  EXPECT_TRUE(req->status().is_success());
+  EXPECT_EQ(200, req->response_headers()->response_code());
+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptNetworkError) {
+  // intercept the main request to simulate a network error
+  interceptor_->simulate_main_network_error_ = true;
+
+  // intercept that error and respond with an OK response
+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->final_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_simulate_error_main_);
+  EXPECT_TRUE(interceptor_->did_intercept_final_);
+
+  // Check we received one good response
+  EXPECT_TRUE(req->status().is_success());
+  EXPECT_EQ(200, req->response_headers()->response_code());
+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRestartRequired) {
+  // restart the main request
+  interceptor_->restart_main_request_ = true;
+
+  // then intercept the new main request and respond with an OK response
+  interceptor_->intercept_main_request_ = true;
+  interceptor_->main_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->main_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_restart_main_);
+  EXPECT_TRUE(interceptor_->did_intercept_main_);
+
+  // Check we received one good response
+  EXPECT_TRUE(req->status().is_success());
+  if (req->status().is_success()) {
+    EXPECT_EQ(200, req->response_headers()->response_code());
+  }

[mmenke, 2014/11/05 18:43:44]

nit:  Remove braces.

[bengr, 2014/11/05 22:51:37]

Done.

+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRespectsCancelMain) {
+  // intercept the main request and cancel from within the restarted job
+  interceptor_->cancel_main_request_ = true;
+
+  // setup to intercept final response and override it with an OK response
+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->final_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_cancel_main_);
+  EXPECT_FALSE(interceptor_->did_intercept_final_);
+
+  // Check we see a canceled request
+  EXPECT_FALSE(req->status().is_success());
+  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRespectsCancelRedirect) {
+  // intercept the main request and respond with a redirect
+  interceptor_->intercept_main_request_ = true;
+  interceptor_->main_headers_ = MockURLRequestInterceptor::redirect_headers();
+  interceptor_->main_data_ = MockURLRequestInterceptor::redirect_data();
+
+  // intercept the redirect and cancel from within that job
+  interceptor_->cancel_redirect_request_ = true;
+
+  // setup to intercept final response and override it with an OK response

[mmenke, 2014/11/05 18:43:44]

nit:  Set up

[bengr, 2014/11/05 22:51:38]

Done.

+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->final_data_ = MockURLRequestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_intercept_main_);
+  EXPECT_TRUE(interceptor_->did_cancel_redirect_);
+  EXPECT_FALSE(interceptor_->did_intercept_final_);
+
+  // Check we see a canceled request
+  EXPECT_FALSE(req->status().is_success());
+  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRespectsCancelFinal) {
+  // intercept the main request to simulate a network error
+  interceptor_->simulate_main_network_error_ = true;
+
+  // setup to intercept final response and cancel from within that job
+  interceptor_->cancel_final_request_ = true;
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_simulate_error_main_);
+  EXPECT_TRUE(interceptor_->did_cancel_final_);
+
+  // Check we see a canceled request
+  EXPECT_FALSE(req->status().is_success());
+  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
+}
+
+TEST_F(URLRequestInterceptorTest, InterceptRespectsCancelInRestart) {
+  // intercept the main request and cancel then restart from within that job
+  interceptor_->cancel_then_restart_main_request_ = true;
+
+  // setup to intercept final response and override it with an OK response
+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = TestInterceptor::ok_headers();
+  interceptor_->final_data_ = TestInterceptor::ok_data();
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  // Check the interceptor got called as expected
+  EXPECT_TRUE(interceptor_->did_cancel_then_restart_main_);
+  EXPECT_FALSE(interceptor_->did_intercept_final_);
+
+  // Check we see a canceled request
+  EXPECT_FALSE(req->status().is_success());
+  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
+}
+
+

[mmenke, 2014/11/05 18:43:45]

nit:  Remove extra blank line.

[bengr, 2014/11/05 22:51:37]

Done.

 LoadTimingInfo RunLoadTimingTest(const LoadTimingInfo& job_load_timing,
                                  URLRequestContext* context) {
   TestInterceptor interceptor;
   interceptor.intercept_main_request_ = true;
   interceptor.main_request_load_timing_info_ = job_load_timing;

[mmenke, 2014/11/05 18:43:44]

How do these tests work?  You aren't injecting the interceptor into the context.

Since the context is already initialized, it seems a bad idea to do that here,
actually.  I think it's simplest just to make this a method of
URLRequestInterceptorTest and use its interceptor_.

[bengr, 2014/11/05 22:51:38]

Acknowledged.

   TestDelegate d;
   scoped_ptr<URLRequest> req(context->CreateRequest(
       GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
+  req->Start();
+  base::RunLoop().Run();
+
+  LoadTimingInfo resulting_load_timing;
+  req->GetLoadTimingInfo(&resulting_load_timing);
+
+  // None of these should be modified by the URLRequest.
+  EXPECT_EQ(job_load_timing.socket_reused, resulting_load_timing.socket_reused);
+  EXPECT_EQ(job_load_timing.socket_log_id, resulting_load_timing.socket_log_id);
+  EXPECT_EQ(job_load_timing.send_start, resulting_load_timing.send_start);
+  EXPECT_EQ(job_load_timing.send_end, resulting_load_timing.send_end);
+  EXPECT_EQ(job_load_timing.receive_headers_end,
+            resulting_load_timing.receive_headers_end);
+
+  return resulting_load_timing;
+}
+
+// "Normal" LoadTimingInfo as returned by a job.  Everything is in order, not
+// reused.  |connect_time_flags| is used to indicate if there should be dns
+// or SSL times, and |used_proxy| is used for proxy times.
+LoadTimingInfo NormalLoadTimingInfo(base::TimeTicks now,
+                                    int connect_time_flags,
+                                    bool used_proxy) {
+  LoadTimingInfo load_timing;
+  load_timing.socket_log_id = 1;
+
+  if (used_proxy) {
+    load_timing.proxy_resolve_start = now + base::TimeDelta::FromDays(1);
+    load_timing.proxy_resolve_end = now + base::TimeDelta::FromDays(2);
+  }
+
+  LoadTimingInfo::ConnectTiming& connect_timing = load_timing.connect_timing;
+  if (connect_time_flags & CONNECT_TIMING_HAS_DNS_TIMES) {
+    connect_timing.dns_start = now + base::TimeDelta::FromDays(3);
+    connect_timing.dns_end = now + base::TimeDelta::FromDays(4);
+  }
+  connect_timing.connect_start = now + base::TimeDelta::FromDays(5);
+  if (connect_time_flags & CONNECT_TIMING_HAS_SSL_TIMES) {
+    connect_timing.ssl_start = now + base::TimeDelta::FromDays(6);
+    connect_timing.ssl_end = now + base::TimeDelta::FromDays(7);
+  }
+  connect_timing.connect_end = now + base::TimeDelta::FromDays(8);
+
+  load_timing.send_start = now + base::TimeDelta::FromDays(9);
+  load_timing.send_end = now + base::TimeDelta::FromDays(10);
+  load_timing.receive_headers_end = now + base::TimeDelta::FromDays(11);
+  return load_timing;
+}
+
+// Same as above, but in the case of a reused socket.
+LoadTimingInfo NormalLoadTimingInfoReused(base::TimeTicks now,
+                                          bool used_proxy) {
+  LoadTimingInfo load_timing;
+  load_timing.socket_log_id = 1;
+  load_timing.socket_reused = true;
+
+  if (used_proxy) {
+    load_timing.proxy_resolve_start = now + base::TimeDelta::FromDays(1);
+    load_timing.proxy_resolve_end = now + base::TimeDelta::FromDays(2);
+  }
+
+  load_timing.send_start = now + base::TimeDelta::FromDays(9);
+  load_timing.send_end = now + base::TimeDelta::FromDays(10);
+  load_timing.receive_headers_end = now + base::TimeDelta::FromDays(11);
+  return load_timing;
+}
+
+// Basic test that the intercept + load timing tests work.
+TEST_F(URLRequestTest, InterceptLoadTiming) {

[mmenke, 2014/11/05 18:43:44]

We don't need two sets of load timing tests - these are just using the
interceptors to test load timing behavior, as opposed to testing the load timing
behavior of the interceptors.

[bengr, 2014/11/05 22:51:39]

Done.

+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing =
+      NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_DNS_TIMES, false);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Nothing should have been changed by the URLRequest.
+  EXPECT_EQ(job_load_timing.proxy_resolve_start,
+            load_timing_result.proxy_resolve_start);
+  EXPECT_EQ(job_load_timing.proxy_resolve_end,
+            load_timing_result.proxy_resolve_end);
+  EXPECT_EQ(job_load_timing.connect_timing.dns_start,
+            load_timing_result.connect_timing.dns_start);
+  EXPECT_EQ(job_load_timing.connect_timing.dns_end,
+            load_timing_result.connect_timing.dns_end);
+  EXPECT_EQ(job_load_timing.connect_timing.connect_start,
+            load_timing_result.connect_timing.connect_start);
+  EXPECT_EQ(job_load_timing.connect_timing.connect_end,
+            load_timing_result.connect_timing.connect_end);
+  EXPECT_EQ(job_load_timing.connect_timing.ssl_start,
+            load_timing_result.connect_timing.ssl_start);
+  EXPECT_EQ(job_load_timing.connect_timing.ssl_end,
+            load_timing_result.connect_timing.ssl_end);
+
+  // Redundant sanity check.
+  TestLoadTimingNotReused(load_timing_result, CONNECT_TIMING_HAS_DNS_TIMES);
+}
+
+// Another basic test, with proxy and SSL times, but no DNS times.
+TEST_F(URLRequestTest, InterceptLoadTimingProxy) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing =
+      NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_SSL_TIMES, true);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Nothing should have been changed by the URLRequest.
+  EXPECT_EQ(job_load_timing.proxy_resolve_start,
+            load_timing_result.proxy_resolve_start);
+  EXPECT_EQ(job_load_timing.proxy_resolve_end,
+            load_timing_result.proxy_resolve_end);
+  EXPECT_EQ(job_load_timing.connect_timing.dns_start,
+            load_timing_result.connect_timing.dns_start);
+  EXPECT_EQ(job_load_timing.connect_timing.dns_end,
+            load_timing_result.connect_timing.dns_end);
+  EXPECT_EQ(job_load_timing.connect_timing.connect_start,
+            load_timing_result.connect_timing.connect_start);
+  EXPECT_EQ(job_load_timing.connect_timing.connect_end,
+            load_timing_result.connect_timing.connect_end);
+  EXPECT_EQ(job_load_timing.connect_timing.ssl_start,
+            load_timing_result.connect_timing.ssl_start);
+  EXPECT_EQ(job_load_timing.connect_timing.ssl_end,
+            load_timing_result.connect_timing.ssl_end);
+
+  // Redundant sanity check.
+  TestLoadTimingNotReusedWithProxy(load_timing_result,
+                                   CONNECT_TIMING_HAS_SSL_TIMES);
+}
+
+// Make sure that URLRequest correctly adjusts proxy times when they're before
+// |request_start|, due to already having a connected socket.  This happens in
+// the case of reusing a SPDY session.  The connected socket is not considered
+// reused in this test (May be a preconnect).
+//
+// To mix things up from the test above, assumes DNS times but no SSL times.
+TEST_F(URLRequestTest, InterceptLoadTimingEarlyProxyResolution) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing =
+      NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_DNS_TIMES, true);
+  job_load_timing.proxy_resolve_start = now - base::TimeDelta::FromDays(6);
+  job_load_timing.proxy_resolve_end = now - base::TimeDelta::FromDays(5);
+  job_load_timing.connect_timing.dns_start = now - base::TimeDelta::FromDays(4);
+  job_load_timing.connect_timing.dns_end = now - base::TimeDelta::FromDays(3);
+  job_load_timing.connect_timing.connect_start =
+      now - base::TimeDelta::FromDays(2);
+  job_load_timing.connect_timing.connect_end =
+      now - base::TimeDelta::FromDays(1);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Proxy times, connect times, and DNS times should all be replaced with
+  // request_start.
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.proxy_resolve_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.proxy_resolve_end);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.dns_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.dns_end);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.connect_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.connect_end);
+
+  // Other times should have been left null.
+  TestLoadTimingNotReusedWithProxy(load_timing_result,
+                                   CONNECT_TIMING_HAS_DNS_TIMES);
+}
+
+// Same as above, but in the reused case.
+TEST_F(URLRequestTest, InterceptLoadTimingEarlyProxyResolutionReused) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing = NormalLoadTimingInfoReused(now, true);
+  job_load_timing.proxy_resolve_start = now - base::TimeDelta::FromDays(4);
+  job_load_timing.proxy_resolve_end = now - base::TimeDelta::FromDays(3);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Proxy times and connect times should all be replaced with request_start.
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.proxy_resolve_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.proxy_resolve_end);
+
+  // Other times should have been left null.
+  TestLoadTimingReusedWithProxy(load_timing_result);
+}
+
+// Make sure that URLRequest correctly adjusts connect times when they're before
+// |request_start|, due to reusing a connected socket.  The connected socket is
+// not considered reused in this test (May be a preconnect).
+//
+// To mix things up, the request has SSL times, but no DNS times.
+TEST_F(URLRequestTest, InterceptLoadTimingEarlyConnect) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing =
+      NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_SSL_TIMES, false);
+  job_load_timing.connect_timing.connect_start =
+      now - base::TimeDelta::FromDays(1);
+  job_load_timing.connect_timing.ssl_start = now - base::TimeDelta::FromDays(2);
+  job_load_timing.connect_timing.ssl_end = now - base::TimeDelta::FromDays(3);
+  job_load_timing.connect_timing.connect_end =
+      now - base::TimeDelta::FromDays(4);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Connect times, and SSL times should be replaced with request_start.
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.connect_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.ssl_start);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.ssl_end);
+  EXPECT_EQ(load_timing_result.request_start,
+            load_timing_result.connect_timing.connect_end);
+
+  // Other times should have been left null.
+  TestLoadTimingNotReused(load_timing_result, CONNECT_TIMING_HAS_SSL_TIMES);
+}
+
+// Make sure that URLRequest correctly adjusts connect times when they're before
+// |request_start|, due to reusing a connected socket in the case that there
+// are also proxy times.  The connected socket is not considered reused in this
+// test (May be a preconnect).
+//
+// In this test, there are no SSL or DNS times.
+TEST_F(URLRequestTest, InterceptLoadTimingEarlyConnectWithProxy) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  LoadTimingInfo job_load_timing =
+      NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_CONNECT_TIMES_ONLY, true);
+  job_load_timing.connect_timing.connect_start =
+      now - base::TimeDelta::FromDays(1);
+  job_load_timing.connect_timing.connect_end =
+      now - base::TimeDelta::FromDays(2);
+
+  LoadTimingInfo load_timing_result =
+      RunLoadTimingTest(job_load_timing, &default_context_);
+
+  // Connect times should be replaced with proxy_resolve_end.
+  EXPECT_EQ(load_timing_result.proxy_resolve_end,
+            load_timing_result.connect_timing.connect_start);
+  EXPECT_EQ(load_timing_result.proxy_resolve_end,
+            load_timing_result.connect_timing.connect_end);
+
+  // Other times should have been left null.
+  TestLoadTimingNotReusedWithProxy(load_timing_result,
+                                   CONNECT_TIMING_HAS_CONNECT_TIMES_ONLY);
+}
+
+LoadTimingInfo RunURLRequestInterceptorLoadTimingTest(
+    const LoadTimingInfo& job_load_timing,
+    URLRequestContext* context,
+    MockURLRequestInterceptor* interceptor) {
+  interceptor->intercept_main_request_ = true;
+  interceptor->main_request_load_timing_info_ = job_load_timing;
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(context->CreateRequest(
+      GURL("http://test_intercept/foo"), DEFAULT_PRIORITY, &d, NULL));
   req->Start();
   base::RunLoop().Run();
 
   LoadTimingInfo resulting_load_timing;
   req->GetLoadTimingInfo(&resulting_load_timing);
 
   // None of these should be modified by the URLRequest.
   EXPECT_EQ(job_load_timing.socket_reused, resulting_load_timing.socket_reused);
   EXPECT_EQ(job_load_timing.socket_log_id, resulting_load_timing.socket_log_id);
   EXPECT_EQ(job_load_timing.send_start, resulting_load_timing.send_start);
   EXPECT_EQ(job_load_timing.send_end, resulting_load_timing.send_end);
   EXPECT_EQ(job_load_timing.receive_headers_end,
             resulting_load_timing.receive_headers_end);
 
   return resulting_load_timing;
 }
 
-// "Normal" LoadTimingInfo as returned by a job.  Everything is in order, not
-// reused.  |connect_time_flags| is used to indicate if there should be dns
-// or SSL times, and |used_proxy| is used for proxy times.
-LoadTimingInfo NormalLoadTimingInfo(base::TimeTicks now,
-                                    int connect_time_flags,
-                                    bool used_proxy) {
-  LoadTimingInfo load_timing;
-  load_timing.socket_log_id = 1;
-
-  if (used_proxy) {
-    load_timing.proxy_resolve_start = now + base::TimeDelta::FromDays(1);
-    load_timing.proxy_resolve_end = now + base::TimeDelta::FromDays(2);
-  }
-
-  LoadTimingInfo::ConnectTiming& connect_timing = load_timing.connect_timing;
-  if (connect_time_flags & CONNECT_TIMING_HAS_DNS_TIMES) {
-    connect_timing.dns_start = now + base::TimeDelta::FromDays(3);
-    connect_timing.dns_end = now + base::TimeDelta::FromDays(4);
-  }
-  connect_timing.connect_start = now + base::TimeDelta::FromDays(5);
-  if (connect_time_flags & CONNECT_TIMING_HAS_SSL_TIMES) {
-    connect_timing.ssl_start = now + base::TimeDelta::FromDays(6);
-    connect_timing.ssl_end = now + base::TimeDelta::FromDays(7);
-  }
-  connect_timing.connect_end = now + base::TimeDelta::FromDays(8);
-
-  load_timing.send_start = now + base::TimeDelta::FromDays(9);
-  load_timing.send_end = now + base::TimeDelta::FromDays(10);
-  load_timing.receive_headers_end = now + base::TimeDelta::FromDays(11);
-  return load_timing;
-}
-
-// Same as above, but in the case of a reused socket.
-LoadTimingInfo NormalLoadTimingInfoReused(base::TimeTicks now,
-                                          bool used_proxy) {
-  LoadTimingInfo load_timing;
-  load_timing.socket_log_id = 1;
-  load_timing.socket_reused = true;
-
-  if (used_proxy) {
-    load_timing.proxy_resolve_start = now + base::TimeDelta::FromDays(1);
-    load_timing.proxy_resolve_end = now + base::TimeDelta::FromDays(2);
-  }
-
-  load_timing.send_start = now + base::TimeDelta::FromDays(9);
-  load_timing.send_end = now + base::TimeDelta::FromDays(10);
-  load_timing.receive_headers_end = now + base::TimeDelta::FromDays(11);
-  return load_timing;
-}
-
 // Basic test that the intercept + load timing tests work.
-TEST_F(URLRequestTest, InterceptLoadTiming) {
+TEST_F(URLRequestInterceptorTest, InterceptLoadTiming) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing =
       NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_DNS_TIMES, false);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Nothing should have been changed by the URLRequest.
   EXPECT_EQ(job_load_timing.proxy_resolve_start,
             load_timing_result.proxy_resolve_start);
   EXPECT_EQ(job_load_timing.proxy_resolve_end,
             load_timing_result.proxy_resolve_end);
   EXPECT_EQ(job_load_timing.connect_timing.dns_start,
             load_timing_result.connect_timing.dns_start);
   EXPECT_EQ(job_load_timing.connect_timing.dns_end,
             load_timing_result.connect_timing.dns_end);
   EXPECT_EQ(job_load_timing.connect_timing.connect_start,
             load_timing_result.connect_timing.connect_start);
   EXPECT_EQ(job_load_timing.connect_timing.connect_end,
             load_timing_result.connect_timing.connect_end);
   EXPECT_EQ(job_load_timing.connect_timing.ssl_start,
             load_timing_result.connect_timing.ssl_start);
   EXPECT_EQ(job_load_timing.connect_timing.ssl_end,
             load_timing_result.connect_timing.ssl_end);
 
   // Redundant sanity check.
   TestLoadTimingNotReused(load_timing_result, CONNECT_TIMING_HAS_DNS_TIMES);
 }
 
 // Another basic test, with proxy and SSL times, but no DNS times.
-TEST_F(URLRequestTest, InterceptLoadTimingProxy) {
+TEST_F(URLRequestInterceptorTest, InterceptLoadTimingProxy) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing =
       NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_SSL_TIMES, true);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Nothing should have been changed by the URLRequest.
   EXPECT_EQ(job_load_timing.proxy_resolve_start,
             load_timing_result.proxy_resolve_start);
   EXPECT_EQ(job_load_timing.proxy_resolve_end,
             load_timing_result.proxy_resolve_end);
   EXPECT_EQ(job_load_timing.connect_timing.dns_start,
             load_timing_result.connect_timing.dns_start);
   EXPECT_EQ(job_load_timing.connect_timing.dns_end,
             load_timing_result.connect_timing.dns_end);
@@ skipping 10 matching lines @@
   TestLoadTimingNotReusedWithProxy(load_timing_result,
                                    CONNECT_TIMING_HAS_SSL_TIMES);
 }
 
 // Make sure that URLRequest correctly adjusts proxy times when they're before
 // |request_start|, due to already having a connected socket.  This happens in
 // the case of reusing a SPDY session.  The connected socket is not considered
 // reused in this test (May be a preconnect).
 //
 // To mix things up from the test above, assumes DNS times but no SSL times.
-TEST_F(URLRequestTest, InterceptLoadTimingEarlyProxyResolution) {
+TEST_F(URLRequestInterceptorTest, InterceptLoadTimingEarlyProxyResolution) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing =
       NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_DNS_TIMES, true);
   job_load_timing.proxy_resolve_start = now - base::TimeDelta::FromDays(6);
   job_load_timing.proxy_resolve_end = now - base::TimeDelta::FromDays(5);
   job_load_timing.connect_timing.dns_start = now - base::TimeDelta::FromDays(4);
   job_load_timing.connect_timing.dns_end = now - base::TimeDelta::FromDays(3);
   job_load_timing.connect_timing.connect_start =
       now - base::TimeDelta::FromDays(2);
   job_load_timing.connect_timing.connect_end =
       now - base::TimeDelta::FromDays(1);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Proxy times, connect times, and DNS times should all be replaced with
   // request_start.
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.proxy_resolve_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.proxy_resolve_end);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.dns_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.dns_end);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.connect_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.connect_end);
 
   // Other times should have been left null.
   TestLoadTimingNotReusedWithProxy(load_timing_result,
                                    CONNECT_TIMING_HAS_DNS_TIMES);
 }
 
 // Same as above, but in the reused case.
-TEST_F(URLRequestTest, InterceptLoadTimingEarlyProxyResolutionReused) {
+TEST_F(URLRequestInterceptorTest,
+       InterceptLoadTimingEarlyProxyResolutionReused) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing = NormalLoadTimingInfoReused(now, true);
   job_load_timing.proxy_resolve_start = now - base::TimeDelta::FromDays(4);
   job_load_timing.proxy_resolve_end = now - base::TimeDelta::FromDays(3);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Proxy times and connect times should all be replaced with request_start.
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.proxy_resolve_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.proxy_resolve_end);
 
   // Other times should have been left null.
   TestLoadTimingReusedWithProxy(load_timing_result);
 }
 
 // Make sure that URLRequest correctly adjusts connect times when they're before
 // |request_start|, due to reusing a connected socket.  The connected socket is
 // not considered reused in this test (May be a preconnect).
 //
 // To mix things up, the request has SSL times, but no DNS times.
-TEST_F(URLRequestTest, InterceptLoadTimingEarlyConnect) {
+TEST_F(URLRequestInterceptorTest, InterceptLoadTimingEarlyConnect) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing =
       NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_SSL_TIMES, false);
   job_load_timing.connect_timing.connect_start =
       now - base::TimeDelta::FromDays(1);
   job_load_timing.connect_timing.ssl_start = now - base::TimeDelta::FromDays(2);
   job_load_timing.connect_timing.ssl_end = now - base::TimeDelta::FromDays(3);
   job_load_timing.connect_timing.connect_end =
       now - base::TimeDelta::FromDays(4);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Connect times, and SSL times should be replaced with request_start.
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.connect_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.ssl_start);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.ssl_end);
   EXPECT_EQ(load_timing_result.request_start,
             load_timing_result.connect_timing.connect_end);
 
   // Other times should have been left null.
   TestLoadTimingNotReused(load_timing_result, CONNECT_TIMING_HAS_SSL_TIMES);
 }
 
 // Make sure that URLRequest correctly adjusts connect times when they're before
 // |request_start|, due to reusing a connected socket in the case that there
 // are also proxy times.  The connected socket is not considered reused in this
 // test (May be a preconnect).
 //
 // In this test, there are no SSL or DNS times.
-TEST_F(URLRequestTest, InterceptLoadTimingEarlyConnectWithProxy) {
+TEST_F(URLRequestInterceptorTest, InterceptLoadTimingEarlyConnectWithProxy) {
   base::TimeTicks now = base::TimeTicks::Now();
   LoadTimingInfo job_load_timing =
       NormalLoadTimingInfo(now, CONNECT_TIMING_HAS_CONNECT_TIMES_ONLY, true);
   job_load_timing.connect_timing.connect_start =
       now - base::TimeDelta::FromDays(1);
   job_load_timing.connect_timing.connect_end =
       now - base::TimeDelta::FromDays(2);
 
   LoadTimingInfo load_timing_result =
-      RunLoadTimingTest(job_load_timing, &default_context_);
+      RunURLRequestInterceptorLoadTimingTest(
+          job_load_timing, &default_context_, interceptor_);
 
   // Connect times should be replaced with proxy_resolve_end.
   EXPECT_EQ(load_timing_result.proxy_resolve_end,
             load_timing_result.connect_timing.connect_start);
   EXPECT_EQ(load_timing_result.proxy_resolve_end,
             load_timing_result.connect_timing.connect_end);
 
   // Other times should have been left null.
   TestLoadTimingNotReusedWithProxy(load_timing_result,
                                    CONNECT_TIMING_HAS_CONNECT_TIMES_ONLY);
@@ skipping 4602 matching lines @@
   scoped_ptr<URLRequest> req(context.CreateRequest(
       GURL("http://127.0.0.1/"), DEFAULT_PRIORITY, &d, NULL));
   req->Start();
   base::RunLoop().Run();
 
   EXPECT_TRUE(d.request_failed());
   EXPECT_EQ(URLRequestStatus::FAILED, req->status().status());
   EXPECT_EQ(ERR_NETWORK_IO_SUSPENDED, req->status().error());
 }
 
+class URLRequestInterceptorTestHTTP : public URLRequestInterceptorTest {
+ public:
+  URLRequestInterceptorTestHTTP()
+      : test_server_(base::FilePath(FILE_PATH_LITERAL(
+                         "net/data/url_request_unittest"))) {
+  }
+
+ protected:

[mmenke, 2014/11/05 18:43:44]

nit:  Protected not needed.

[bengr, 2014/11/05 22:51:37]

Done.

+  // Requests |redirect_url|, which must return a HTTP 3xx redirect.
+  // |request_method| is the method to use for the initial request.
+  // |redirect_method| is the method that is expected to be used for the second
+  // request, after redirection.
+  // If |include_data| is true, data is uploaded with the request.  The
+  // response body is expected to match it exactly, if and only if
+  // |request_method| == |redirect_method|.
+  void HTTPRedirectMethodTest(const GURL& redirect_url,
+                              const std::string& request_method,
+                              const std::string& redirect_method,
+                              bool include_data) {
+    static const char kData[] = "hello world";
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+        redirect_url, DEFAULT_PRIORITY, &d, NULL));
+    req->set_method(request_method);
+    if (include_data) {
+      req->set_upload(CreateSimpleUploadData(kData));
+      HttpRequestHeaders headers;
+      headers.SetHeader(HttpRequestHeaders::kContentLength,
+                        base::UintToString(arraysize(kData) - 1));
+      req->SetExtraRequestHeaders(headers);
+    }
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(redirect_method, req->method());
+    EXPECT_EQ(URLRequestStatus::SUCCESS, req->status().status());
+    EXPECT_EQ(OK, req->status().error());
+    if (include_data) {
+      if (request_method == redirect_method) {
+        EXPECT_EQ(kData, d.data_received());
+      } else {
+        EXPECT_NE(kData, d.data_received());
+      }
+    }
+    if (HasFailure())
+      LOG(WARNING) << "Request method was: " << request_method;
+  }
+
+  void HTTPUploadDataOperationTest(const std::string& method) {
+    const int kMsgSize = 20000;  // multiple of 10
+    const int kIterations = 50;
+    char* uploadBytes = new char[kMsgSize+1];
+    char* ptr = uploadBytes;
+    char marker = 'a';
+    for (int idx = 0; idx < kMsgSize/10; idx++) {
+      memcpy(ptr, "----------", 10);
+      ptr += 10;
+      if (idx % 100 == 0) {
+        ptr--;
+        *ptr++ = marker;
+        if (++marker > 'z')
+          marker = 'a';
+      }
+    }
+    uploadBytes[kMsgSize] = '\0';
+
+    for (int i = 0; i < kIterations; ++i) {
+      TestDelegate d;
+      scoped_ptr<URLRequest> r(default_context_.CreateRequest(
+          test_server_.GetURL("echo"), DEFAULT_PRIORITY, &d, NULL));
+      r->set_method(method.c_str());
+
+      r->set_upload(CreateSimpleUploadData(uploadBytes));
+
+      r->Start();
+      EXPECT_TRUE(r->is_pending());
+
+      base::RunLoop().Run();
+
+      ASSERT_EQ(1, d.response_started_count())
+          << "request failed: " << r->status().status()
+          << ", os error: " << r->status().error();
+
+      EXPECT_FALSE(d.received_data_before_response());
+      EXPECT_EQ(uploadBytes, d.data_received());
+    }
+    delete[] uploadBytes;
+  }
+
+  void AddChunksToUpload(URLRequest* r) {
+    r->AppendChunkToUpload("a", 1, false);
+    r->AppendChunkToUpload("bcd", 3, false);
+    r->AppendChunkToUpload("this is a longer chunk than before.", 35, false);
+    r->AppendChunkToUpload("\r\n\r\n", 4, false);
+    r->AppendChunkToUpload("0", 1, false);
+    r->AppendChunkToUpload("2323", 4, true);
+  }
+
+  void VerifyReceivedDataMatchesChunks(URLRequest* r, TestDelegate* d) {
+    // This should match the chunks sent by AddChunksToUpload().
+    const std::string expected_data =
+        "abcdthis is a longer chunk than before.\r\n\r\n02323";
+
+    ASSERT_EQ(1, d->response_started_count())
+        << "request failed: " << r->status().status()
+        << ", os error: " << r->status().error();
+
+    EXPECT_FALSE(d->received_data_before_response());
+
+    EXPECT_EQ(expected_data.size(), static_cast<size_t>(d->bytes_received()));
+    EXPECT_EQ(expected_data, d->data_received());
+  }
+
+  bool DoManyCookiesRequest(int num_cookies) {
+    TestDelegate d;
+    scoped_ptr<URLRequest> r(default_context_.CreateRequest(
+        test_server_.GetURL("set-many-cookies?" +
+                                base::IntToString(num_cookies)),
+        DEFAULT_PRIORITY, &d, NULL));
+
+    r->Start();
+    EXPECT_TRUE(r->is_pending());
+
+    base::RunLoop().Run();
+
+    bool is_success = r->status().is_success();
+
+    if (!is_success) {
+      EXPECT_TRUE(r->status().error() == ERR_RESPONSE_HEADERS_TOO_BIG);

[mmenke, 2014/11/05 18:43:44]

EXPECT_EQ(ERR_..., ...);

[bengr, 2014/11/05 22:51:37]

Done.

+      // The test server appears to be unable to handle subsequent requests
+      // after this error is triggered. Force it to restart.
+      EXPECT_TRUE(test_server_.Stop());
+      EXPECT_TRUE(test_server_.Start());
+    }
+
+    return is_success;
+  }
+
+  LocalHttpTestServer test_server_;

[mmenke, 2014/11/05 18:43:44]

Per Google style guide, class variables should all be private.  Can just add an
accessor.

[bengr, 2014/11/05 22:51:39]

Done.

+};
+
+TEST_F(URLRequestInterceptorTestHTTP,
+       NetworkDelegateNotificationOnRedirectIntercept) {
+  interceptor_->intercept_redirect_ = true;
+  interceptor_->redirect_headers_ =  MockURLRequestInterceptor::ok_headers();
+  interceptor_->redirect_data_ = MockURLRequestInterceptor::ok_data();
+
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      test_server_.GetURL("files/redirect-test.html"), DEFAULT_PRIORITY,
+      &d, NULL));
+  req->Start();
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(interceptor_->did_intercept_redirect_);
+  // Check we got one good response
+  EXPECT_TRUE(req->status().is_success());
+  if (req->status().is_success()) {
+    EXPECT_EQ(200, req->response_headers()->response_code());
+  }
+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+
+  EXPECT_EQ(1, default_network_delegate_.created_requests());
+  EXPECT_EQ(1, default_network_delegate_.before_send_headers_count());
+  EXPECT_EQ(1, default_network_delegate_.headers_received_count());
+}
+
+TEST_F(URLRequestInterceptorTestHTTP,
+       NetworkDelegateNotificationOnErrorIntercept) {
+  // Intercept that error and respond with an OK response.
+  interceptor_->intercept_final_response_ = true;
+  interceptor_->final_headers_ = MockURLRequestInterceptor::ok_headers();
+  interceptor_->final_data_ = MockURLRequestInterceptor::ok_data();
+  default_network_delegate_.set_can_be_intercepted_on_error(true);
+
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      test_server_.GetURL("files/two-content-lengths.html"), DEFAULT_PRIORITY,
+      &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(interceptor_->did_intercept_final_);
+
+  // Check we received one good response.
+  EXPECT_TRUE(req->status().is_success());
+  EXPECT_EQ(200, req->response_headers()->response_code());
+  EXPECT_EQ(MockURLRequestInterceptor::ok_data(), d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+
+  EXPECT_EQ(1, default_network_delegate_.created_requests());
+  EXPECT_EQ(1, default_network_delegate_.before_send_headers_count());
+  EXPECT_EQ(0, default_network_delegate_.headers_received_count());
+}
+
+TEST_F(URLRequestInterceptorTestHTTP,
+       NetworkDelegateNotificationOnResponseIntercept) {
+  // Intercept that error and respond with an OK response.
+  interceptor_->intercept_final_response_ = true;
+
+  // Intercept with a real URLRequestHttpJob.
+  interceptor_->use_url_request_http_job_ = true;
+
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
+      test_server_.GetURL("files/simple.html"), DEFAULT_PRIORITY,
+      &d, NULL));
+  req->set_method("GET");
+  req->Start();
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(interceptor_->did_intercept_final_);
+
+  // Check we received one good response.
+  EXPECT_TRUE(req->status().is_success());
+  EXPECT_EQ(200, req->response_headers()->response_code());
+  EXPECT_EQ("hello", d.data_received());
+  EXPECT_EQ(1, d.response_started_count());
+  EXPECT_EQ(0, d.received_redirect_count());
+
+  EXPECT_EQ(1, default_network_delegate_.created_requests());
+  EXPECT_EQ(2, default_network_delegate_.before_send_headers_count());
+  EXPECT_EQ(2, default_network_delegate_.headers_received_count());
+}
+
+

[mmenke, 2014/11/05 18:43:44]

nit:  Remove extra line break

[bengr, 2014/11/05 22:51:38]

Done.

 class HTTPSRequestTest : public testing::Test {
  public:
   HTTPSRequestTest() : default_context_(true) {
     default_context_.set_network_delegate(&default_network_delegate_);
     default_context_.Init();
   }
   ~HTTPSRequestTest() override {}
 
  protected:
   TestNetworkDelegate default_network_delegate_;  // Must outlive URLRequest.
@@ skipping 1828 matching lines @@
 
     EXPECT_FALSE(r->is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net