Componentize proxy code from chrome/browser/net

components/proxy_config/pref_proxy_config_tracker_impl.cc

Index: components/proxy_config/pref_proxy_config_tracker_impl.cc
diff --git a/chrome/browser/net/pref_proxy_config_tracker_impl.cc b/components/proxy_config/pref_proxy_config_tracker_impl.cc
similarity index 84%
rename from chrome/browser/net/pref_proxy_config_tracker_impl.cc
rename to components/proxy_config/pref_proxy_config_tracker_impl.cc
index 6de119ea7f9ca9223fe181e13166b5775ee8dceb..2544795ed33585391f1c965fd62d231a822310f1 100644
--- a/chrome/browser/net/pref_proxy_config_tracker_impl.cc
+++ b/components/proxy_config/pref_proxy_config_tracker_impl.cc
@@ -2,25 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/net/pref_proxy_config_tracker_impl.h"
+#include "components/proxy_config/pref_proxy_config_tracker_impl.h"
 
 #include "base/bind.h"
+#include "base/location.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/proxy_config/proxy_config_dictionary.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
+#include "components/proxy_config/proxy_config_pref_names.h"
 #include "net/proxy/proxy_list.h"
 #include "net/proxy/proxy_server.h"
-
-using content::BrowserThread;
+#include "url/gurl.h"
 
 namespace {
 
@@ -86,34 +83,33 @@ void RemoveGooglezipDataReductionProxies(
 
 }  // namespace
 
-//============================= ChromeProxyConfigService =======================
+//============================= ProxyConfigServiceImpl =======================
 
-ChromeProxyConfigService::ChromeProxyConfigService(
+ProxyConfigServiceImpl::ProxyConfigServiceImpl(
     net::ProxyConfigService* base_service)
     : base_service_(base_service),
       pref_config_state_(ProxyPrefs::CONFIG_UNSET),
       pref_config_read_pending_(true),
-      registered_observer_(false) {
-}
+      registered_observer_(false) {}
 
-ChromeProxyConfigService::~ChromeProxyConfigService() {
+ProxyConfigServiceImpl::~ProxyConfigServiceImpl() {
   if (registered_observer_ && base_service_.get())
     base_service_->RemoveObserver(this);
 }
 
-void ChromeProxyConfigService::AddObserver(
+void ProxyConfigServiceImpl::AddObserver(
     net::ProxyConfigService::Observer* observer) {
   RegisterObserver();
   observers_.AddObserver(observer);
 }
 
-void ChromeProxyConfigService::RemoveObserver(
+void ProxyConfigServiceImpl::RemoveObserver(
     net::ProxyConfigService::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 net::ProxyConfigService::ConfigAvailability
-    ChromeProxyConfigService::GetLatestProxyConfig(net::ProxyConfig* config) {
+ProxyConfigServiceImpl::GetLatestProxyConfig(net::ProxyConfig* config) {
   RegisterObserver();
 
   if (pref_config_read_pending_)
@@ -128,21 +124,19 @@ net::ProxyConfigService::ConfigAvailability
 
   ProxyPrefs::ConfigState config_state;
   return PrefProxyConfigTrackerImpl::GetEffectiveProxyConfig(
-      pref_config_state_, pref_config_,
-      system_availability, system_config, false,
-      &config_state, config);
+      pref_config_state_, pref_config_, system_availability, system_config,
+      false, &config_state, config);
 }
 
-void ChromeProxyConfigService::OnLazyPoll() {
+void ProxyConfigServiceImpl::OnLazyPoll() {
   if (base_service_.get())
     base_service_->OnLazyPoll();
 }
 
-void ChromeProxyConfigService::UpdateProxyConfig(
+void ProxyConfigServiceImpl::UpdateProxyConfig(
     ProxyPrefs::ConfigState config_state,
     const net::ProxyConfig& config) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
+  DCHECK(thread_checker_.CalledOnValidThread());

[Abhishek, 2015/08/26 09:48:59]

Tests are crashing at this place.

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...

Do I need to remove this CHECK?
Or
Need to convert in io_task_runner?

[droger, 2015/08/26 10:11:22]

It looks like it happens because the object is created on the UI thread, and
thus the thread_checker_ is attached to the UI thread, not the IO thread.

If that works, I would suggest calling
thread_checker_.DetachFromThread() in the ProxyConfigServiceImpl contstructor,
with a comment:

  // ProxyConfigServiceImpl is created on the UI thread, but used on the network
  // thread.
  thread_checker_.DetachFromThread();


Another option would be to pass the io_task_runner_ to ProxyConfigServiceImpl
and then do the checks using io_task_runner_BelongsToCurrentThread().

[Abhishek, 2015/08/26 13:44:36]

I've changed it as suggested here.

   pref_config_read_pending_ = false;
   pref_config_state_ = config_state;
   pref_config_ = config;
@@ -166,10 +160,10 @@ void ChromeProxyConfigService::UpdateProxyConfig(
   }
 }
 
-void ChromeProxyConfigService::OnProxyConfigChanged(
+void ProxyConfigServiceImpl::OnProxyConfigChanged(
     const net::ProxyConfig& config,
     ConfigAvailability availability) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Check whether there is a proxy configuration defined by preferences. In
   // this case that proxy configuration takes precedence and the change event
@@ -182,8 +176,8 @@ void ChromeProxyConfigService::OnProxyConfigChanged(
   }
 }
 
-void ChromeProxyConfigService::RegisterObserver() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void ProxyConfigServiceImpl::RegisterObserver() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!registered_observer_ && base_service_.get()) {
     base_service_->AddObserver(this);
     registered_observer_ = true;
@@ -193,13 +187,15 @@ void ChromeProxyConfigService::RegisterObserver() {
 //========================= PrefProxyConfigTrackerImpl =========================
 
 PrefProxyConfigTrackerImpl::PrefProxyConfigTrackerImpl(
-    PrefService* pref_service)
+    PrefService* pref_service,
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
     : pref_service_(pref_service),
-      chrome_proxy_config_service_(NULL),
-      update_pending_(true) {
+      proxy_config_service_impl_(NULL),
+      update_pending_(true),
+      io_task_runner_(io_task_runner) {
   config_state_ = ReadPrefConfig(pref_service_, &pref_config_);
   proxy_prefs_.Init(pref_service);
-  proxy_prefs_.Add(prefs::kProxy,
+  proxy_prefs_.Add(ProxyPrefs::prefs::kProxy,
                    base::Bind(&PrefProxyConfigTrackerImpl::OnProxyPrefChanged,
                               base::Unretained(this)));
 }
@@ -211,22 +207,22 @@ PrefProxyConfigTrackerImpl::~PrefProxyConfigTrackerImpl() {
 scoped_ptr<net::ProxyConfigService>
 PrefProxyConfigTrackerImpl::CreateTrackingProxyConfigService(
     scoped_ptr<net::ProxyConfigService> base_service) {
-  chrome_proxy_config_service_ =
-      new ChromeProxyConfigService(base_service.release());
+  proxy_config_service_impl_ =
+      new ProxyConfigServiceImpl(base_service.release());
   VLOG(1) << this << ": set chrome proxy config service to "
-          << chrome_proxy_config_service_;
-  if (chrome_proxy_config_service_ && update_pending_)
+          << proxy_config_service_impl_;
+  if (proxy_config_service_impl_ && update_pending_)
     OnProxyConfigChanged(config_state_, pref_config_);
 
-  return scoped_ptr<net::ProxyConfigService>(chrome_proxy_config_service_);
+  return scoped_ptr<net::ProxyConfigService>(proxy_config_service_impl_);
 }
 
 void PrefProxyConfigTrackerImpl::DetachFromPrefService() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Stop notifications.
   proxy_prefs_.RemoveAll();
   pref_service_ = NULL;
-  chrome_proxy_config_service_ = NULL;
+  proxy_config_service_impl_ = NULL;
 }
 
 // static
@@ -284,7 +280,7 @@ net::ProxyConfigService::ConfigAvailability
 void PrefProxyConfigTrackerImpl::RegisterPrefs(PrefRegistrySimple* registry) {
   base::DictionaryValue* default_settings =
       ProxyConfigDictionary::CreateSystem();
-  registry->RegisterDictionaryPref(prefs::kProxy, default_settings);
+  registry->RegisterDictionaryPref(ProxyPrefs::prefs::kProxy, default_settings);
 }
 
 // static
@@ -292,25 +288,24 @@ void PrefProxyConfigTrackerImpl::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* pref_service) {
   base::DictionaryValue* default_settings =
       ProxyConfigDictionary::CreateSystem();
-  pref_service->RegisterDictionaryPref(prefs::kProxy, default_settings);
+  pref_service->RegisterDictionaryPref(ProxyPrefs::prefs::kProxy,
+                                       default_settings);
 }
 
 // static
 ProxyPrefs::ConfigState PrefProxyConfigTrackerImpl::ReadPrefConfig(
     const PrefService* pref_service,
     net::ProxyConfig* config) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
   // Clear the configuration and source.
   *config = net::ProxyConfig();
   ProxyPrefs::ConfigState config_state = ProxyPrefs::CONFIG_UNSET;
 
   const PrefService::Preference* pref =
-      pref_service->FindPreference(prefs::kProxy);
+      pref_service->FindPreference(ProxyPrefs::prefs::kProxy);
   DCHECK(pref);
 
   const base::DictionaryValue* dict =
-      pref_service->GetDictionary(prefs::kProxy);
+      pref_service->GetDictionary(ProxyPrefs::prefs::kProxy);
   DCHECK(dict);
   ProxyConfigDictionary proxy_dict(dict);
 
@@ -322,7 +317,7 @@ ProxyPrefs::ConfigState PrefProxyConfigTrackerImpl::ReadPrefConfig(
         config_state = ProxyPrefs::CONFIG_EXTENSION;
       else
         config_state = ProxyPrefs::CONFIG_OTHER_PRECEDE;
-    } else  {
+    } else {
       config_state = ProxyPrefs::CONFIG_FALLBACK;
     }
   }
@@ -332,7 +327,7 @@ ProxyPrefs::ConfigState PrefProxyConfigTrackerImpl::ReadPrefConfig(
 
 ProxyPrefs::ConfigState PrefProxyConfigTrackerImpl::GetProxyConfig(
     net::ProxyConfig* config) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (config_state_ != ProxyPrefs::CONFIG_UNSET)
     *config = pref_config_;
   return config_state_;
@@ -341,16 +336,15 @@ ProxyPrefs::ConfigState PrefProxyConfigTrackerImpl::GetProxyConfig(
 void PrefProxyConfigTrackerImpl::OnProxyConfigChanged(
     ProxyPrefs::ConfigState config_state,
     const net::ProxyConfig& config) {
-  if (!chrome_proxy_config_service_) {
+  if (!proxy_config_service_impl_) {
     VLOG(1) << "No chrome proxy config service to push to UpdateProxyConfig";
     update_pending_ = true;
     return;
   }
-  update_pending_ = !BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&ChromeProxyConfigService::UpdateProxyConfig,
-                 base::Unretained(chrome_proxy_config_service_),
-                 config_state, config));
+  update_pending_ = !io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&ProxyConfigServiceImpl::UpdateProxyConfig,
+                            base::Unretained(proxy_config_service_impl_),
+                            config_state, config));
   VLOG(1) << this << (update_pending_ ? ": Error" : ": Done")
           << " pushing proxy to UpdateProxyConfig";
 }
@@ -417,10 +411,10 @@ bool PrefProxyConfigTrackerImpl::PrefConfigToNetConfig(
 }
 
 void PrefProxyConfigTrackerImpl::OnProxyPrefChanged() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(thread_checker_.CalledOnValidThread());
   net::ProxyConfig new_config;
-  ProxyPrefs::ConfigState config_state = ReadPrefConfig(pref_service_,
-                                                        &new_config);
+  ProxyPrefs::ConfigState config_state =
+      ReadPrefConfig(pref_service_, &new_config);
   if (config_state_ != config_state ||
       (config_state_ != ProxyPrefs::CONFIG_UNSET &&
        !pref_config_.Equals(new_config))) {