Data Reduction Proxy class ownership updates and Settings cleanup

components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
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 573a235e2e0e2a5c6f32070690a2dee0799a3168..b4ebbeed53d937f995b0826785da57658b994735 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
@@ -11,6 +11,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h"
+#include "components/data_reduction_proxy/core/browser/data_saver_service.h"
 #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"
@@ -47,47 +48,70 @@ void RecordNetworkChangeEvent(DataReductionProxyNetworkChangeEvent event) {
 namespace data_reduction_proxy {
 
 DataReductionProxyConfig::DataReductionProxyConfig(
-    scoped_ptr<DataReductionProxyParams> params)
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+    net::NetLog* net_log,
+    scoped_ptr<DataReductionProxyParams> params,
+    DataReductionProxyConfigurator* configurator,
+    DataReductionProxyEventStore* event_store)
     : restricted_by_carrier_(false),
       disabled_on_vpn_(false),
       unreachable_(false),
       enabled_by_user_(false),
       alternative_enabled_by_user_(false),
-      net_log_(nullptr),
-      url_request_context_getter_(nullptr),
-      configurator_(nullptr),
-      event_store_(nullptr) {
-  params_.reset(params.release());
+      params_(params.release()),
+      io_task_runner_(io_task_runner),
+      ui_task_runner_(ui_task_runner),
+      net_log_(net_log),
+      configurator_(configurator),
+      event_store_(event_store) {
+  DCHECK(io_task_runner);
+  DCHECK(ui_task_runner);
+  DCHECK(configurator);
+  DCHECK(event_store);
+  InitOnIOThread();
 }
 
 DataReductionProxyConfig::~DataReductionProxyConfig() {
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
 }
 
-void DataReductionProxyConfig::InitDataReductionProxyConfig(
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
-    net::NetLog* net_log,
-    net::URLRequestContextGetter* url_request_context_getter,
-    DataReductionProxyConfigurator* configurator,
-    DataReductionProxyEventStore* event_store) {
-  DCHECK(io_task_runner);
-  DCHECK(configurator);
-  DCHECK(event_store);
-  DCHECK(io_task_runner->BelongsToCurrentThread());
-  io_task_runner_ = io_task_runner;
-  net_log_ = net_log;
-  url_request_context_getter_ = url_request_context_getter;
-  configurator_ = configurator;
-  event_store_ = event_store;
-  net::NetworkChangeNotifier::AddIPAddressObserver(this);
+void DataReductionProxyConfig::SetDataSaverService(
+    base::WeakPtr<DataSaverService> data_saver_service) {
+  data_saver_service_ = data_saver_service;
+}
+
+void DataReductionProxyConfig::SetProxyPrefs(bool enabled,
+                                             bool alternative_enabled,
+                                             bool at_startup) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&DataReductionProxyConfig::SetProxyPrefsOnIOThread,
+                            base::Unretained(this), enabled,
+                            alternative_enabled, at_startup));
+}
+
+void DataReductionProxyConfig::SetProxyPrefsOnIOThread(bool enabled,

[bengr, 2015/02/17 18:10:24]

What does this have to do with prefs?

[jeremyim, 2015/02/19 21:03:44]

Done.

+                                                       bool alternative_enabled,
+                                                       bool at_startup) {
+  enabled_by_user_ = enabled;
+  alternative_enabled_by_user_ = alternative_enabled;
+  SetProxyConfigs(enabled_by_user_, alternative_enabled_by_user_,
+                  restricted_by_carrier_, at_startup);
+
+  // Check if the proxy has been restricted explicitly by the carrier.
+  if (enabled &&
+      !(alternative_enabled && !params()->alternative_fallback_allowed())) {
+    StartProbe();
+  }
 }
 
 void DataReductionProxyConfig::SetProxyConfigs(bool enabled,
                                                bool alternative_enabled,
                                                bool restricted,
                                                bool at_startup) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
   DCHECK(configurator_);
-
   LogProxyState(enabled, restricted, at_startup);
   // The alternative is only configured if the standard configuration is
   // is enabled.
@@ -126,10 +150,15 @@ void DataReductionProxyConfig::LogProxyState(bool enabled,
                << (at_startup ? kAtStartup : kByUser);
 }
 
