NetworkHandler: Add ui_proxy_config_service

chromeos/network/proxy/ui_proxy_config_service.cc

Index: chromeos/network/proxy/ui_proxy_config_service.cc
diff --git a/chromeos/network/proxy/ui_proxy_config_service.cc b/chromeos/network/proxy/ui_proxy_config_service.cc
index 810d85a0449cc511cbc1dfe1dccbe9ef5c211030..064b8452a431fbdf3bc9806ab4ea4b1938cdaeec 100644
--- a/chromeos/network/proxy/ui_proxy_config_service.cc
+++ b/chromeos/network/proxy/ui_proxy_config_service.cc
@@ -6,6 +6,8 @@
 
 #include <memory>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/network_state.h"
@@ -14,6 +16,7 @@
 #include "chromeos/network/proxy/proxy_config_service_impl.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/proxy_config/pref_proxy_config_tracker_impl.h"
+#include "components/proxy_config/proxy_config_pref_names.h"
 #include "net/proxy/proxy_config.h"
 
 namespace chromeos {
@@ -40,14 +43,14 @@ const char* ModeToString(UIProxyConfig::Mode mode) {
 // Writes the proxy config of |network| to |proxy_config|.  Sets |onc_source| to
 // the source of this configuration. Returns false if no proxy was configured
 // for this network.
-bool GetProxyConfig(const PrefService* profile_prefs,
+bool GetProxyConfig(const PrefService* logged_in_profile_prefs,
                     const PrefService* local_state_prefs,
                     const NetworkState& network,
                     net::ProxyConfig* proxy_config,
                     onc::ONCSource* onc_source) {
   std::unique_ptr<ProxyConfigDictionary> proxy_dict =
-      proxy_config::GetProxyConfigForNetwork(profile_prefs, local_state_prefs,
-                                             network, onc_source);
+      proxy_config::GetProxyConfigForNetwork(
+          logged_in_profile_prefs, local_state_prefs, network, onc_source);
   if (!proxy_dict)
     return false;
   return PrefProxyConfigTrackerImpl::PrefConfigToNetConfig(*proxy_dict,
@@ -62,32 +65,47 @@ bool IsNetworkProxySettingsEditable(const onc::ONCSource onc_source) {
 
 }  // namespace
 
-UIProxyConfigService::UIProxyConfigService()
-    : profile_prefs_(nullptr), local_state_prefs_(nullptr) {}
-
-UIProxyConfigService::~UIProxyConfigService() {}
-
-void UIProxyConfigService::SetPrefs(PrefService* profile_prefs,
-                                    PrefService* local_state_prefs) {
-  profile_prefs_ = profile_prefs;
-  local_state_prefs_ = local_state_prefs;
+UIProxyConfigService::UIProxyConfigService(PrefService* local_state_prefs,
+                                           PrefService* logged_in_profile_prefs)

[michaelpg, 2016/11/02 05:34:51]

nit: Can you order the pref service args to match the order used by
proxy_config::GetProxyConfigForNetwork? Or if it's common to pass both
PrefServices around, use whatever the most common ordering is.

[stevenjb, 2016/11/02 18:03:41]

Argh. I tried to change some code to local, user since local is always set, but
there's a bunch of other code that is user, local and I don't want to try to
change all of it, so I will make all of this code user, local. Sigh.

+    : logged_in_profile_prefs_(logged_in_profile_prefs),

[michaelpg, 2016/11/02 05:34:51]

order according to args

[stevenjb, 2016/11/02 18:03:41]

Done.

+      local_state_prefs_(local_state_prefs) {
+  DCHECK(local_state_prefs);
+  local_state_registrar_.Init(local_state_prefs);
+  local_state_registrar_.Add(
+      ::proxy_config::prefs::kProxy,
+      base::Bind(&UIProxyConfigService::OnPreferenceChanged,
+                 base::Unretained(this)));
+
+  if (logged_in_profile_prefs) {
+    logged_in_profile_registrar_.Init(logged_in_profile_prefs);
+    logged_in_profile_registrar_.Add(
+        ::proxy_config::prefs::kProxy,
+        base::Bind(&UIProxyConfigService::OnPreferenceChanged,
+                   base::Unretained(this)));
+    logged_in_profile_registrar_.Add(
+        ::proxy_config::prefs::kUseSharedProxies,
+        base::Bind(&UIProxyConfigService::OnPreferenceChanged,
+                   base::Unretained(this)));
+  }
 }
 
-void UIProxyConfigService::SetCurrentNetworkGuid(
-    const std::string& current_guid) {
-  current_ui_network_guid_ = current_guid;
-}
+UIProxyConfigService::~UIProxyConfigService() {}
 
-void UIProxyConfigService::UpdateFromPrefs() {
+void UIProxyConfigService::UpdateFromPrefs(const std::string& network_guid) {
+  current_ui_network_guid_ = network_guid;
   const NetworkState* network = nullptr;
-  if (!current_ui_network_guid_.empty()) {
+  if (!network_guid.empty()) {
     network =
         NetworkHandler::Get()->network_state_handler()->GetNetworkStateFromGuid(
-            current_ui_network_guid_);
+            network_guid);
+    if (!network) {
+      NET_LOG(ERROR) << "No NetworkState for guid: " << network_guid;
+    } else if (!network->IsInProfile()) {
+      NET_LOG(ERROR) << "Network not in profile: " << network_guid;
+      network = nullptr;
+    }
   }
-  if (!network || !network->IsInProfile()) {
-    NET_LOG(ERROR) << "No configured NetworkState for guid: "
-                   << current_ui_network_guid_;
+  if (!network) {
     current_ui_network_guid_.clear();
     current_ui_config_ = UIProxyConfig();
     return;
@@ -100,11 +118,16 @@ void UIProxyConfigService::UpdateFromPrefs() {
           << ", modifiable:" << current_ui_config_.user_modifiable;
 }
 
-void UIProxyConfigService::GetProxyConfig(UIProxyConfig* config) const {
+void UIProxyConfigService::GetProxyConfig(const std::string& network_guid,
+                                          UIProxyConfig* config) {
+  if (network_guid != current_ui_network_guid_)
+    UpdateFromPrefs(network_guid);
   *config = current_ui_config_;

[michaelpg, 2016/11/02 05:34:51]

Is this a copy? So current_ui_config_ is just used as a cache to copy from?

[stevenjb, 2016/11/02 18:03:41]

Correct. The old options code will call into this once for each individual proxy
property (there are 15, see proxy_cros_settings_parser.h). The new code will set
the entire dictionary at which point we should be able to abandon this class
entirely...

 }
 
-void UIProxyConfigService::SetProxyConfig(const UIProxyConfig& config) {
+void UIProxyConfigService::SetProxyConfig(const std::string& network_guid,
+                                          const UIProxyConfig& config) {
+  current_ui_network_guid_ = network_guid;
   current_ui_config_ = config;
   if (current_ui_network_guid_.empty())
     return;
@@ -137,7 +160,7 @@ void UIProxyConfigService::DetermineEffectiveConfig(
   // The pref service to read proxy settings that apply to all networks.
   // Settings from the profile overrule local state.
   PrefService* top_pref_service =
-      profile_prefs_ ? profile_prefs_ : local_state_prefs_;
+      logged_in_profile_prefs_ ? logged_in_profile_prefs_ : local_state_prefs_;
 
   // Get prefs proxy config if available.
   net::ProxyConfig pref_config;
@@ -149,8 +172,8 @@ void UIProxyConfigService::DetermineEffectiveConfig(
   net::ProxyConfigService::ConfigAvailability network_availability =
       net::ProxyConfigService::CONFIG_UNSET;
   onc::ONCSource onc_source = onc::ONC_SOURCE_NONE;
-  if (chromeos::GetProxyConfig(profile_prefs_, local_state_prefs_, network,
-                               &network_config, &onc_source)) {
+  if (chromeos::GetProxyConfig(logged_in_profile_prefs_, local_state_prefs_,
+                               network, &network_config, &onc_source)) {
     // Network is private or shared with user using shared proxies.
     VLOG(1) << this << ": using proxy of network: " << network.path();
     network_availability = net::ProxyConfigService::CONFIG_VALID;
@@ -173,8 +196,13 @@ void UIProxyConfigService::DetermineEffectiveConfig(
     current_ui_config_.user_modifiable = false;
   } else {
     current_ui_config_.user_modifiable = !ProxyConfigServiceImpl::IgnoreProxy(
-        profile_prefs_, network.profile_path(), onc_source);
+        logged_in_profile_prefs_, network.profile_path(), onc_source);
   }
 }
 
+void UIProxyConfigService::OnPreferenceChanged(const std::string& pref_name) {
+  if (!current_ui_network_guid_.empty())
+    UpdateFromPrefs(current_ui_network_guid_);
+}
+
 }  // namespace chromeos