redesign and reimplement proxy config service and tracker, revise proxy ui on cros

chrome/browser/chromeos/proxy_config_service_impl.h

Index: chrome/browser/chromeos/proxy_config_service_impl.h
===================================================================
--- chrome/browser/chromeos/proxy_config_service_impl.h	(revision 106059)
+++ chrome/browser/chromeos/proxy_config_service_impl.h	(working copy)
@@ -10,65 +10,59 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/observer_list.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/login/signed_settings.h"
-#include "net/proxy/proxy_config.h"
-#include "net/proxy/proxy_config_service.h"
-#include "net/proxy/proxy_server.h"
+#include "chrome/browser/net/pref_proxy_config_tracker.h"
+#include "chrome/browser/prefs/pref_member.h"
 
 namespace chromeos {
 
 // Implementation of proxy config service for chromeos that:
-// - is RefCountedThreadSafe
-// - is wrapped by chromeos::ProxyConfigService which implements
-//   net::ProxyConfigService interface by fowarding the methods to this class
+// - extends PrefProxyConfigTracker (and so lives and runs entirely on UI
+//   thread) to handle proxy from prefs (via PrefProxyConfigTracker) and system
+//   i.e. network (via flimflam notifications)
+// - exists one per profile and one per local state
 // - retrieves initial system proxy configuration from cros settings persisted
 //   on chromeos device from chromeos revisions before migration to flimflam,
 // - persists proxy setting per network in flimflim
-// - provides network stack with latest proxy configuration for currently
-//   active network for use on IO thread
+// - provides network stack with latest effective proxy configuration for
+//   currently active network via PrefProxyConfigTracker's mechanism of pushing
+//   config to ChromeProxyConfigService
 // - provides UI with methods to retrieve and modify proxy configuration for
-//   any network (either currently active or non-active) on UI thread
+//   any remembered network (either currently active or non-active) of current
+//   user profile
 class ProxyConfigServiceImpl
-    : public base::RefCountedThreadSafe<ProxyConfigServiceImpl>,
+    : public PrefProxyConfigTracker,
       public SignedSettings::Delegate<std::string>,
       public NetworkLibrary::NetworkManagerObserver,
       public NetworkLibrary::NetworkObserver {
  public:
-  // ProxyConfigServiceImpl is created on the UI thread in
-  // chrome/browser/net/proxy_service_factory.cc::CreateProxyConfigService
-  // via BrowserProcess::chromeos_proxy_config_service_impl, and stored in
-  // g_browser_process as a scoped_refptr (because it's RefCountedThreadSafe).
+  // ProxyConfigServiceImpl is created in ProxyServiceFactory::
+  // CreatePrefProxyConfigTracker via Profile::GetProxyConfigTracker() for
+  // profile or IOThread constructor for local state and is owned by the
+  // respective classes.
   //
-  // Past that point, it can be accessed from the IO or UI threads.
-  //
-  // From the IO thread, it is accessed periodically through the wrapper class
-  // chromeos::ProxyConfigService via net::ProxyConfigService interface
-  // (GetLatestProxyConfig, AddObserver, RemoveObserver).
-  //
-  // From the UI thread, it is accessed via
-  // BrowserProcess::chromeos_proxy_config_service_impl to allow user to read
-  // or modify the proxy configuration via UIGetProxyConfig or
+  // From the UI, it is accessed via Profile::GetProxyConfigTracker to allow
+  // user to read or modify the proxy configuration via UIGetProxyConfig or
   // UISetProxyConfigTo* respectively.
-  // The new modified proxy config is posted to the IO thread through
-  // SetNewProxyConfig().  We then notify observers on the IO thread of the
-  // configuration change.
-
+  // The new modified proxy config, together with proxy from prefs if available,
+  // are used to determine the effective proxy config, which is then pushed
+  // through PrefProxyConfigTracker to ChromeProxyConfigService to the network
+  // stack.
+  //
   // In contrary to other platforms which simply use the systems' UI to allow
   // 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

[Mattias Nissler (ping if slow), 2011/10/19 17:03:42]

I'm wondering whether this is still true? net::ProxyConfig might actually fit
the bill now. Anyways, not in this CL :)

[kuan, 2011/10/20 00:41:11]

it is still true, 'cos this structure has ConfigState and user_modifiable and
its members align a lot more closely with the UI input fields :)

   // net:ProxyConfig here to include and handle these UI requirements, e.g.
