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

chrome/browser/chromeos/proxy_cros_settings_parser.cc

 // 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.
 
-#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 -----------------
-
-ProxyCrosSettingsProvider::ProxyCrosSettingsProvider(Profile* profile)
-    : profile_(profile) {
+base::Value* CreateServerHostValue(
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) {
+  return proxy.server.is_valid() ?
+         base::Value::CreateStringValue(proxy.server.host_port_pair().host()) :
+         NULL;
 }
 
-void ProxyCrosSettingsProvider::SetCurrentNetwork(const std::string& network) {
-  GetConfigService()->UISetCurrentNetwork(network);
-  for (size_t i = 0; i < arraysize(kProxySettings); ++i)
-    CrosSettings::Get()->FireObservers(kProxySettings[i]);
+base::Value* CreateServerPortValue(
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) {
+  return proxy.server.is_valid() ?
+         base::Value::CreateIntegerValue(proxy.server.host_port_pair().port()) :
+         NULL;
 }
 
-void ProxyCrosSettingsProvider::MakeActiveNetworkCurrent() {
-  GetConfigService()->UIMakeActiveNetworkCurrent();
-  for (size_t i = 0; i < arraysize(kProxySettings); ++i)
-    CrosSettings::Get()->FireObservers(kProxySettings[i]);
+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::DoSet(const std::string& path,
-                                      Value* in_value) {
+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);
+}
+
+}

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

// namespace

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

Done.

+
+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);
 
   if (path == kProxyPacUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       GURL url(val);
       if (url.is_valid())
         config_service->UISetProxyConfigToPACScript(url);
@@ skipping 122 matching lines @@
     if (in_value->GetAsInteger(&val)) {
       std::string host = config.socks_proxy.server.host_port_pair().host();
       config_service->UISetProxyConfigToProxyPerScheme("socks",
           CreateProxyServerFromPort(val, config.socks_proxy,
                                     StartsWithASCII(host, "socks5://", false) ?
                                         net::ProxyServer::SCHEME_SOCKS5 :
                                         net::ProxyServer::SCHEME_SOCKS4));
     }
   } 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;
         if (list_value->GetString(x, &val)) {
           bypass_rules.AddRuleFromString(val);
         }
       }
       config_service->UISetProxyConfigBypassRules(bypass_rules);
     }
   }
 }
 
-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);
 
   if (path == kProxyPacUrl) {
     // Only show pacurl for pac-script mode.
     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) {
     data = CreateServerHostValue(config.single_proxy);
     found = true;
   } else if (path == kProxySingleHttpPort) {
     data = CreateServerPortValue(config.single_proxy);
     found = true;
   } else if (path == kProxyHttpUrl) {
     data = CreateServerHostValue(config.http_proxy);
     found = true;
   } else if (path == kProxyHttpsUrl) {
     data = CreateServerHostValue(config.https_proxy);
     found = true;
   } else if (path == kProxyType) {
     if (config.mode ==
         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:
          controlled_by = "policyManagedPrefsBannerText";
          break;
        case ProxyPrefs::CONFIG_EXTENSION:
          controlled_by = "extensionManagedPrefsBannerText";
          break;
        case ProxyPrefs::CONFIG_OTHER_PRECEDE:
          controlled_by = "unmodifiablePrefsBannerText";
          break;
        default:
          if (!config.user_modifiable)
            controlled_by = "enableSharedProxiesBannerText";
          break;
     }
     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) {
     data = CreateServerHostValue(config.ftp_proxy);
     found = true;
   } else if (path == kProxySocks) {
     data = CreateServerHostValue(config.socks_proxy);
     found = true;
   } else if (path == kProxyHttpPort) {
     data = CreateServerPortValue(config.http_proxy);
     found = true;
   } else if (path == kProxyHttpsPort) {
     data = CreateServerPortValue(config.https_proxy);
     found = true;
   } else if (path == kProxyFtpPort) {
     data = CreateServerPortValue(config.ftp_proxy);
     found = true;
   } else if (path == kProxySocksPort) {
     data = CreateServerPortValue(config.socks_proxy);
     found = true;
   } else if (path == kProxyIgnoreList) {
     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;
   }
   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) {
       dict->SetString("controlledBy", controlled_by);
       dict->SetBoolean("disabled", !config.user_modifiable);
     }
     *out_value = dict;
     return true;
   } else {
     *out_value = NULL;
     return false;
   }
 }
 
-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