Introduce a separate preference for 'proxy server mode'

chrome/browser/policy/configuration_policy_pref_store.cc

Index: chrome/browser/policy/configuration_policy_pref_store.cc
diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index 3971980cae376ee8c14f998a5ec04c78361d4315..5bed7ecdb202043cd33ba3b2ea8071cc6dc5eca7 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -12,6 +12,7 @@
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/prefs/proxy_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 #if defined(OS_WIN)
@@ -248,7 +249,7 @@ ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList() {
       key::kDefaultSearchProviderIconURL },
     { kPolicyDefaultSearchProviderEncodings, Value::TYPE_STRING,
       key::kDefaultSearchProviderEncodings },
-    { kPolicyProxyServerMode, Value::TYPE_INTEGER, key::kProxyServerMode },
+    { kPolicyProxyMode, Value::TYPE_INTEGER, key::kProxyMode },
     { kPolicyProxyServer, Value::TYPE_STRING, key::kProxyServer },
     { kPolicyProxyPacUrl, Value::TYPE_STRING, key::kProxyPacUrl },
     { kPolicyProxyBypassList, Value::TYPE_STRING, key::kProxyBypassList },
@@ -331,13 +332,10 @@ ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList() {
 ConfigurationPolicyPrefStore::ConfigurationPolicyPrefStore(
     ConfigurationPolicyProvider* provider)
     : provider_(provider),
-      prefs_(new DictionaryValue()),
-      lower_priority_proxy_settings_overridden_(false),
-      proxy_disabled_(false),
-      proxy_configuration_specified_(false),
-      use_system_proxy_(false) {
+      prefs_(new DictionaryValue()) {
   if (!provider_->Provide(this))
     LOG(WARNING) << "Failed to get policy from provider.";
+  FinalizeProxyPolicySettings();
   FinalizeDefaultSearchPolicySettings();
 }
 
@@ -415,17 +413,6 @@ ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore() {
       g_configuration_policy_provider_keeper.Get().recommended_provider());
 }
 
-// static
-void ConfigurationPolicyPrefStore::GetProxyPreferenceSet(
-    ProxyPreferenceSet* proxy_pref_set) {
-  proxy_pref_set->clear();
-  for (size_t current = 0; current < arraysize(kProxyPolicyMap); ++current) {
-    proxy_pref_set->insert(kProxyPolicyMap[current].preference_path);
-  }
-  proxy_pref_set->insert(prefs::kNoProxyServer);
-  proxy_pref_set->insert(prefs::kProxyAutoDetect);
-}
-
 const ConfigurationPolicyPrefStore::PolicyToPreferenceMapEntry*
 ConfigurationPolicyPrefStore::FindPolicyInMap(
     ConfigurationPolicyType policy,
@@ -469,108 +456,163 @@ bool ConfigurationPolicyPrefStore::ApplyPolicyFromMap(
 bool ConfigurationPolicyPrefStore::ApplyProxyPolicy(
     ConfigurationPolicyType policy,
     Value* value) {
-  bool result = false;
-  bool warn_about_proxy_disable_config = false;
-  bool warn_about_proxy_system_config = false;
-
-  const PolicyToPreferenceMapEntry* match_entry =
-      FindPolicyInMap(policy, kProxyPolicyMap, arraysize(kProxyPolicyMap));
-
-  // When the first proxy-related policy is applied, ALL proxy-related
-  // preferences that have been set by command-line switches, extensions,
-  // user preferences or any other mechanism are overridden. Otherwise
-  // it's possible for a user to interfere with proxy policy by setting
-  // proxy-related command-line switches or set proxy-related prefs in an
-  // extension that are related, but not identical, to the ones set through
-  // policy.
-  if (!lower_priority_proxy_settings_overridden_ &&
-      (match_entry ||
-       policy == kPolicyProxyServerMode)) {
-    ProxyPreferenceSet proxy_preference_set;
-    GetProxyPreferenceSet(&proxy_preference_set);
-    for (ProxyPreferenceSet::const_iterator i = proxy_preference_set.begin();
-         i != proxy_preference_set.end(); ++i) {
-      // We use values of TYPE_NULL to mark preferences for which
-      // READ_USE_DEFAULT should be returned by GetValue().
-      prefs_->Set(*i, Value::CreateNullValue());
-    }
-    lower_priority_proxy_settings_overridden_ = true;
+  // We only collect the values until we have sufficient information when
+  // FinalizeProxyPolicySettings() is called to determine whether the presented
+  // values were correct and apply them in that case.
+  scoped_ptr<Value>* target_container = NULL;
+  switch (policy) {
+    case kPolicyProxyMode:
+      target_container = &proxy_mode_container_;
+      break;
+    case kPolicyProxyServer:
+      target_container = &proxy_server_container_;
+      break;
+    case kPolicyProxyPacUrl:
+      target_container = &proxy_pac_url_container_;
+      break;
+    case kPolicyProxyBypassList:
+      target_container = &proxy_bypass_list_container_;
+      break;
+    default:
+      // We are not interested in this policy.
+      return false;
   }
+  CHECK(target_container);
 
-  // Translate the proxy policy into preferences.
-  if (policy == kPolicyProxyServerMode) {
-    int int_value;
-    bool proxy_auto_detect = false;
-    if (value->GetAsInteger(&int_value)) {
-      result = true;
-      switch (int_value) {
-        case kPolicyNoProxyServerMode:
-          if (!proxy_disabled_) {
-            if (proxy_configuration_specified_)
-              warn_about_proxy_disable_config = true;
-            proxy_disabled_ = true;
-          }
-          break;
-        case kPolicyAutoDetectProxyMode:
-          proxy_auto_detect = true;
-          break;
-        case kPolicyManuallyConfiguredProxyMode:
-          break;
-        case kPolicyUseSystemProxyMode:
-          if (!use_system_proxy_) {
-            if (proxy_configuration_specified_)
-              warn_about_proxy_system_config = true;
-            use_system_proxy_ = true;
-          }
-          break;
-        default:
-          // Not a valid policy, don't assume ownership of |value|
-          result = false;
-          break;
-      }
+  (*target_container).reset(value);
+  return true;
+}
 
-      if (int_value != kPolicyUseSystemProxyMode) {
-        prefs_->Set(prefs::kNoProxyServer,
-                    Value::CreateBooleanValue(proxy_disabled_));
-        prefs_->Set(prefs::kProxyAutoDetect,
-                    Value::CreateBooleanValue(proxy_auto_detect));
-      }
-    }
-  } else if (match_entry) {
-    // Determine if the applied proxy policy settings conflict and issue
-    // a corresponding warning if they do.
-    if (!proxy_configuration_specified_) {
-      if (proxy_disabled_)
-        warn_about_proxy_disable_config = true;
-      if (use_system_proxy_)
-        warn_about_proxy_system_config = true;
-      proxy_configuration_specified_ = true;
-    }
-    if (!use_system_proxy_ && !proxy_disabled_) {
-      prefs_->Set(match_entry->preference_path, value);
-      // The ownership of value has been passed on to |prefs_|,
-      // don't clean it up later.
-      value = NULL;
-    }
-    result = true;
+void ConfigurationPolicyPrefStore::FinalizeProxyPolicySettings() {
+  if (CheckProxySettings())
+    ApplyProxySettings();
+  proxy_mode_container_.reset(NULL);
+  proxy_server_container_.reset(NULL);
+  proxy_pac_url_container_.reset(NULL);
+  proxy_bypass_list_container_.reset(NULL);
+}
+
+bool ConfigurationPolicyPrefStore::CheckProxySettings() {
+  bool mode = proxy_mode_container_.get() &&
+              !proxy_mode_container_->IsType(Value::TYPE_NULL);
+  bool server = proxy_server_container_.get() &&
+                !proxy_server_container_->IsType(Value::TYPE_NULL);
+  bool pac_url = proxy_pac_url_container_.get() &&
+                 !proxy_pac_url_container_->IsType(Value::TYPE_NULL);
+  bool bypass_list = proxy_bypass_list_container_.get() &&
+                     !proxy_bypass_list_container_->IsType(Value::TYPE_NULL);
+
+  if ((server || pac_url || bypass_list) && !mode) {
+    LOG(WARNING) << "A centrally-administered policy defines proxy setting"
+                 << " details without setting a proxy mode.";
+    return false;
   }
 
-  if (warn_about_proxy_disable_config) {
-    LOG(WARNING) << "A centrally-administered policy disables the use of"
-                 << " a proxy but also specifies an explicit proxy"
-                 << " configuration.";
+  if (!mode)
+    return true;
+
+  int mode_value;
+  if (!proxy_mode_container_->GetAsInteger(&mode_value)) {
+    LOG(WARNING) << "Invalid proxy mode value.";
+    return false;
   }
 
-  if (warn_about_proxy_system_config) {
-    LOG(WARNING) << "A centrally-administered policy dictates that the"
-                 << " system proxy settings should be used but also specifies"
-                 << " an explicit proxy configuration.";
+  switch (mode_value) {
+    case kPolicyNoProxyServerMode:
+      if (server || pac_url || bypass_list) {
+        LOG(WARNING) << "A centrally-administered policy disables the use of"
+                     << " a proxy but also specifies an explicit proxy"
+                     << " configuration.";
+        return false;
+      }
+      break;
+    case kPolicyAutoDetectProxyMode:
+      if (server || bypass_list) {
+        LOG(WARNING) << "A centrally-administered policy dictates that a proxy"
+                     << " shall be auto configured but specifies fixed proxy"
+                     << " servers or a by-pass list.";
+        return false;
+      }
+      break;
+    case kPolicyManuallyConfiguredProxyMode:
+      if (!server) {
+        LOG(WARNING) << "A centrally-administered policy dictates that the"
+                     << " system proxy settings should use fixed proxy servers"
+                     << " without specifying which ones.";
+        return false;
+      }
+      if (pac_url) {
+        LOG(WARNING) << "A centrally-administered policy dictates that the"
+                     << " system proxy settings should use fixed proxy servers"
+                     << " but also specifies a PAC script.";
+        return false;
+      }
+      break;
+    case kPolicyUseSystemProxyMode:
+      if (server || pac_url || bypass_list) {
+        LOG(WARNING) << "A centrally-administered policy dictates that the"
+                     << " system proxy settings should be used but also "
+                     << " specifies an explicit proxy configuration.";
+        return false;
+      }
+      break;
+    default:
+      LOG(WARNING) << "Invalid proxy mode " << mode_value;
+      return false;
   }
+  return true;
+}
 
-  // If the policy was a proxy policy, cleanup |value|.
-  if (result && value)
-    delete value;
-  return result;
+void ConfigurationPolicyPrefStore::ApplyProxySettings() {
+  if (proxy_mode_container_.get() == NULL ||
+      proxy_mode_container_->IsType(Value::TYPE_NULL))
+    return;
+
+  int int_mode;
+  CHECK(proxy_mode_container_->GetAsInteger(&int_mode));
+  ProxyPrefs::ProxyMode mode;
+  switch (int_mode) {
+  case kPolicyNoProxyServerMode:
+    mode = ProxyPrefs::kModeDirect;
+    break;
+  case kPolicyAutoDetectProxyMode:
+    mode = ProxyPrefs::kModeAutoDetect;
+    if (proxy_pac_url_container_.get() &&
+        !proxy_pac_url_container_->IsType(Value::TYPE_NULL)) {
+      mode = ProxyPrefs::kModePacScript;
+    }
+    break;
+  case kPolicyManuallyConfiguredProxyMode:
+    mode = ProxyPrefs::kModeFixedServers;
+    break;
+  case kPolicyUseSystemProxyMode:
+    mode = ProxyPrefs::kModeSystem;
+    break;
+  default:
+    mode = ProxyPrefs::kModeDirect;
+    NOTREACHED();
+  }
+  prefs_->Set(prefs::kProxyMode, Value::CreateIntegerValue(mode));
+
+  if (proxy_server_container_.get() &&
+      !proxy_server_container_->IsType(Value::TYPE_NULL)) {
+    prefs_->Set(prefs::kProxyServer, proxy_server_container_.release());
+  } else {
+    prefs_->Set(prefs::kProxyServer, Value::CreateNullValue());
+  }
+  if (proxy_pac_url_container_.get() &&
+      !proxy_pac_url_container_->IsType(Value::TYPE_NULL)) {
+    prefs_->Set(prefs::kProxyPacUrl, proxy_pac_url_container_.release());
+  } else {
+    prefs_->Set(prefs::kProxyPacUrl, Value::CreateNullValue());
+  }
+  if (proxy_bypass_list_container_.get() &&
+      !proxy_bypass_list_container_->IsType(Value::TYPE_NULL)) {
+    prefs_->Set(prefs::kProxyBypassList,
+                proxy_bypass_list_container_.release());
+  } else {
+    prefs_->Set(prefs::kProxyBypassList, Value::CreateNullValue());
+  }
 }
 
 bool ConfigurationPolicyPrefStore::ApplySyncPolicy(