-  // - where configuration was picked up from - policy or owner
+  // - where configuration was picked up from - policy, extension, etc (refer
+  //   to ProxyPrefs::ConfigSource)

[Mattias Nissler (ping if slow), 2011/10/19 17:03:42]

ConfigSource is no longer present.

[kuan, 2011/10/20 00:41:11]

Done.

   // - the read/write access of a proxy setting
   // - may add more stuff later.
-  // This is then converted to the common net::ProxyConfig before being returned
-  // to ProxyService::GetLatestProxyConfig on the IO thread to be used on the
-  // network stack.
+  // This is then converted to the common net::ProxyConfig before being pushed
+  // to PrefProxyConfigTracker::OnProxyConfigChanged and then to the network
+  // stack.
   struct ProxyConfig {
     // Specifies if proxy config is direct, auto-detect, using pac script,
     // single-proxy, or proxy-per-scheme.
@@ -80,28 +74,14 @@
       MODE_PROXY_PER_SCHEME,
     };
 
-    // Specifies where proxy configuration was picked up from.
-    enum Source {
-      SOURCE_NONE,    // No default configuration.
-      SOURCE_POLICY,  // Configuration is from policy.
-      SOURCE_OWNER,   // Configuration is from owner.
-    };
-
-    struct Setting {
-      Setting() : source(SOURCE_NONE) {}
-      bool CanBeWrittenByUser(bool user_is_owner);
-
-      Source source;
-    };
-
     // Proxy setting for mode = direct or auto-detect or using pac script.
-    struct AutomaticProxy : public Setting {
-      GURL    pac_url;  // Set if proxy is 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 : public Setting {
-      net::ProxyServer  server;
+    struct ManualProxy {
+      net::ProxyServer server;
     };
 
     ProxyConfig();
@@ -110,11 +90,16 @@
     // Converts |this| to net::ProxyConfig.
     void ToNetProxyConfig(net::ProxyConfig* net_config);
 
-    // Returns true if proxy config can be written by user.
-    // If mode is MODE_PROXY_PER_SCHEME, |scheme| is one of "http", "https",
-    // "ftp" or "socks"; otherwise, it should be empty or will be ignored.
-    bool CanBeWrittenByUser(bool user_is_owner, const std::string& scheme);
+    // 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).
+    DictionaryValue* ToPrefProxyConfig();
+
+    // Converts |this| from Dictionary of ProxyConfigDictionary format.
+    bool FromPrefProxyConfig(const DictionaryValue* proxy_dict);
+
     // 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);
@@ -137,11 +122,16 @@
     // Returns true if the given config is equivalent to this config.
     bool Equals(const ProxyConfig& other) const;
 
-    // Creates a textual dump of the configuration.
-    std::string ToString() const;
-
     Mode mode;
 
+    ProxyPrefs::ConfigState state;
+
+    // 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.
@@ -163,55 +153,30 @@
     static void EncodeAndAppendProxyServer(const std::string& scheme,
                                            const net::ProxyServer& server,
                                            std::string* spec);
-
-    // Converts |this| to Dictionary of ProxyConfigDictionary format (which
-    // is the same format used by prefs).
-    DictionaryValue* ToPrefProxyConfig();
-
-    // Converts |this| from Dictionary of ProxyConfigDictionary format.
-    bool FromPrefProxyConfig(const DictionaryValue* proxy_dict);
   };
 
-  // Usual constructor.
-  ProxyConfigServiceImpl();
-  // Constructor for testing.
-  // |init_config| specifies the ProxyConfig to use for initialization.
-  explicit ProxyConfigServiceImpl(const ProxyConfig& init_config);
+  // Constructor.
+  explicit ProxyConfigServiceImpl(PrefService* pref_service);
   virtual ~ProxyConfigServiceImpl();
 
-  // Methods called on IO thread from wrapper class chromeos::ProxyConfigService
-  // as ProxyConfigService methods.
-  void AddObserver(net::ProxyConfigService::Observer* observer);
-  void RemoveObserver(net::ProxyConfigService::Observer* observer);
-  // Called from GetLatestProxyConfig.
-  net::ProxyConfigService::ConfigAvailability IOGetProxyConfig(
-      net::ProxyConfig* config);
+  // Called by UI to set service path of |network| to be displayed or edited.
+  // Subsequent UISet* methods will use this network, until UI calls it again
+  // with a different network.
+  void UISetCurrentNetwork(const std::string& current_network);
 
