Refactor proxy handling for ChromeOS to not go through the CrosSettings interface.

chrome/browser/chromeos/proxy_cros_settings_parser.cc

Index: chrome/browser/chromeos/proxy_cros_settings_parser.cc
diff --git a/chrome/browser/chromeos/proxy_cros_settings_provider.cc b/chrome/browser/chromeos/proxy_cros_settings_parser.cc
similarity index 73%
rename from chrome/browser/chromeos/proxy_cros_settings_provider.cc
rename to chrome/browser/chromeos/proxy_cros_settings_parser.cc
index 85a89a12b7e6d561cd2b936f7217808dbd35dfa4..3209cacc50a008c4e1bb2b3f9293e9d7ba698822 100644
--- a/chrome/browser/chromeos/proxy_cros_settings_provider.cc
+++ b/chrome/browser/chromeos/proxy_cros_settings_parser.cc
@@ -2,73 +2,85 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/proxy_cros_settings_provider.h"
+#include "chrome/browser/chromeos/proxy_cros_settings_parser.h"
 
 #include "base/string_util.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/cros_settings.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/browser_list.h"
 
 namespace chromeos {
 
-static const char kProxyPacUrl[]         = "cros.session.proxy.pacurl";
-static const char kProxySingleHttp[]     = "cros.session.proxy.singlehttp";
-static const char kProxySingleHttpPort[] = "cros.session.proxy.singlehttpport";
-static const char kProxyHttpUrl[]        = "cros.session.proxy.httpurl";
-static const char kProxyHttpPort[]       = "cros.session.proxy.httpport";
-static const char kProxyHttpsUrl[]       = "cros.session.proxy.httpsurl";
-static const char kProxyHttpsPort[]      = "cros.session.proxy.httpsport";
-static const char kProxyType[]           = "cros.session.proxy.type";
-static const char kProxySingle[]         = "cros.session.proxy.single";
-static const char kProxyFtpUrl[]         = "cros.session.proxy.ftpurl";
-static const char kProxyFtpPort[]        = "cros.session.proxy.ftpport";
-static const char kProxySocks[]          = "cros.session.proxy.socks";
-static const char kProxySocksPort[]      = "cros.session.proxy.socksport";
-static const char kProxyIgnoreList[]     = "cros.session.proxy.ignorelist";
+// Names of proxy preferences.
+const char kProxyPacUrl[]         = "cros.session.proxy.pacurl";
+const char kProxySingleHttp[]     = "cros.session.proxy.singlehttp";
+const char kProxySingleHttpPort[] = "cros.session.proxy.singlehttpport";
+const char kProxyHttpUrl[]        = "cros.session.proxy.httpurl";
+const char kProxyHttpPort[]       = "cros.session.proxy.httpport";
+const char kProxyHttpsUrl[]       = "cros.session.proxy.httpsurl";
+const char kProxyHttpsPort[]      = "cros.session.proxy.httpsport";
+const char kProxyType[]           = "cros.session.proxy.type";
+const char kProxySingle[]         = "cros.session.proxy.single";
+const char kProxyFtpUrl[]         = "cros.session.proxy.ftpurl";
+const char kProxyFtpPort[]        = "cros.session.proxy.ftpport";
+const char kProxySocks[]          = "cros.session.proxy.socks";
+const char kProxySocksPort[]      = "cros.session.proxy.socksport";
+const char kProxyIgnoreList[]     = "cros.session.proxy.ignorelist";
 
-static const char* const kProxySettings[] = {
-  kProxyPacUrl,
-  kProxySingleHttp,
-  kProxySingleHttpPort,
-  kProxyHttpUrl,
-  kProxyHttpPort,
-  kProxyHttpsUrl,
-  kProxyHttpsPort,
-  kProxyType,
-  kProxySingle,
-  kProxyFtpUrl,
-  kProxyFtpPort,
-  kProxySocks,
-  kProxySocksPort,
-  kProxyIgnoreList,
-};
+namespace {
 
-//------------------ ProxyCrosSettingsProvider: public methods -----------------
+base::Value* CreateServerHostValue(
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) {
+  return proxy.server.is_valid() ?
+         base::Value::CreateStringValue(proxy.server.host_port_pair().host()) :
+         NULL;
+}
+
+base::Value* CreateServerPortValue(
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) {
+  return proxy.server.is_valid() ?
+         base::Value::CreateIntegerValue(proxy.server.host_port_pair().port()) :
+         NULL;
+}
 
-ProxyCrosSettingsProvider::ProxyCrosSettingsProvider(Profile* profile)
-    : profile_(profile) {
+net::ProxyServer CreateProxyServerFromHost(
+    const std::string& host,
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+    net::ProxyServer::Scheme scheme) {
+  uint16 port = 0;
+  if (proxy.server.is_valid())
+    port = proxy.server.host_port_pair().port();
+  if (host.length() == 0 && port == 0)
+    return net::ProxyServer();
+  if (port == 0)
+    port = net::ProxyServer::GetDefaultPortForScheme(scheme);
+  net::HostPortPair host_port_pair(host, port);
+  return net::ProxyServer(scheme, host_port_pair);
 }
 
-void ProxyCrosSettingsProvider::SetCurrentNetwork(const std::string& network) {
-  GetConfigService()->UISetCurrentNetwork(network);
-  for (size_t i = 0; i < arraysize(kProxySettings); ++i)
-    CrosSettings::Get()->FireObservers(kProxySettings[i]);
+net::ProxyServer CreateProxyServerFromPort(
+    uint16 port,
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+    net::ProxyServer::Scheme scheme) {
+  std::string host;
+  if (proxy.server.is_valid())
+    host = proxy.server.host_port_pair().host();
+  if (host.length() == 0 && port == 0)
+    return net::ProxyServer();
+  net::HostPortPair host_port_pair(host, port);
+  return net::ProxyServer(scheme, host_port_pair);
 }
 
-void ProxyCrosSettingsProvider::MakeActiveNetworkCurrent() {
-  GetConfigService()->UIMakeActiveNetworkCurrent();
-  for (size_t i = 0; i < arraysize(kProxySettings); ++i)
-    CrosSettings::Get()->FireObservers(kProxySettings[i]);
 }

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

// namespace

[pastarmovj, 2011/11/09 17:51:53]

Done.

 
-void ProxyCrosSettingsProvider::DoSet(const std::string& path,
-                                      Value* in_value) {
+void ProxyCrosSettingsParser::SetProxyPrefValue(Profile* profile,

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

These large functions suck. I wonder whether we can just switch
ProxyConfigServiceImpl to hand out and accept a DictionaryValue representation
of proxy config? Kuan, WDYT? Probably not in this CL though.

[kuan, 2011/11/08 14:45:25]

somewhere some code has to do the parsing with multiple if's.  u're, in essence,
suggesting moving these large functions from here to ProxyConfigServiceImpl. 
personally, i prefer to separate ui, data and logic. these dictionaries are
really used only for UI to pass info from javascript to native code, whereas
ProxyConfigServiceImpl should just handle the so-called "backend" like using
structures.
however, if ur suggestion is the preferred approach by concensus, u're right
that it probably shldn't be in this cl.

[pastarmovj, 2011/11/09 17:51:53]

Ok keeping as is for now. Those functions are anyhow now two standalone
functions and pretty straightforward in their intent so we can keep them there
for a while.

+                                                const std::string& path,
+                                                const base::Value* in_value) {
   if (!in_value) {
+    NOTREACHED();
     return;
   }
 
-  chromeos::ProxyConfigServiceImpl* config_service = GetConfigService();
+  chromeos::ProxyConfigServiceImpl* config_service =
+      profile->GetProxyConfigTracker();
   // Retrieve proxy config.
   chromeos::ProxyConfigServiceImpl::ProxyConfig config;
   config_service->UIGetProxyConfig(&config);
@@ -211,7 +223,7 @@ void ProxyCrosSettingsProvider::DoSet(const std::string& path,
     }
   } else if (path == kProxyIgnoreList) {
     net::ProxyBypassRules bypass_rules;
-    if (in_value->GetType() == Value::TYPE_LIST) {
+    if (in_value->GetType() == base::Value::TYPE_LIST) {
       const ListValue* list_value = static_cast<const ListValue*>(in_value);
       for (size_t x = 0; x < list_value->GetSize(); x++) {
         std::string val;
@@ -224,13 +236,15 @@ void ProxyCrosSettingsProvider::DoSet(const std::string& path,
   }
 }
 
-bool ProxyCrosSettingsProvider::Get(const std::string& path,
-                                    Value** out_value) const {
+bool ProxyCrosSettingsParser::GetProxyPrefValue(Profile* profile,
+                                                const std::string& path,
+                                                base::Value** out_value) {
   bool found = false;
   bool managed = false;
   std::string controlled_by;
-  Value* data = NULL;
-  chromeos::ProxyConfigServiceImpl* config_service = GetConfigService();
+  base::Value* data = NULL;
+  chromeos::ProxyConfigServiceImpl* config_service =
+      profile->GetProxyConfigTracker();
   chromeos::ProxyConfigServiceImpl::ProxyConfig config;
   config_service->UIGetProxyConfig(&config);
 
@@ -239,7 +253,8 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
     if (config.mode ==
             chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_PAC_SCRIPT &&
         config.automatic_proxy.pac_url.is_valid()) {
-      data = Value::CreateStringValue(config.automatic_proxy.pac_url.spec());
+      data =
+          base::Value::CreateStringValue(config.automatic_proxy.pac_url.spec());
     }
     found = true;
   } else if (path == kProxySingleHttp) {
@@ -259,14 +274,14 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_AUTO_DETECT ||
         config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_PAC_SCRIPT) {
-      data = Value::CreateIntegerValue(3);
+      data = base::Value::CreateIntegerValue(3);
     } else if (config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_SINGLE_PROXY ||
         config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_PROXY_PER_SCHEME) {
-      data = Value::CreateIntegerValue(2);
+      data = base::Value::CreateIntegerValue(2);
     } else {
-      data = Value::CreateIntegerValue(1);
+      data = base::Value::CreateIntegerValue(1);
     }
     switch (config.state) {
        case ProxyPrefs::CONFIG_POLICY:
@@ -285,7 +300,7 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
     }
     found = true;
   } else if (path == kProxySingle) {
-    data = Value::CreateBooleanValue(config.mode ==
+    data = base::Value::CreateBooleanValue(config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_SINGLE_PROXY);
     found = true;
   } else if (path == kProxyFtpUrl) {
@@ -310,7 +325,7 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
     ListValue* list =  new ListValue();
     net::ProxyBypassRules::RuleList bypass_rules = config.bypass_rules.rules();
     for (size_t x = 0; x < bypass_rules.size(); x++) {
-      list->Append(Value::CreateStringValue(bypass_rules[x]->ToString()));
+      list->Append(base::Value::CreateStringValue(bypass_rules[x]->ToString()));
     }
     *out_value = list;
     return true;
@@ -318,7 +333,7 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
   if (found) {
     DictionaryValue* dict = new DictionaryValue;
     if (!data)
-      data = Value::CreateStringValue("");
+      data = base::Value::CreateStringValue("");
     dict->Set("value", data);
     dict->SetBoolean("managed", managed);
     if (path == kProxyType) {
@@ -333,57 +348,4 @@ bool ProxyCrosSettingsProvider::Get(const std::string& path,
   }
 }
 
-bool ProxyCrosSettingsProvider::HandlesSetting(const std::string& path) const {
-  return ::StartsWithASCII(path, "cros.session.proxy", true);
-}
-
-//----------------- ProxyCrosSettingsProvider: private methods -----------------
-
-chromeos::ProxyConfigServiceImpl*
-    ProxyCrosSettingsProvider::GetConfigService() const {
-  return profile_->GetProxyConfigTracker();
-}
-
-net::ProxyServer ProxyCrosSettingsProvider::CreateProxyServerFromHost(
-    const std::string& host,
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
-    net::ProxyServer::Scheme scheme) const {
-  uint16 port = 0;
-  if (proxy.server.is_valid())
-    port = proxy.server.host_port_pair().port();
-  if (host.length() == 0 && port == 0)
-    return net::ProxyServer();
-  if (port == 0)
-    port = net::ProxyServer::GetDefaultPortForScheme(scheme);
-  net::HostPortPair host_port_pair(host, port);
-  return net::ProxyServer(scheme, host_port_pair);
-}
-
-net::ProxyServer ProxyCrosSettingsProvider::CreateProxyServerFromPort(
-    uint16 port,
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
-    net::ProxyServer::Scheme scheme) const {
-  std::string host;
-  if (proxy.server.is_valid())
-    host = proxy.server.host_port_pair().host();
-  if (host.length() == 0 && port == 0)
-    return net::ProxyServer();
-  net::HostPortPair host_port_pair(host, port);
-  return net::ProxyServer(scheme, host_port_pair);
-}
-
-Value* ProxyCrosSettingsProvider::CreateServerHostValue(
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) const {
-  return proxy.server.is_valid() ?
-         Value::CreateStringValue(proxy.server.host_port_pair().host()) :
-         NULL;
-}
-
-Value* ProxyCrosSettingsProvider::CreateServerPortValue(
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) const {
-  return proxy.server.is_valid() ?
-         Value::CreateIntegerValue(proxy.server.host_port_pair().port()) :
-         NULL;
-}
-
 }  // namespace chromeos