Add data reduction functionality to all platforms.

chrome/browser/io_thread.cc

Index: chrome/browser/io_thread.cc
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 97b602f6c5c6083af810b42fa349aa8e241198bf..188afc5d7cd7d24036d7b6b73607c179831f88fe 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -39,7 +39,11 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
+#include "components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h"
+#include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_prefs.h"
+#include "components/data_reduction_proxy/browser/data_reduction_proxy_protocol.h"
+#include "components/data_reduction_proxy/browser/data_reduction_proxy_settings.h"
 #include "components/policy/core/common/policy_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
@@ -94,13 +98,6 @@
 #include "net/ocsp/nss_ocsp.h"
 #endif
 
-#if defined(OS_ANDROID) || defined(OS_IOS)
-#include "components/data_reduction_proxy/browser/data_reduction_proxy_auth_request_handler.h"
-#include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
-#include "components/data_reduction_proxy/browser/data_reduction_proxy_protocol.h"
-#include "components/data_reduction_proxy/browser/data_reduction_proxy_settings.h"
-#endif
-
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/login/users/user_manager.h"
 #include "chrome/browser/chromeos/net/cert_verify_proc_chromeos.h"
@@ -108,13 +105,10 @@
 #endif
 
 using content::BrowserThread;
-
-#if defined(OS_ANDROID) || defined(OS_IOS)
 using data_reduction_proxy::DataReductionProxyAuthRequestHandler;
 using data_reduction_proxy::DataReductionProxyParams;
 using data_reduction_proxy::DataReductionProxyUsageStats;
 using data_reduction_proxy::DataReductionProxySettings;
-#endif
 
 class SafeBrowsingURLRequestContext;
 
@@ -545,6 +539,38 @@ void IOThread::Init() {
   // the UI thread
 }
 