-void DataReductionProxyConfig::OnURLFetchComplete(
-    const net::URLFetcher* source) {
-  DCHECK(source == fetcher_.get());
-  net::URLRequestStatus status = source->GetStatus();
+void DataReductionProxyConfig::HandleProbeResponse(
+    const std::string& response,
+    const net::URLRequestStatus& status) {
+  if (!io_task_runner_->BelongsToCurrentThread()) {
+    io_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&DataReductionProxyConfig::HandleProbeResponse,
+                              base::Unretained(this), response, status));
+    return;
+  }
 
   if (event_store_) {
     event_store_->EndCanaryRequest(bound_net_log_, status.error());
@@ -147,9 +176,6 @@ void DataReductionProxyConfig::OnURLFetchComplete(
                                 std::abs(status.error()));
   }
 
-  std::string response;
-  source->GetResponseAsString(&response);
-
   if ("OK" == response.substr(0, 2)) {
     DVLOG(1) << "The data reduction proxy is unrestricted.";
 
@@ -185,6 +211,7 @@ void DataReductionProxyConfig::OnURLFetchComplete(
 }
 
 void DataReductionProxyConfig::OnIPAddressChanged() {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
   if (enabled_by_user_) {
     DCHECK(params()->allowed());
     RecordNetworkChangeEvent(IP_CHANGED);
@@ -194,10 +221,25 @@ void DataReductionProxyConfig::OnIPAddressChanged() {
         !params()->alternative_fallback_allowed()) {
       return;
     }
-    ProbeWhetherDataReductionProxyIsAvailable();
+    StartProbe();
   }
 }
 
+void DataReductionProxyConfig::InitOnIOThread() {
+  if (!io_task_runner_->BelongsToCurrentThread()) {
+    io_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&DataReductionProxyConfig::InitOnIOThread,
+                              base::Unretained(this)));
+    return;
+  }
+
+  if (!params_->allowed())
+    return;
+
+  AddDefaultProxyBypassRules();
+  net::NetworkChangeNotifier::AddIPAddressObserver(this);
+}
+
 void DataReductionProxyConfig::AddDefaultProxyBypassRules() {
   // localhost
   DCHECK(configurator_);
@@ -230,33 +272,26 @@ void DataReductionProxyConfig::RecordProbeURLFetchResult(
                             PROBE_URL_FETCH_RESULT_COUNT);
 }
 
-net::URLFetcher* DataReductionProxyConfig::GetURLFetcherForProbe() {
-  net::URLFetcher* fetcher =
-      net::URLFetcher::Create(params_->probe_url(), net::URLFetcher::GET, this);
-  fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE | net::LOAD_BYPASS_PROXY);
-  DCHECK(url_request_context_getter_);
-  fetcher->SetRequestContext(url_request_context_getter_);
-  // Configure max retries to be at most kMaxRetries times for 5xx errors.
-  static const int kMaxRetries = 5;
-  fetcher->SetMaxRetriesOn5xx(kMaxRetries);
-  fetcher->SetAutomaticallyRetryOnNetworkChanges(kMaxRetries);
-  return fetcher;
-}
-
-void DataReductionProxyConfig::ProbeWhetherDataReductionProxyIsAvailable() {
-  net::URLFetcher* fetcher = GetURLFetcherForProbe();
-  if (!fetcher)
+void DataReductionProxyConfig::StartProbe() {
+  if (!ui_task_runner_->BelongsToCurrentThread()) {

[bengr, 2015/02/17 18:10:24]

This construct bugs me. Why do we not know which thread this will come in on?

[jeremyim, 2015/02/19 21:03:44]

Done.

+    ui_task_runner_->PostTask(FROM_HERE,
+                              base::Bind(&DataReductionProxyConfig::StartProbe,
+                                         base::Unretained(this)));
     return;
-  fetcher_.reset(fetcher);
+  }
 
   bound_net_log_ = net::BoundNetLog::Make(
       net_log_, net::NetLog::SOURCE_DATA_REDUCTION_PROXY);
-  if (event_store_) {
-    event_store_->BeginCanaryRequest(bound_net_log_,
-                                     fetcher_->GetOriginalURL());
-  }
+  if (data_saver_service_) {
+    if (event_store_) {
+      event_store_->BeginCanaryRequest(bound_net_log_, params_->probe_url());
+    }
 
-  fetcher_->Start();
+    data_saver_service_->CheckProbeURL(
+        params_->probe_url(),
+        base::Bind(&DataReductionProxyConfig::HandleProbeResponse,
+                   base::Unretained(this)));
+  }
 }
 
 void DataReductionProxyConfig::GetNetworkList(