-  // Called from UI thread to retrieve proxy configuration in |config|.
-  void UIGetProxyConfig(ProxyConfig* config);
+  // Called from UI to make the currently active network the one to be displayed
+  // or edited. Subsequent UISet* methods will use this network until UI calls
+  // it again when the active network has changed.
+  void UIMakeActiveNetworkCurrent();
 
-  // Called from UI thread to set service path of network to be displayed or
-  // edited.  Subsequent UISet* methods will use this network, until UI calls
-  // it again with a different network.
-  bool UISetCurrentNetwork(const std::string& current_network);
+  // Called from UI to get name of the current network set via
+  // UISetCurrentNetwork or UIMakeActiveNetworkCurrent.
+  void UIGetCurrentNetworkName(std::string* network_name);
 
-  // Called from UI thread to make the currently active network the one to be
-  // displayed or edited. Subsequent UISet* methods will use this network. until
-  // UI calls it again when the active network has changed.
-  bool UIMakeActiveNetworkCurrent();
+  // Called from UI to retrieve proxy configuration in |current_ui_config_|.
+  void UIGetProxyConfig(ProxyConfig* config);
 
-  // Called from UI thread to get name of the current active network.
-  const std::string& current_network_name() const {
-    return current_ui_network_name_;
-  }
-
-  // Called from UI thread to set/get user preference use_shared_proxies.
-  void UISetUseSharedProxies(bool use_shared);
-  bool use_shared_proxies() const {
-    return use_shared_proxies_;
-  }
-
-  // Called from UI thread to update proxy configuration for different modes.
+  // Called from UI to update proxy configuration for different modes.
   // Returns true if config is set properly and persisted to flimflam for the
   // current network (set via UISetCurrentNetwork/UIMakeActiveNetworkCurrent).
   // If this network is also currently active, config service proceeds to start
@@ -240,25 +205,30 @@
   // NetworkLibrary::NetworkObserver implementation.
   virtual void OnNetworkChanged(NetworkLibrary* cros, const Network* network);
 
+  // PrefProxyConfigTracker implementation.
+  virtual void OnProxyConfigChanged(ProxyPrefs::ConfigState config_state,
+                                    const net::ProxyConfig& config);

[stevenjb, 2011/10/19 17:29:39]

All of the above should also have OVERRIDE

[kuan, 2011/10/20 00:41:11]

Done.

+
 #if defined(UNIT_TEST)
-  void SetTesting() {
-    testing_ = true;
-    active_network_ = "test";
+  void SetTesting(ProxyConfig* test_config) {
     UIMakeActiveNetworkCurrent();
+    if (test_config) {
+      std::string value;
+      test_config->SerializeForNetwork(&value);
+      SetProxyConfigForNetwork(active_network_, value, false);
+    }
   }
 #endif  // defined(UNIT_TEST)
 
  private:
-  friend class base::RefCountedThreadSafe<ProxyConfigServiceImpl>;
+  // NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
 
-  // Called from UI thread from the various UISetProxyConfigTo*.
+  // Called from the various UISetProxyConfigTo*.
   void OnUISetProxyConfig();
 
-  // Posted from UI thread to IO thread to carry the new config information.
-  void IOSetProxyConfig(
-      const ProxyConfig& new_config,
-      net::ProxyConfigService::ConfigAvailability new_availability);
-
   // Called from OnNetworkManagerChanged and OnNetworkChanged for currently
   // active network, to handle previously active network, new active network,
   // and if necessary, migrates device settings to flimflam and/or activates
@@ -267,73 +237,70 @@
                               const Network* active_network);
 
   // Sets proxy config for |network_path| into flimflam and activates setting
-  // if the network is currently active.
+  // if the network is currently active.  If  |only_set_if_empty| is true,

[Mattias Nissler (ping if slow), 2011/10/19 17:03:42]

remove one space after If

[kuan, 2011/10/20 00:41:11]

Done.

+  // proxy will be set and saved only if network has no proxy.
   void SetProxyConfigForNetwork(const std::string& network_path,
                                 const std::string& value,
                                 bool only_set_if_empty);
 
