Migrate ProxyConfigServiceImpl to NetworkStateHandler and NetworkProfileHandler.

chrome/browser/chromeos/ui_proxy_config.h

+// Copyright (c) 2012 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_UI_PROXY_CONFIG_H_
+#define CHROME_BROWSER_CHROMEOS_UI_PROXY_CONFIG_H_
+
+#include <string>
+
+#include "chrome/browser/prefs/proxy_prefs.h"
+#include "googleurl/src/gurl.h"
+#include "net/proxy/proxy_bypass_rules.h"
+#include "net/proxy/proxy_server.h"
+
+namespace base {
+class DictionaryValue;
+}
+
+namespace net {
+class ProxyConfig;
+}
+
+namespace chromeos {
+
+// In contrary to other platforms which simply use the systems' UI to allow

[stevenjb, 2013/05/15 17:44:14]

nit: s/In contrary to/Contrary to (or 'In contrast to')/

[pneubeck (no reviews), 2013/05/15 20:24:48]

Done.

+// users to configure proxies, we have to implement our own UI on the chromeos
+// device.  This requires extra and specific UI requirements that
+// net::ProxyConfig does not suffice.  So we create an augmented analog to

[stevenjb, 2013/05/15 17:44:14]

nit: s/suffice/supply

[pneubeck (no reviews), 2013/05/15 20:24:48]

Done.

+// net::ProxyConfig here to include and handle these UI requirements, e.g.
+// - state of configuration e.g. where it was picked up from - policy,
+//   extension, etc (refer to ProxyPrefs::ConfigState)
+// - the read/write access of a proxy setting
+// - may add more stuff later.
+// This is then converted to the common net::ProxyConfig before being pushed
+// to PrefProxyConfigTrackerImpl::OnProxyConfigChanged and then to the network
+// stack.
+struct UIProxyConfig {

[stevenjb, 2013/05/15 17:44:14]

This is complex enough that a class might be better, unless we're implicitly or
regularly copying it?

[pneubeck (no reviews), 2013/05/15 20:24:48]

Yes it's regularly copied.

I totally agree (also to the next comment) that this should be changed.

However, I'd like to postpone that change for a separate CL. There is still huge
potential to simplify the UI related part. And this struct is one of the first
candidates to be extremely reduced.
And this CL is already to huge.

[stevenjb, 2013/05/16 17:04:34]

OK, that's fine, as long as we do follow up; the C++ code is a bit challenging
to follow.

+  // Specifies if proxy config is direct, auto-detect, using pac script,
+  // single-proxy, or proxy-per-scheme.
+  enum Mode {
+    MODE_DIRECT,
+    MODE_AUTO_DETECT,
+    MODE_PAC_SCRIPT,
+    MODE_SINGLE_PROXY,
+    MODE_PROXY_PER_SCHEME,
+  };
+
+  // Proxy setting for mode = direct or auto-detect or using pac script.
+  struct AutomaticProxy {
+    GURL pac_url;  // Set if proxy is using pac script.
+  };
+
+  // Proxy setting for mode = single-proxy or proxy-per-scheme.
+  struct ManualProxy {
+    net::ProxyServer server;
+  };
+
+  UIProxyConfig();
+  ~UIProxyConfig();
+
+  void SetPacUrl(const GURL& pac_url);
+  void SetSingleProxy(const net::ProxyServer& server);
+
+  // |scheme| is one of "http", "https", "ftp" or "socks".
+  void SetProxyForScheme(const std::string& scheme,
+                         const net::ProxyServer& server);
+
+  // Only valid for MODE_SINGLE_PROXY or MODE_PROXY_PER_SCHEME.
+  void SetBypassRules(const net::ProxyBypassRules& rules);
+
+  // Converts net::ProxyConfig to |this|.
+  bool FromNetProxyConfig(const net::ProxyConfig& net_config);
+
+  // Converts |this| to Dictionary of ProxyConfigDictionary format (which
+  // is the same format used by prefs).
+  base::DictionaryValue* ToPrefProxyConfig();
+
+  // Map |scheme| (one of "http", "https", "ftp" or "socks") to the correct
+  // ManualProxy.  Returns NULL if scheme is invalid.
+  ManualProxy* MapSchemeToProxy(const std::string& scheme);
+
+  // We've migrated device settings to shill, so we only need to
+  // deserialize previously persisted device settings.
+  // Deserializes from signed setting on device as std::string into a
+  // protobuf and then into the config.
+  bool DeserializeForDevice(const std::string& input);
+
+  // Serializes config into a ProxyConfigDictionary and then std::string
+  // persisted as string property in shill for a network.
+  bool SerializeForNetwork(std::string* output);
+
+  // Encodes the proxy server as "<url-scheme>=<proxy-scheme>://<proxy>"
+  static void EncodeAndAppendProxyServer(const std::string& url_scheme,
+                                         const net::ProxyServer& server,
+                                         std::string* spec);
+
+  Mode mode;
+
+  ProxyPrefs::ConfigState state;

[stevenjb, 2013/05/15 17:44:14]

Without the trailing _, these look like locals in the code. We should either:
* Make this a class
* name these something like config_mode and config_state
* move the complex methods to either a helper class or global namespaced
functions

[pneubeck (no reviews), 2013/05/15 20:24:48]

I totally agree. So far this is only moved code with only the necessary changes.
See previous comment.

+
+  // True if user can modify proxy settings via UI.
+  // If proxy is managed by policy or extension or other_precde or is for
+  // shared network but kUseSharedProxies is turned off, it can't be modified
+  // by user.
+  bool user_modifiable;
+
+  // Set if mode is MODE_DIRECT or MODE_AUTO_DETECT or MODE_PAC_SCRIPT.
+  AutomaticProxy  automatic_proxy;
+  // Set if mode is MODE_SINGLE_PROXY.
+  ManualProxy     single_proxy;
+  // Set if mode is MODE_PROXY_PER_SCHEME and has http proxy.
+  ManualProxy     http_proxy;
+  // Set if mode is MODE_PROXY_PER_SCHEME and has https proxy.
+  ManualProxy     https_proxy;
+  // Set if mode is MODE_PROXY_PER_SCHEME and has ftp proxy.
+  ManualProxy     ftp_proxy;
+  // Set if mode is MODE_PROXY_PER_SCHEME and has socks proxy.
+  ManualProxy     socks_proxy;
+
+  // Exceptions for when not to use a proxy.
+  net::ProxyBypassRules bypass_rules;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_UI_PROXY_CONFIG_H_