+void IOThread::SetupDataReductionProxy(
+    ChromeNetworkDelegate* network_delegate) {
+  int drp_flags = 0;

[Ryan Sleevi, 2014/07/22 01:56:30]

drop the drp_. Don't abbreviate like that, but this isn't necessary.

[Not at Google. Contact bengr, 2014/07/22 23:03:52]

Done.

+  // TODO(kundaji): Gate instantitation of all data reduction classes based
+  // on this method.
+  if (DataReductionProxyParams::IsIncludedInFieldTrial()) {

[Ryan Sleevi, 2014/07/23 00:54:44]

Procedurally, this has the effect of enabling the feature for all users, which
is not something we do prior to the approval of the bug (
http://crbug.com/375484 ). Even if you were going to do a finch experiment, you
wouldn't be landing this code until the necessary teams have signed off, which
doesn't seem to be the case.

See also the comments in DataCompressionProxyBaseFieldTrial, which is setup in
such a way that some users will be opted-in for this.

The solution to this is that this code needs to remain entirely disabled for all
users, and a separate CL dealing with field trials and enabling the feature -
even for a small population - need to be blocked on getting approvals and
reviews.

[Not at Google. Contact bengr, 2014/08/26 17:31:08]

Removed trial code. Launching for all users.

+    drp_flags = DataReductionProxyParams::kAllowed |
+        DataReductionProxyParams::kFallbackAllowed;
+    if (DataReductionProxyParams::IsIncludedInAlternativeFieldTrial())
+      drp_flags |= DataReductionProxyParams::kAlternativeAllowed;
+    if (DataReductionProxyParams::IsIncludedInPromoFieldTrial())
+      drp_flags |= DataReductionProxyParams::kPromoAllowed;
+  }
+  DataReductionProxyParams* proxy_params =
+      new DataReductionProxyParams(drp_flags);
+  globals_->data_reduction_proxy_params.reset(proxy_params);
+  globals_->data_reduction_proxy_auth_request_handler.reset(
+      new DataReductionProxyAuthRequestHandler(proxy_params));
+  globals_->on_resolve_proxy_handler =
+      ChromeNetworkDelegate::OnResolveProxyHandler(
+          base::Bind(data_reduction_proxy::OnResolveProxyHandler));

[Ryan Sleevi, 2014/07/22 08:22:24]

As mentioned in past discussions, there are concerns about whether or not the
ChromeNetworkDelegate is the right layer to do this hooking. The interaction
with extensions, among other things, which you don't have to worry about on
mobile.

[bengr, 2014/07/22 18:26:55]

What past discussions are you referring to? I had many, many meetings with
OWNERS of net/, particularly with pauljensen and mmenke, and this was the
approach we arrived at. Everyone knew that desktop platforms were on the table.

Note that I tried implementing this as a set of classes that wrapped
UrlRequestHttpJob etc. to add our functionality, but this required bigger
changes to the net stack interfaces, seemed error prone, and required far, far
more glue code. 

+  DataReductionProxyUsageStats* proxy_usage_stats =
+      new DataReductionProxyUsageStats(proxy_params,
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+  network_delegate->set_data_reduction_proxy_params(proxy_params);
+  globals_->data_reduction_proxy_usage_stats.reset(proxy_usage_stats);
+  network_delegate->set_data_reduction_proxy_usage_stats(proxy_usage_stats);
+  network_delegate->set_data_reduction_proxy_auth_request_handler(
+      globals_->data_reduction_proxy_auth_request_handler.get());
+}
+
 void IOThread::InitAsync() {
   TRACE_EVENT0("startup", "IOThread::InitAsync");
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -638,36 +664,9 @@ void IOThread::InitAsync() {
   }
 #endif
   globals_->ssl_config_service = GetSSLConfigService();
-#if defined(OS_ANDROID) || defined(OS_IOS)
-#if defined(SPDY_PROXY_AUTH_ORIGIN)
-  int drp_flags = DataReductionProxyParams::kFallbackAllowed;
-  if (DataReductionProxyParams::IsIncludedInFieldTrial())
-    drp_flags |= DataReductionProxyParams::kAllowed;
-  if (DataReductionProxyParams::IsIncludedInAlternativeFieldTrial())
-    drp_flags |= DataReductionProxyParams::kAlternativeAllowed;
-  if (DataReductionProxyParams::IsIncludedInPromoFieldTrial())
-    drp_flags |= DataReductionProxyParams::kPromoAllowed;
-  DataReductionProxyParams* proxy_params =
-      new DataReductionProxyParams(drp_flags);
-  globals_->data_reduction_proxy_params.reset(proxy_params);
-  globals_->data_reduction_proxy_auth_request_handler.reset(
-      new DataReductionProxyAuthRequestHandler(proxy_params));
-  globals_->on_resolve_proxy_handler =
-      ChromeNetworkDelegate::OnResolveProxyHandler(
-          base::Bind(data_reduction_proxy::OnResolveProxyHandler));
-  DataReductionProxyUsageStats* proxy_usage_stats =
-      new DataReductionProxyUsageStats(proxy_params,
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
-  network_delegate->set_data_reduction_proxy_params(proxy_params);
-  globals_->data_reduction_proxy_usage_stats.reset(proxy_usage_stats);
-  network_delegate->set_data_reduction_proxy_usage_stats(proxy_usage_stats);
-  network_delegate->set_data_reduction_proxy_auth_request_handler(
-      globals_->data_reduction_proxy_auth_request_handler.get());
+  SetupDataReductionProxy(network_delegate);
   network_delegate->set_on_resolve_proxy_handler(
       globals_->on_resolve_proxy_handler);
-#endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
-#endif  // defined(OS_ANDROID) || defined(OS_IOS)
   globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
       globals_->host_resolver.get()));
   globals_->http_server_properties.reset(new net::HttpServerPropertiesImpl());
@@ -1104,6 +1103,13 @@ void IOThread::InitSystemRequestContextOnIOThread() {
           system_proxy_config_service_.release(),
           command_line,
           quick_check_enabled_.GetValue()));
+  DCHECK(globals_->data_reduction_proxy_params);
+
+  // This is temporary until the histogram code is factored out of
+  // Proxy{Server|Service}.

[Ryan Sleevi, 2014/07/22 01:56:30]

Based on the team, I have trouble believing this.

At a minimum, file a bug, and note the bug, and set a Release-Block label with
an M-Stone target. Refactoring/cleanup needs to be prioritized.

[bengr, 2014/07/22 20:08:20]

Ryan, factoring the data reduction proxy out of net is something we are
committed to. I also don't like this code being here. 

Rohit, when you file the bug, put M-38 on it. I don't think this is a release
blocker, but I do agree that we should schedule the work and get it done asap.

[Not at Google. Contact bengr, 2014/07/22 23:03:52]

Done. Filed bug 396222.

[Not at Google. Contact bengr, 2014/08/26 17:31:08]

Fixed https://code.google.com/p/chromium/issues/detail?id=396699

+  globals_->system_proxy_service->SetDataReductionProxyOrigins(
+      globals_->data_reduction_proxy_params->origin(),
+      globals_->data_reduction_proxy_params->fallback_origin());
 
   net::HttpNetworkSession::Params system_params;
   InitializeNetworkSessionParams(&system_params);