Make CrOS proxy configuration write prefs directly.

chrome/browser/chromeos/proxy_config_service_impl.cc

Index: chrome/browser/chromeos/proxy_config_service_impl.cc
diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc
index b5c0d6c5d4cf0ffdbd6c1566015d246335348943..fefcc77121e57c0e17ef9d9c8d513cb0cf031837 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -9,9 +9,13 @@
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/task.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/json_value_serializer.h"
+#include "chrome/common/pref_names.h"
 
 namespace chromeos {
 
@@ -181,40 +185,55 @@ bool ProxyConfigServiceImpl::ProxyConfig::ManualProxy::Decode(
 
 //----------- ProxyConfigServiceImpl::ProxyConfig: public methods --------------
 
-void ProxyConfigServiceImpl::ProxyConfig::ToNetProxyConfig(
-    net::ProxyConfig* net_config) {
+// static
+void ProxyConfigServiceImpl::ProxyConfig::FormatProxySpec(
+    std::string* spec,
+    const std::string& scheme,
+    const net::ProxyServer& server) {
+  DCHECK(spec);
+
+  if (!server.is_valid())
+    return;
+
+  if (!spec->empty())
+    *spec += ";";
+  if (!scheme.empty()) {
+    *spec += scheme;
+    *spec += "=";
+  }
+  *spec += server.ToURI();
+}
+
+DictionaryValue*
+    ProxyConfigServiceImpl::ProxyConfig::ToProxyConfigDictionary() const {
+  std::string proxy_spec;
+
   switch (mode) {
     case MODE_DIRECT:
-      *net_config = net::ProxyConfig::CreateDirect();
-      break;
+      return ProxyConfigDictionary::CreateDirect();
     case MODE_AUTO_DETECT:
-      *net_config = net::ProxyConfig::CreateAutoDetect();
-      break;
+      return ProxyConfigDictionary::CreateAutoDetect();
     case MODE_PAC_SCRIPT:
-      *net_config = net::ProxyConfig::CreateFromCustomPacURL(
-          automatic_proxy.pac_url);
-      break;
+      return ProxyConfigDictionary::CreatePacScript(
+          automatic_proxy.pac_url.spec());
     case MODE_SINGLE_PROXY:
-      *net_config = net::ProxyConfig();
-      net_config->proxy_rules().type =
-             net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY;
-      net_config->proxy_rules().single_proxy = single_proxy.server;
-      net_config->proxy_rules().bypass_rules = bypass_rules;
-      break;
+      FormatProxySpec(&proxy_spec, "", single_proxy.server);
+      return ProxyConfigDictionary::CreateFixedServers(proxy_spec,
+                                                       bypass_rules.ToString());
     case MODE_PROXY_PER_SCHEME:
-      *net_config = net::ProxyConfig();
-      net_config->proxy_rules().type =
-          net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME;
-      net_config->proxy_rules().proxy_for_http = http_proxy.server;
-      net_config->proxy_rules().proxy_for_https = https_proxy.server;
-      net_config->proxy_rules().proxy_for_ftp = ftp_proxy.server;
-      net_config->proxy_rules().fallback_proxy = socks_proxy.server;
-      net_config->proxy_rules().bypass_rules = bypass_rules;
-      break;
+      proxy_spec.reserve(256);
+      FormatProxySpec(&proxy_spec, "http", http_proxy.server);
+      FormatProxySpec(&proxy_spec, "https", https_proxy.server);
+      FormatProxySpec(&proxy_spec, "ftp", ftp_proxy.server);
+      FormatProxySpec(&proxy_spec, "socks", socks_proxy.server);
+      return ProxyConfigDictionary::CreateFixedServers(proxy_spec,
+                                                       bypass_rules.ToString());
     default:
       NOTREACHED() << "Unrecognized proxy config mode";
       break;
   }
+
+  return NULL;
 }
 
 bool ProxyConfigServiceImpl::ProxyConfig::CanBeWrittenByUser(
@@ -389,11 +408,14 @@ bool ProxyConfigServiceImpl::ProxyConfig::DecodeManualProxy(
 
 //------------------- ProxyConfigServiceImpl: public methods -------------------
 
-ProxyConfigServiceImpl::ProxyConfigServiceImpl()
+ProxyConfigServiceImpl::ProxyConfigServiceImpl(PrefService* local_state)
     : can_post_task_(false),
       has_config_(false),
       persist_to_device_(true),
-      persist_to_device_pending_(false) {
+      persist_to_device_pending_(false),
+      local_state_(local_state) {
+  DCHECK(local_state_);
+
   // Start async fetch of proxy config from settings persisted on device.
   // TODO(kuan): retrieve config from policy and owner and merge them
   bool use_default = true;
@@ -409,49 +431,43 @@ ProxyConfigServiceImpl::ProxyConfigServiceImpl()
     }
   }
   if (use_default)
-    InitConfigToDefault(false);
+    InitConfigToDefault();
   can_post_task_ = true;
 }
 
-ProxyConfigServiceImpl::ProxyConfigServiceImpl(const ProxyConfig& init_config)
+ProxyConfigServiceImpl::ProxyConfigServiceImpl(PrefService* local_state,
+                                               const ProxyConfig& init_config)
     : can_post_task_(true),
       has_config_(true),
       persist_to_device_(false),
-      persist_to_device_pending_(false) {
+      persist_to_device_pending_(false),
+      local_state_(local_state) {
+  DCHECK(local_state_);
   reference_config_ = init_config;
-  // Update the IO-accessible copy in |cached_config_| as well.
-  cached_config_ = reference_config_;
+  OnUISetProxyConfig(false);
 }
 
 ProxyConfigServiceImpl::~ProxyConfigServiceImpl() {
 }
 
 void ProxyConfigServiceImpl::UIGetProxyConfig(ProxyConfig* config) {

[battre, 2011/02/22 10:32:40]

Any reason for removing the checks? The method is public.

[Mattias Nissler (ping if slow), 2011/02/22 11:08:02]

The class doesn't claim to be thread safe any longer, so the default applies:
you cannot expected it to be thread safe. Classes that are not purpose-build for
the Chrome threading infrastructure should be assumed to be non-thread safe and
usable on any (single) thread you want them on, so the checks don't make sense
any longer.

-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   // Simply returns the copy on the UI thread.
   *config = reference_config_;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToDirect() {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_DIRECT;
   OnUISetProxyConfig(persist_to_device_);
   return true;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToAutoDetect() {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_AUTO_DETECT;
   OnUISetProxyConfig(persist_to_device_);
   return true;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToPACScript(const GURL& pac_url) {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_PAC_SCRIPT;
   reference_config_.automatic_proxy.pac_url = pac_url;
   OnUISetProxyConfig(persist_to_device_);
@@ -460,8 +476,6 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToPACScript(const GURL& pac_url) {
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToSingleProxy(
     const net::ProxyServer& server) {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_SINGLE_PROXY;
   reference_config_.single_proxy.server = server;
   OnUISetProxyConfig(persist_to_device_);
@@ -470,8 +484,6 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToSingleProxy(
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToProxyPerScheme(
     const std::string& scheme, const net::ProxyServer& server) {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   ProxyConfig::ManualProxy* proxy = reference_config_.MapSchemeToProxy(scheme);
   if (!proxy) {
     NOTREACHED() << "Cannot set proxy: invalid scheme [" << scheme << "]";
@@ -485,8 +497,6 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToProxyPerScheme(
 
 bool ProxyConfigServiceImpl::UISetProxyConfigBypassRules(
     const net::ProxyBypassRules& bypass_rules) {
-  // Should be called from UI thread.
-  CheckCurrentlyOnUIThread();
   DCHECK(reference_config_.mode == ProxyConfig::MODE_SINGLE_PROXY ||
          reference_config_.mode == ProxyConfig::MODE_PROXY_PER_SCHEME);
   if (reference_config_.mode != ProxyConfig::MODE_SINGLE_PROXY &&
@@ -500,31 +510,6 @@ bool ProxyConfigServiceImpl::UISetProxyConfigBypassRules(
   return true;
 }
 
-void ProxyConfigServiceImpl::AddObserver(
-    net::ProxyConfigService::Observer* observer) {
-  // Should be called from IO thread.
-  CheckCurrentlyOnIOThread();
-  observers_.AddObserver(observer);
-}
-
-void ProxyConfigServiceImpl::RemoveObserver(
-    net::ProxyConfigService::Observer* observer) {
-  // Should be called from IO thread.
-  CheckCurrentlyOnIOThread();
-  observers_.RemoveObserver(observer);
-}
-
-bool ProxyConfigServiceImpl::IOGetProxyConfig(net::ProxyConfig* net_config) {
-  // Should be called from IO thread.
-  CheckCurrentlyOnIOThread();
-  if (has_config_) {
-    // Simply return the last cached proxy configuration.
-    cached_config_.ToNetProxyConfig(net_config);
-    return true;
-  }
-  return false;
-}
-
 void ProxyConfigServiceImpl::OnSettingsOpCompleted(
     SignedSettings::ReturnCode code,
     bool value) {
@@ -546,28 +531,22 @@ void ProxyConfigServiceImpl::OnSettingsOpCompleted(
       OnUISetProxyConfig(false);
     } else {
       LOG(WARNING) << "Error deserializing device's proxy setting";
-      InitConfigToDefault(true);
+      InitConfigToDefault();
     }
   } else {
     LOG(WARNING) << "Error retrieving proxy setting from device";
-    InitConfigToDefault(true);
+    InitConfigToDefault();
   }
   retrieve_property_op_ = NULL;
 }
 
 //------------------ ProxyConfigServiceImpl: private methods -------------------
 
-void ProxyConfigServiceImpl::InitConfigToDefault(bool post_to_io_thread) {
+void ProxyConfigServiceImpl::InitConfigToDefault() {
   VLOG(1) << "Using default proxy config: auto-detect";
   reference_config_.mode = ProxyConfig::MODE_AUTO_DETECT;
   reference_config_.automatic_proxy.source = ProxyConfig::SOURCE_OWNER;
-  if (post_to_io_thread && can_post_task_) {
-    OnUISetProxyConfig(false);
-  } else {
-    // Update the IO-accessible copy in |cached_config_| as well.
-    cached_config_ = reference_config_;
-    has_config_ = true;
-  }
+  OnUISetProxyConfig(false);
 }
 
 void ProxyConfigServiceImpl::PersistConfigToDevice() {
@@ -585,14 +564,12 @@ void ProxyConfigServiceImpl::PersistConfigToDevice() {
 }
 
 void ProxyConfigServiceImpl::OnUISetProxyConfig(bool persist_to_device) {
-  // Posts a task to IO thread with the new config, so it can update
-  // |cached_config_|.
-  Task* task = NewRunnableMethod(this,
-      &ProxyConfigServiceImpl::IOSetProxyConfig, reference_config_);
-  if (!BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, task)) {
-    VLOG(1) << "Couldn't post task to IO thread to set new proxy config";
-    delete task;
-  }
+  scoped_ptr<DictionaryValue> proxy_dict(
+      reference_config_.ToProxyConfigDictionary());
+  if (proxy_dict.get())
+    local_state_->Set(prefs::kProxy, *proxy_dict);
+  else
+    local_state_->ClearPref(prefs::kProxy);
 
   if (persist_to_device && CrosLibrary::Get()->EnsureLoaded()) {
     if (store_property_op_) {
@@ -604,25 +581,4 @@ void ProxyConfigServiceImpl::OnUISetProxyConfig(bool persist_to_device) {
   }
 }
 
-void ProxyConfigServiceImpl::CheckCurrentlyOnIOThread() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-}
-
-void ProxyConfigServiceImpl::CheckCurrentlyOnUIThread() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-}
-
-void ProxyConfigServiceImpl::IOSetProxyConfig(const ProxyConfig& new_config) {
-  // This is called on the IO thread (posted from UI thread).
-  CheckCurrentlyOnIOThread();
-  VLOG(1) << "Proxy configuration changed";
-  has_config_ = true;
-  cached_config_ = new_config;
-  // Notify observers of new proxy config.
-  net::ProxyConfig net_config;
-  cached_config_.ToNetProxyConfig(&net_config);
-  FOR_EACH_OBSERVER(net::ProxyConfigService::Observer, observers_,
-                    OnProxyConfigChanged(net_config));
-}
-
 }  // namespace chromeos