Set trusted SPDY proxy dynamically on per-profile basis

chrome/browser/profiles/profile_io_data.cc

Index: chrome/browser/profiles/profile_io_data.cc
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index 5c854107903727ee32bf221053364e4ab5c3be17..0cd289f4af2bab665f97775985a26b1a9043c766 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/profiles/profile_io_data.h"
 
 #include <stddef.h>
-#include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ -16,6 +15,7 @@
 #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"
@@ -58,6 +58,7 @@
 #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"
@@ -69,6 +70,7 @@
 #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"
@@ -80,6 +82,7 @@
 #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"
@@ -169,6 +172,8 @@ using content::ResourceContext;
 
 namespace {
 
+const char kTrustedSpdyProxyFieldTrialName[] = "TrustedSpdyProxy";
+
 net::CertVerifier* g_cert_verifier_for_testing = nullptr;
 
 #if defined(DEBUG_DEVTOOLS)
@@ -1298,6 +1303,27 @@ scoped_ptr<net::HttpNetworkSession> ProfileIOData::CreateHttpNetworkSession(
   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();