Set trusted SPDY proxy dynamically on per-profile basis

chrome/browser/profiles/profile_io_data.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 "chrome/browser/profiles/profile_io_data.h"
 
 #include <stddef.h>
-#include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 22 matching lines @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/about_handler/about_protocol_handler.h"
 #include "components/content_settings/core/browser/content_settings_provider.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/cookie_config/cookie_store_util.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/net_log/chrome_net_log.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "components/sync_driver/pref_names.h"
 #include "components/url_formatter/url_fixer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/resource_context.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/keygen_handler.h"
 #include "net/base/network_quality_estimator.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/multi_log_ct_verifier.h"
 #include "net/cookies/canonical_cookie.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/http/http_util.h"
 #include "net/http/transport_security_persister.h"
 #include "net/proxy/proxy_config_service_fixed.h"
 #include "net/proxy/proxy_script_fetcher_impl.h"
+#include "net/proxy/proxy_server.h"
 #include "net/proxy/proxy_service.h"
 #include "net/ssl/channel_id_service.h"
 #include "net/ssl/client_cert_store.h"
 #include "net/url_request/certificate_report_sender.h"
 #include "net/url_request/data_protocol_handler.h"
 #include "net/url_request/file_protocol_handler.h"
 #include "net/url_request/ftp_protocol_handler.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
@@ skipping 69 matching lines @@
 #if defined(OS_MACOSX)
 #include "net/ssl/client_cert_store_mac.h"
 #endif
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::ResourceContext;
 
 namespace {
 
+const char kTrustedSpdyProxyFieldTrialName[] = "TrustedSpdyProxy";
+
 net::CertVerifier* g_cert_verifier_for_testing = nullptr;
 
 #if defined(DEBUG_DEVTOOLS)
 bool IsSupportedDevToolsURL(const GURL& url, base::FilePath* path) {
   std::string bundled_path_prefix(chrome::kChromeUIDevToolsBundledPath);
   bundled_path_prefix = "/" + bundled_path_prefix + "/";
 
   if (!url.SchemeIs(content::kChromeDevToolsScheme) ||
       url.host() != chrome::kChromeUIDevToolsHost ||
       !base::StartsWith(url.path(), bundled_path_prefix,
@@ skipping 1109 matching lines @@
   net::URLRequestContext* context = main_request_context();
 
   IOThread* const io_thread = profile_params.io_thread;
 
   io_thread->InitializeNetworkSessionParams(&params);
   net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(context,
                                                                  &params);
   if (!IsOffTheRecord()) {
     params.socket_performance_watcher_factory =
         io_thread->globals()->network_quality_estimator.get();
+
+    // Set trusted SPDY proxy if it is not set through command line switch, data

[bengr, 2015/12/29 17:46:00]

Set -> Set the
through -> through the
data -> the data
group of -> group of the

[tbansal1, 2015/12/29 23:34:21]

Done.

+    // reduction proxy is enabled by the user, and the session is part of the
+    // enabled group of kTrustedSpdyProxyFieldTrialName field trial.
+    if (params.trusted_spdy_proxy.empty() && IsDataReductionProxyEnabled() &&

[bengr, 2015/12/29 17:46:00]

I would move this block to a helper method.

[tbansal1, 2015/12/29 23:34:21]

Obsolete, since this is a single line change now.

+        base::FieldTrialList::FindFullName(kTrustedSpdyProxyFieldTrialName)
+                .find("Enabled") == 0) {
+      if (data_reduction_proxy_io_data()->config()) {

[bengr, 2015/12/29 17:46:00]

Why not do the following so that a lot of the business logic moves into DRP
code:

if (!params.trusted_spdy_proxy.empty() ||
    !IsDataReductionProxyEnabled()) {
  return;
}

DataReductionProxyConfig* config =
    data_reduction_proxy_io_data->config();

if (!config || !config->IsTrustedSpdyProxyFieldTrialEnabled())
  return;

config->MaybeSetTrustedSpdyProxy(&params.trusted_spdy_proxy);

[tbansal1, 2015/12/29 23:34:21]

Done.

+        // proxies_for_http contains the data reduction proxies for HTTP
+        // requests.
+        const std::vector<net::ProxyServer> proxies_for_http =
+            data_reduction_proxy_io_data()->config()->proxies_for_http();
+        // Make sure that the proxy is a secure proxy before setting it as a
+        // trusted SPDY proxy.
+        if (proxies_for_http.size() >= 1 && proxies_for_http[0].is_valid() &&
+            (proxies_for_http[0].is_https() || proxies_for_http[0].is_quic())) {
+          params.trusted_spdy_proxy =
+              proxies_for_http[0].host_port_pair().ToString();
+        }
+      }
+    }

[bengr, 2015/12/29 17:46:00]

Once you rewrite and move to a helper method, please write tests for the helper
and for the method(s) you add to DRPConfig.

[tbansal1, 2015/12/29 23:34:21]

Added tests for methods in DRPConfig.

   }
   if (data_reduction_proxy_io_data_.get())
     params.proxy_delegate = data_reduction_proxy_io_data_->proxy_delegate();
 
   return scoped_ptr<net::HttpNetworkSession>(
       new net::HttpNetworkSession(params));
 }
 
 scoped_ptr<net::HttpCache> ProfileIOData::CreateMainHttpFactory(
     net::HttpNetworkSession* session,
@@ skipping 11 matching lines @@
       make_scoped_ptr(new DevToolsNetworkTransactionFactory(
           network_controller_handle_.GetController(), shared_session)),
       std::move(backend), true /* set_up_quic_server_info */));
 }
 
 void ProfileIOData::SetCookieSettingsForTesting(
     content_settings::CookieSettings* cookie_settings) {
   DCHECK(!cookie_settings_.get());
   cookie_settings_ = cookie_settings;
 }