-  // Determines and activates proxy config of |network| based on if network is
-  // shared/private or user is using shared proxies, etc.
-  void DetermineConfigFromNetwork(const Network* network);
+  // Sets the use-shared-proxies user pref; if this triggers new effective
+  // config, activate it.
+  void SetUseSharedProxies(bool use_shared);
 
-  // Set |current_ui_network_name_| with name of |network|.
-  void SetCurrentNetworkName(const Network* network);
+  // Determines effective proxy config based on prefs from config tracker,
+  // |network| and if user is using shared proxies.
+  // If |activate| is true, effective config is stored in |active_config_| and
+  // activated on network stack, and hence, picked up by observers.
+  // if |activate| is false, effective config is stored in |current_ui_config_|
+  // but not activated on network stack, and hence, not picked up by observers.
+  void DetermineEffectiveConfig(const Network* network, bool activate);
 
-  // Checks that method is called on BrowserThread::IO thread.
-  void CheckCurrentlyOnIOThread();
+  // Determines UI-related variables (|current_network_name_| and
+  // |current_ui_config_|) based on |network|, called from UISetCurrentNetwork
+  // and UIMakeActiveNetworkActive.
+  void OnUISetCurrentNetwork(const Network* network);
 
-  // Checks that method is called on BrowserThread::UI thread.
-  void CheckCurrentlyOnUIThread();
+  // Returns true if proxy is to be ignored for network, which happens if
+  // network is shared and use-shared-proxies is turned off.
+  bool IgnoreProxy(const Network* network) {
+    return network->profile_type() == PROFILE_SHARED && !use_shared_proxies_;
+  }
 
+  // Reset UI cache variables that keep track of UI activities.
+  void ResetUICache();
+
   // Data members.
 
-  // True if running unit_tests, which will need to specifically exclude
-  // flimflam logic.
-  bool testing_;
-
-  // True if tasks can be posted, which can only happen if constructor has
-  // completed (NewRunnableMethod cannot be created for a RefCountedThreadBase's
-  // method until the class's ref_count is at least one).
-  bool can_post_task_;
-
-  // Availability status of the configuration, initialized on UI thread, but
-  // afterwards only accessed from IO thread.
-  net::ProxyConfigService::ConfigAvailability config_availability_;
-
   // Service path of currently active network (determined via flimflam
-  // notifications) whose proxy config is taking effect.
+  // notifications); if effective proxy config is from system, proxy of this
+  // network will be the one taking effect.
   std::string active_network_;
-  // Proxy configuration of |active_network_|, only accessed from UI thread.
+
+  // State of |active_config_|.  |active_config_| is only valid if
+  // |active_config_state_| is not ProxyPrefs::CONFIG_UNSET.
+  ProxyPrefs::ConfigState active_config_state_;
+
+  // Active proxy configuration, which could be from prefs or network.
   ProxyConfig active_config_;
 
   // Proxy config retreived from device, in format generated from
   // SerializeForNetwork, that can be directly set into flimflam.
   std::string device_config_;
 
-  // Cached proxy configuration, to be converted to net::ProxyConfig and
-  // returned by IOGetProxyConfig.
-  // Initially populated from UI thread, but afterwards only accessed from IO
-  // thread.
-  ProxyConfig cached_config_;
+  // True if user preference UseSharedProxies is true.
+  bool use_shared_proxies_;
 
   // Service path of network whose proxy configuration is being displayed or
   // edited via UI, separate from |active_network_| which may be same or
   // different.
   std::string current_ui_network_;
 
-  // Name of network with current_ui_network_, set in UIMakeActiveNetworkCurrent
-  // and UISetCurrentNetwork.
-  std::string current_ui_network_name_;
-
-  // Proxy configuration of |current_ui_network|.
+  // Proxy configuration of |current_ui_network_|.
   ProxyConfig current_ui_config_;
 
-  // True if user preference UseSharedProxies is true.
-  bool use_shared_proxies_;
+  // Track changes in user preference UseSharedProxies.
+  BooleanPrefMember use_shared_proxies_pref_;
 
-  // List of observers for changes in proxy config.
-  ObserverList<net::ProxyConfigService::Observer> observers_;
-
   // Operation to retrieve proxy setting from device.
   scoped_refptr<SignedSettings> retrieve_property_op_;