Use a custom URLRequestContextGetter for performing the secure proxy check.

BUG=478610

Review URL: https://codereview.chromium.org/1066513005

Cr-Commit-Position: refs/heads/master@{#326138}

7 files changed, 103 insertions, 48 deletions

diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
index 6e9f2a5..c884740 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
@@ -15,7 +15,6 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "net/base/load_flags.h"
-#include "net/http/http_network_layer.h"
 #include "net/proxy/proxy_server.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
@@ -53,30 +52,10 @@ namespace data_reduction_proxy {
 // Checks if the secure proxy is allowed by the carrier by sending a probe.
 class SecureProxyChecker : public net::URLFetcherDelegate {
  public:
-  SecureProxyChecker(net::URLRequestContext* url_request_context,
-                     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
-      : io_task_runner_(io_task_runner) {
-    DCHECK(io_task_runner_->BelongsToCurrentThread());
-
-    url_request_context_.reset(new net::URLRequestContext());
-    url_request_context_->CopyFrom(url_request_context);
-
-    net::HttpNetworkSession::Params params_modified =
-        *(url_request_context_->GetNetworkSessionParams());
-    params_modified.enable_quic = false;
-    params_modified.next_protos = net::NextProtosWithSpdyAndQuic(false, false);
-
-    http_network_layer_.reset(new net::HttpNetworkLayer(
-        new net::HttpNetworkSession(params_modified)));
-    url_request_context_->set_http_transaction_factory(
-        http_network_layer_.get());
-
-    url_request_context_getter_ = new net::TrivialURLRequestContextGetter(
-        url_request_context_.get(), io_task_runner_);
-  }
+  SecureProxyChecker(net::URLRequestContextGetter* url_request_context_getter)
+      : url_request_context_getter_(url_request_context_getter) {}
 
   void OnURLFetchComplete(const net::URLFetcher* source) override {
-    DCHECK(io_task_runner_->BelongsToCurrentThread());
     DCHECK_EQ(source, fetcher_.get());
     net::URLRequestStatus status = source->GetStatus();
 
@@ -88,7 +67,6 @@ class SecureProxyChecker : public net::URLFetcherDelegate {
 
   void CheckIfSecureProxyIsAllowed(const GURL& secure_proxy_check_url,
                                    FetcherResponseCallback fetcher_callback) {
-    DCHECK(io_task_runner_->BelongsToCurrentThread());
     fetcher_.reset(net::URLFetcher::Create(secure_proxy_check_url,
                                            net::URLFetcher::GET, this));
     fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE | net::LOAD_BYPASS_PROXY);
@@ -111,11 +89,7 @@ class SecureProxyChecker : public net::URLFetcherDelegate {
   ~SecureProxyChecker() override {}
 
  private:
-  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
-
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
-  scoped_ptr<net::URLRequestContext> url_request_context_;
-  scoped_ptr<net::HttpNetworkLayer> http_network_layer_;
 
   // The URLFetcher being used for the secure proxy check.
   scoped_ptr<net::URLFetcher> fetcher_;
@@ -515,9 +489,8 @@ void DataReductionProxyConfig::SecureProxyCheck(
   }
 
   if (!secure_proxy_checker_) {
-    DCHECK(url_request_context_getter_->GetURLRequestContext());
-    secure_proxy_checker_.reset(new SecureProxyChecker(
-        url_request_context_getter_->GetURLRequestContext(), io_task_runner_));
+    secure_proxy_checker_.reset(
+        new SecureProxyChecker(url_request_context_getter_));
   }
   secure_proxy_checker_->CheckIfSecureProxyIsAllowed(secure_proxy_check_url,
                                                      fetcher_callback);
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
index 9034bae..5e0c3f8 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/prefs/pref_member.h"
 #include "base/single_thread_task_runner.h"
@@ -25,22 +26,82 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "net/log/net_log.h"
+#include "net/url_request/http_user_agent_settings.h"
+#include "net/url_request/static_http_user_agent_settings.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_builder.h"
+#include "net/url_request/url_request_context_getter.h"
 
 namespace data_reduction_proxy {
 
+// A |net::URLRequestContextGetter| which uses only vanilla HTTP/HTTPS for
+// performing requests. This is used by the secure proxy check to prevent the
+// use of SPDY and QUIC which may be used by the primary request contexts.
+class BasicHTTPURLRequestContextGetter : public net::URLRequestContextGetter {
+ public:
+  BasicHTTPURLRequestContextGetter(
+      const std::string& user_agent,
+      const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner);
+
+  // Overridden from net::URLRequestContextGetter:
+  net::URLRequestContext* GetURLRequestContext() override;
+  scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
+      const override;
+
+ private:
+  ~BasicHTTPURLRequestContextGetter() override;
+
+  scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
+  scoped_ptr<net::HttpUserAgentSettings> user_agent_settings_;
+  scoped_ptr<net::URLRequestContext> url_request_context_;
+
+  DISALLOW_COPY_AND_ASSIGN(BasicHTTPURLRequestContextGetter);
+};
+
+BasicHTTPURLRequestContextGetter::BasicHTTPURLRequestContextGetter(
+    const std::string& user_agent,
+    const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner)
+    : network_task_runner_(network_task_runner),
+      user_agent_settings_(
+          new net::StaticHttpUserAgentSettings(std::string(), user_agent)) {
+}
+
+net::URLRequestContext*
+BasicHTTPURLRequestContextGetter::GetURLRequestContext() {
+  if (!url_request_context_) {
+    net::URLRequestContextBuilder builder;
+    builder.set_proxy_service(net::ProxyService::CreateDirect());
+    builder.SetSpdyAndQuicEnabled(false, false);
+    url_request_context_.reset(builder.Build());
+  }
+
+  return url_request_context_.get();
+}
+
+scoped_refptr<base::SingleThreadTaskRunner>
+BasicHTTPURLRequestContextGetter::GetNetworkTaskRunner() const {
+  return network_task_runner_;
+}
+
+BasicHTTPURLRequestContextGetter::~BasicHTTPURLRequestContextGetter() {
+}
+
 DataReductionProxyIOData::DataReductionProxyIOData(
     const Client& client,
     int param_flags,
     net::NetLog* net_log,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
-    bool enable_quic)
+    bool enable_quic,
+    const std::string& user_agent)
     : client_(client),
       net_log_(net_log),
       io_task_runner_(io_task_runner),
       ui_task_runner_(ui_task_runner),
       shutdown_on_ui_(false),
       url_request_context_getter_(nullptr),
+      basic_url_request_context_getter_(
+          new BasicHTTPURLRequestContextGetter(user_agent, io_task_runner)),
       weak_factory_(this) {
   DCHECK(net_log);
   DCHECK(io_task_runner_);
@@ -124,7 +185,7 @@ void DataReductionProxyIOData::SetDataReductionProxyService(
 
 void DataReductionProxyIOData::InitializeOnIOThread() {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
-  config_->InitializeOnIOThread(url_request_context_getter_);
+  config_->InitializeOnIOThread(basic_url_request_context_getter_.get());
   if (config_client_.get())
     config_client_->RetrieveConfig();
   ui_task_runner_->PostTask(
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
index 7e45c16..c5b13ef 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_IO_DATA_H_
 #define COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_IO_DATA_H_
 
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/prefs/pref_member.h"
@@ -42,7 +44,8 @@ class DataReductionProxyIOData {
       net::NetLog* net_log,
       scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
-      bool enable_quic);
+      bool enable_quic,
+      const std::string& user_agent);
 
   virtual ~DataReductionProxyIOData();
 
@@ -131,6 +134,7 @@ class DataReductionProxyIOData {
 
  private:
   friend class TestDataReductionProxyIOData;
+  FRIEND_TEST_ALL_PREFIXES(DataReductionProxyIODataTest, TestConstruction);
 
   // Used for testing.
   DataReductionProxyIOData();
@@ -192,6 +196,10 @@ class DataReductionProxyIOData {
   // The net::URLRequestContextGetter used for making URL requests.
   net::URLRequestContextGetter* url_request_context_getter_;
 
+  // A net::URLRequestContextGetter used for making secure proxy checks. It
+  // does not use alternate protocols.
+  scoped_refptr<net::URLRequestContextGetter> basic_url_request_context_getter_;
+
   base::WeakPtrFactory<DataReductionProxyIOData> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyIOData);
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc
index c9c92e1..4caf215 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data_unittest.cc
@@ -14,7 +14,11 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
+#include "net/http/http_network_session.h"
 #include "net/log/net_log.h"
+#include "net/socket/next_proto.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_interceptor.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -90,7 +94,22 @@ class DataReductionProxyIODataTest : public testing::Test {
 TEST_F(DataReductionProxyIODataTest, TestConstruction) {
   scoped_ptr<DataReductionProxyIOData> io_data(new DataReductionProxyIOData(
       Client::UNKNOWN, DataReductionProxyParams::kAllowed, net_log(),
-      message_loop_proxy(), message_loop_proxy(), false /* enable_quic */));
+      message_loop_proxy(), message_loop_proxy(), false /* enable_quic */,
+      std::string() /* user_agent */));
+
+  // Check that the SimpleURLRequestContextGetter uses vanilla HTTP.
+  net::URLRequestContext* request_context =
+      io_data->basic_url_request_context_getter_.get()->GetURLRequestContext();
+  const net::HttpNetworkSession::Params* http_params =
+      request_context->GetNetworkSessionParams();
+  EXPECT_TRUE(http_params->use_alternate_protocols);
+  EXPECT_FALSE(http_params->enable_quic);
+  net::NextProtoVector expected_protos =
+      net::NextProtosWithSpdyAndQuic(false, false);
+  EXPECT_EQ(expected_protos.size(), http_params->next_protos.size());
+  size_t proto_index = 0;
+  for (const auto& proto : expected_protos)
+    EXPECT_EQ(proto, http_params->next_protos[proto_index++]);
 
   // Check that io_data creates an interceptor. Such an interceptor is
   // thoroughly tested by DataReductionProxyInterceptoTest.
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
index 2b1a17b..dce4f0a 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
@@ -185,18 +185,6 @@ TEST(DataReductionProxySettingsStandaloneTest, TestEndToEndSecureProxyCheck) {
             .SkipSettingsInitialization()
             .Build();
 
-    // Enabling QUIC should have no effect since secure proxy should not
-    // use QUIC. If secure proxy check incorrectly uses QUIC, the tests will
-    // fail because Mock sockets do not speak QUIC.
-    scoped_ptr<net::HttpNetworkSession::Params> params(
-        new net::HttpNetworkSession::Params());
-    params->use_alternate_protocols = true;
-    params->enable_quic = true;
-    params->origin_to_force_quic_on = net::HostPortPair::FromString(
-        TestDataReductionProxyParams::DefaultSecureProxyCheckURL());
-
-    context.set_http_network_session_params(params.Pass());
-
     context.set_net_log(drp_test_context->net_log());
     net::MockClientSocketFactory mock_socket_factory;
     context.set_client_socket_factory(&mock_socket_factory);
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
index b3c9c4a..1966107 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
@@ -370,6 +370,7 @@ DataReductionProxyTestContext::Builder::Build() {
           task_runner, config.Pass(), event_store.Pass(),
           request_options.Pass(), configurator.Pass(), config_client.Pass()));
   io_data->InitOnUIThread(pref_service.get());
+  io_data->SetSimpleURLRequestContextGetter(request_context_getter);
 
   scoped_ptr<DataReductionProxyTestContext> test_context(
       new DataReductionProxyTestContext(
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
index b1d29d2..653c5c8 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
@@ -22,6 +22,7 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.h"
 #include "net/base/backoff_entry.h"
 #include "net/log/test_net_log.h"
+#include "net/url_request/url_request_context_getter.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 class TestingPrefServiceSimple;
@@ -34,7 +35,6 @@ namespace net {
 class MockClientSocketFactory;
 class NetLog;
 class URLRequestContext;
-class URLRequestContextGetter;
 class URLRequestContextStorage;
 }
 
@@ -169,6 +169,11 @@ class TestDataReductionProxyIOData : public DataReductionProxyIOData {
     return config_client_.get();
   }
 
+  void SetSimpleURLRequestContextGetter(
+      const scoped_refptr<net::URLRequestContextGetter> context_getter) {
+    basic_url_request_context_getter_ = context_getter;
+  }
+
   base::WeakPtr<DataReductionProxyIOData> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }