Policy: Add ProxyMode and deprecate ProxyServerMode.

- Add support for 'deprecated' attribute in template generator
- Add support for both int- and string- based enums in the template generator
- Add logic to handle ProxyMode and fall back to ProxyServerMode

BUG=68134
TEST=ConfigurationPolicyPrefStore*, new policy template generator tests

Review URL: http://codereview.chromium.org/5958014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71315 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 208 insertions, 95 deletions

diff --git a/chrome/browser/policy/config_dir_policy_provider_unittest.cc b/chrome/browser/policy/config_dir_policy_provider_unittest.cc
index 982ac6fe..30c2189 100644
--- a/chrome/browser/policy/config_dir_policy_provider_unittest.cc
+++ b/chrome/browser/policy/config_dir_policy_provider_unittest.cc
@@ -261,9 +261,12 @@ INSTANTIATE_TEST_CASE_P(
         ValueTestParams::ForStringPolicy(
             kPolicyDefaultSearchProviderEncodings,
             key::kDefaultSearchProviderEncodings),
-        ValueTestParams::ForIntegerPolicy(
+        ValueTestParams::ForStringPolicy(
             kPolicyProxyMode,
             key::kProxyMode),
+        ValueTestParams::ForIntegerPolicy(
+            kPolicyProxyServerMode,
+            key::kProxyServerMode),
         ValueTestParams::ForStringPolicy(
             kPolicyProxyServer,
             key::kProxyServer),
diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index 1330279..58b5cac 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -106,8 +106,9 @@ class ConfigurationPolicyPrefKeeper
   // respective values in |prefs_|.
   void FinalizeProxyPolicySettings();
 
-  // Returns true if the policy values stored in proxy_* represent a valid
-  // proxy configuration.
+  // Returns true if the policy values stored in proxy_* represent a valid proxy
+  // configuration, including the case in which there is no configuration at
+  // all.
   bool CheckProxySettings();
 
   // Assumes CheckProxySettings returns true and applies the values stored
@@ -363,6 +364,7 @@ bool ConfigurationPolicyPrefKeeper::ApplyProxyPolicy(
   // FinalizeProxyPolicySettings() is called to determine whether the presented
   // values were correct and apply them in that case.
   if (policy == kPolicyProxyMode ||
+      policy == kPolicyProxyServerMode ||
       policy == kPolicyProxyServer ||
       policy == kPolicyProxyPacUrl ||
       policy == kPolicyProxyBypassList) {
@@ -492,96 +494,151 @@ void ConfigurationPolicyPrefKeeper::FinalizeProxyPolicySettings() {
 
 bool ConfigurationPolicyPrefKeeper::CheckProxySettings() {
   bool mode = HasProxyPolicy(kPolicyProxyMode);
+  bool server_mode = HasProxyPolicy(kPolicyProxyServerMode);  // deprecated
   bool server = HasProxyPolicy(kPolicyProxyServer);
   bool pac_url = HasProxyPolicy(kPolicyProxyPacUrl);
   bool bypass_list = HasProxyPolicy(kPolicyProxyBypassList);
 
-  if ((server || pac_url || bypass_list) && !mode) {
+  if ((server || pac_url || bypass_list) && !(mode || server_mode)) {
     LOG(WARNING) << "A centrally-administered policy defines proxy setting"
                  << " details without setting a proxy mode.";
     return false;
   }
 
-  if (!mode)
-    return true;
+  // If there's a server mode, convert it into a mode.
+  std::string mode_value;
+  if (mode) {
+    if (server_mode)
+      LOG(WARNING) << "Both ProxyMode and ProxyServerMode policies defined, "
+                   << "ignoring ProxyMode.";
+    if (!proxy_policies_[kPolicyProxyMode]->GetAsString(&mode_value)) {
+      LOG(WARNING) << "Invalid ProxyMode value.";
+      return false;
+    }
+  } else if (server_mode) {
+    int server_mode_value;
+    if (!proxy_policies_[kPolicyProxyServerMode]->GetAsInteger(
+        &server_mode_value)) {
+      LOG(WARNING) << "Invalid ProxyServerMode value.";
+      return false;
+    }
 
-  int mode_value;
-  if (!proxy_policies_[kPolicyProxyMode]->GetAsInteger(&mode_value)) {
-    LOG(WARNING) << "Invalid proxy mode value.";
-    return false;
+    switch (server_mode_value) {
+      case kPolicyNoProxyServerMode:
+        mode_value = ProxyPrefs::kDirectProxyModeName;
+        break;
+      case kPolicyAutoDetectProxyServerMode:
+        mode_value = ProxyPrefs::kAutoDetectProxyModeName;
+        break;
+      case kPolicyManuallyConfiguredProxyServerMode:
+        if (server && pac_url) {
+          LOG(WARNING) << "A centrally-administered policy dictates that"
+                       << " both fixed proxy servers and a .pac url. should"
+                       << " be used for proxy configuration.";
+          return false;
+        }
+        if (!server && !pac_url) {
+          LOG(WARNING) << "A centrally-administered policy dictates that the"
+                       << " proxy settings should use either fixed proxy"
+                       << " servers or a .pac url, but specifies neither.";
+          return false;
+        }
+        if (pac_url)
+          mode_value = ProxyPrefs::kPacScriptProxyModeName;
+        else
+          mode_value = ProxyPrefs::kFixedServersProxyModeName;
+        break;
+      case kPolicyUseSystemProxyServerMode:
+        mode_value = ProxyPrefs::kSystemProxyModeName;
+        break;
+      default:
+        LOG(WARNING) << "Invalid proxy mode " << server_mode_value;
+        return false;
+    }
   }
 
-  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 || pac_url) {
-        LOG(WARNING) << "A centrally-administered policy dictates that a proxy"
-                     << " shall be auto configured but specifies fixed proxy"
-                     << " servers, a by-pass list or a .pac script URL.";
-        return false;
-      }
-      break;
-    case kPolicyManuallyConfiguredProxyMode:
-      if (server && pac_url) {
-        LOG(WARNING) << "A centrally-administered policy dictates that the"
-                     << " system proxy settings should use both a fixed"
-                     << " proxy server and a .pac url.";
-        return false;
-      }
-      if (!server && !pac_url) {
-        LOG(WARNING) << "A centrally-administered policy dictates that the"
-                     << " system proxy settings should use either a fixed"
-                     << " proxy server or a .pac url, but specifies neither.";
-        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;
+  // If neither ProxyMode nor ProxyServerMode are specified, mode_value will be
+  // empty and the proxy shouldn't be configured at all.
+  if (mode_value.empty())
+    return true;
+
+  if (mode_value == ProxyPrefs::kDirectProxyModeName) {
+    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;
+    }
+  } else if (mode_value == ProxyPrefs::kAutoDetectProxyModeName) {
+    if (server || bypass_list || pac_url) {
+      LOG(WARNING) << "A centrally-administered policy dictates that a proxy"
+                   << " shall be auto configured but specifies fixed proxy"
+                   << " servers, a by-pass list or a .pac script URL.";
       return false;
+    }
+  } else if (mode_value == ProxyPrefs::kPacScriptProxyModeName) {
+    if (server || bypass_list) {
+      LOG(WARNING) << "A centrally-administered policy dictates that a .pac"
+                   << " script URL should be used for proxy configuration but"
+                   << " also specifies policies required only for fixed"
+                   << " proxy servers.";
+      return false;
+    }
+  } else if (mode_value == ProxyPrefs::kFixedServersProxyModeName) {
+    if (pac_url) {
+      LOG(WARNING) << "A centrally-administered policy dictates that"
+                   << " fixed proxy servers should be used but also"
+                   << " specifies a .pac script URL.";
+      return false;
+    }
+  } else if (mode_value == ProxyPrefs::kSystemProxyModeName) {
+    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;
+    }
+  } else {
+    LOG(WARNING) << "Invalid proxy mode " << mode_value;
+    return false;
   }
   return true;
 }
 
 void ConfigurationPolicyPrefKeeper::ApplyProxySettings() {
-  if (!HasProxyPolicy(kPolicyProxyMode))
-    return;
-
-  int int_mode;
-  CHECK(proxy_policies_[kPolicyProxyMode]->GetAsInteger(&int_mode));
   ProxyPrefs::ProxyMode mode;
-  switch (int_mode) {
-    case kPolicyNoProxyServerMode:
-      mode = ProxyPrefs::MODE_DIRECT;
-      break;
-    case kPolicyAutoDetectProxyMode:
-      mode = ProxyPrefs::MODE_AUTO_DETECT;
-      break;
-    case kPolicyManuallyConfiguredProxyMode:
-      mode = ProxyPrefs::MODE_FIXED_SERVERS;
-      if (HasProxyPolicy(kPolicyProxyPacUrl))
-        mode = ProxyPrefs::MODE_PAC_SCRIPT;
-      break;
-    case kPolicyUseSystemProxyMode:
-      mode = ProxyPrefs::MODE_SYSTEM;
-      break;
-    default:
-      mode = ProxyPrefs::MODE_DIRECT;
-      NOTREACHED();
+  if (HasProxyPolicy(kPolicyProxyMode)) {
+    std::string string_mode;
+    CHECK(proxy_policies_[kPolicyProxyMode]->GetAsString(&string_mode));
+    if (!ProxyPrefs::StringToProxyMode(string_mode, &mode)) {
+      LOG(WARNING) << "A centrally-administered policy specifies a value for"
+                   << "the ProxyMode policy that isn't recognized.";
+      return;
+    }
+  } else if (HasProxyPolicy(kPolicyProxyServerMode)) {
+    int int_mode = 0;
+    CHECK(proxy_policies_[kPolicyProxyServerMode]->GetAsInteger(&int_mode));
+    switch (int_mode) {
+      case kPolicyNoProxyServerMode:
+        mode = ProxyPrefs::MODE_DIRECT;
+        break;
+      case kPolicyAutoDetectProxyServerMode:
+        mode = ProxyPrefs::MODE_AUTO_DETECT;
+        break;
+      case kPolicyManuallyConfiguredProxyServerMode:
+        mode = ProxyPrefs::MODE_FIXED_SERVERS;
+        if (HasProxyPolicy(kPolicyProxyPacUrl))
+          mode = ProxyPrefs::MODE_PAC_SCRIPT;
+        break;
+      case kPolicyUseSystemProxyServerMode:
+        mode = ProxyPrefs::MODE_SYSTEM;
+        break;
+      default:
+        mode = ProxyPrefs::MODE_DIRECT;
+        NOTREACHED();
+    }
+  } else {
+    return;
   }
   prefs_.SetValue(prefs::kProxyMode, Value::CreateIntegerValue(mode));
 
@@ -710,7 +767,8 @@ ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList() {
       key::kDefaultSearchProviderIconURL },
     { kPolicyDefaultSearchProviderEncodings, Value::TYPE_STRING,
       key::kDefaultSearchProviderEncodings },
-    { kPolicyProxyMode, Value::TYPE_INTEGER, key::kProxyMode },
+    { kPolicyProxyMode, Value::TYPE_STRING, key::kProxyMode },
+    { kPolicyProxyServerMode, Value::TYPE_INTEGER, key::kProxyServerMode },
     { kPolicyProxyServer, Value::TYPE_STRING, key::kProxyServer },
     { kPolicyProxyPacUrl, Value::TYPE_STRING, key::kProxyPacUrl },
     { kPolicyProxyBypassList, Value::TYPE_STRING, key::kProxyBypassList },
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index 9f461c1..d81ff91 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -269,9 +269,9 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ManualOptions) {
                      Value::CreateStringValue("http://chromium.org/override"));
   provider.AddPolicy(kPolicyProxyServer,
                      Value::CreateStringValue("chromium.org"));
-  provider.AddPolicy(kPolicyProxyMode,
+  provider.AddPolicy(kPolicyProxyServerMode,
                      Value::CreateIntegerValue(
-                         kPolicyManuallyConfiguredProxyMode));
+                         kPolicyManuallyConfiguredProxyServerMode));
 
   ConfigurationPolicyPrefStore store(&provider);
   VerifyProxyPrefs(
@@ -281,9 +281,9 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ManualOptions) {
 
 TEST_F(ConfigurationPolicyPrefStoreProxyTest, ManualOptionsReversedApplyOrder) {
   MockConfigurationPolicyProvider provider;
-  provider.AddPolicy(kPolicyProxyMode,
+  provider.AddPolicy(kPolicyProxyServerMode,
                      Value::CreateIntegerValue(
-                         kPolicyManuallyConfiguredProxyMode));
+                         kPolicyManuallyConfiguredProxyServerMode));
   provider.AddPolicy(kPolicyProxyBypassList,
                      Value::CreateStringValue("http://chromium.org/override"));
   provider.AddPolicy(kPolicyProxyServer,
@@ -295,50 +295,95 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ManualOptionsReversedApplyOrder) {
       ProxyPrefs::MODE_FIXED_SERVERS);
 }
 
-TEST_F(ConfigurationPolicyPrefStoreProxyTest, NoProxy) {
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, NoProxyServerMode) {
   MockConfigurationPolicyProvider provider;
-  provider.AddPolicy(kPolicyProxyMode,
+  provider.AddPolicy(kPolicyProxyServerMode,
                      Value::CreateIntegerValue(kPolicyNoProxyServerMode));
 
   ConfigurationPolicyPrefStore store(&provider);
   VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_DIRECT);
 }
 
-TEST_F(ConfigurationPolicyPrefStoreProxyTest, AutoDetect) {
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, NoProxyModeName) {
   MockConfigurationPolicyProvider provider;
-  provider.AddPolicy(kPolicyProxyMode,
-                     Value::CreateIntegerValue(kPolicyAutoDetectProxyMode));
+  provider.AddPolicy(
+      kPolicyProxyMode,
+      Value::CreateStringValue(ProxyPrefs::kDirectProxyModeName));
+
+  ConfigurationPolicyPrefStore store(&provider);
+  VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_DIRECT);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, AutoDetectProxyServerMode) {
+  MockConfigurationPolicyProvider provider;
+  provider.AddPolicy(
+      kPolicyProxyServerMode,
+      Value::CreateIntegerValue(kPolicyAutoDetectProxyServerMode));
 
   ConfigurationPolicyPrefStore store(&provider);
   VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_AUTO_DETECT);
 }
 
-TEST_F(ConfigurationPolicyPrefStoreProxyTest, AutoDetectPac) {
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, AutoDetectProxyModeName) {
+  MockConfigurationPolicyProvider provider;
+  provider.AddPolicy(
+      kPolicyProxyMode,
+      Value::CreateStringValue(ProxyPrefs::kAutoDetectProxyModeName));
+
+  ConfigurationPolicyPrefStore store(&provider);
+  VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_AUTO_DETECT);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, PacScriptProxyMode) {
   MockConfigurationPolicyProvider provider;
   provider.AddPolicy(kPolicyProxyPacUrl,
                      Value::CreateStringValue("http://short.org/proxy.pac"));
   provider.AddPolicy(
       kPolicyProxyMode,
-      Value::CreateIntegerValue(kPolicyManuallyConfiguredProxyMode));
+      Value::CreateStringValue(ProxyPrefs::kPacScriptProxyModeName));
 
   ConfigurationPolicyPrefStore store(&provider);
   VerifyProxyPrefs(
       store, "", "http://short.org/proxy.pac", "", ProxyPrefs::MODE_PAC_SCRIPT);
 }
 
-TEST_F(ConfigurationPolicyPrefStoreProxyTest, UseSystem) {
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, UseSystemProxyServerMode) {
   MockConfigurationPolicyProvider provider;
-  provider.AddPolicy(kPolicyProxyMode,
-                     Value::CreateIntegerValue(kPolicyUseSystemProxyMode));
+  provider.AddPolicy(
+      kPolicyProxyServerMode,
+      Value::CreateIntegerValue(kPolicyUseSystemProxyServerMode));
 
   ConfigurationPolicyPrefStore store(&provider);
   VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_SYSTEM);
 }
 
+TEST_F(ConfigurationPolicyPrefStoreProxyTest, UseSystemProxyMode) {
+  MockConfigurationPolicyProvider provider;
+  provider.AddPolicy(
+      kPolicyProxyMode,
+      Value::CreateStringValue(ProxyPrefs::kSystemProxyModeName));
+
+  ConfigurationPolicyPrefStore store(&provider);
+  VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_SYSTEM);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreProxyTest,
+       ProxyModeOverridesProxyServerMode) {
+  MockConfigurationPolicyProvider provider;
+  provider.AddPolicy(kPolicyProxyServerMode,
+                     Value::CreateIntegerValue(kPolicyNoProxyServerMode));
+  provider.AddPolicy(
+      kPolicyProxyMode,
+      Value::CreateStringValue(ProxyPrefs::kAutoDetectProxyModeName));
+
+  ConfigurationPolicyPrefStore store(&provider);
+  VerifyProxyPrefs(store, "", "", "", ProxyPrefs::MODE_AUTO_DETECT);
+}
+
 TEST_F(ConfigurationPolicyPrefStoreProxyTest, ProxyInvalid) {
   for (int i = 0; i < MODE_COUNT; ++i) {
     MockConfigurationPolicyProvider provider;
-    provider.AddPolicy(kPolicyProxyMode, Value::CreateIntegerValue(i));
+    provider.AddPolicy(kPolicyProxyServerMode, Value::CreateIntegerValue(i));
     // No mode expects all three parameters being set.
     provider.AddPolicy(kPolicyProxyPacUrl,
                        Value::CreateStringValue("http://short.org/proxy.pac"));
diff --git a/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc b/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
index 4203fb0..11d04a7 100644
--- a/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
@@ -235,9 +235,12 @@ INSTANTIATE_TEST_CASE_P(
         PolicyTestParams::ForStringPolicy(
             kPolicyDefaultSearchProviderEncodings,
             key::kDefaultSearchProviderEncodings),
-        PolicyTestParams::ForIntegerPolicy(
+        PolicyTestParams::ForStringPolicy(
             kPolicyProxyMode,
             key::kProxyMode),
+        PolicyTestParams::ForIntegerPolicy(
+            kPolicyProxyServerMode,
+            key::kProxyServerMode),
         PolicyTestParams::ForStringPolicy(
             kPolicyProxyServer,
             key::kProxyServer),
diff --git a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc
index 6085192..481e1d8 100644
--- a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc
@@ -393,9 +393,12 @@ INSTANTIATE_TEST_CASE_P(
         PolicyTestParams::ForStringPolicy(
             kPolicyDefaultSearchProviderEncodings,
             key::kDefaultSearchProviderEncodings),
-        PolicyTestParams::ForIntegerPolicy(
+        PolicyTestParams::ForStringPolicy(
             kPolicyProxyMode,
             key::kProxyMode),
+        PolicyTestParams::ForIntegerPolicy(
+            kPolicyProxyServerMode,
+            key::kProxyServerMode),
         PolicyTestParams::ForStringPolicy(
             kPolicyProxyServer,
             key::kProxyServer),
diff --git a/chrome/browser/policy/configuration_policy_store_interface.h b/chrome/browser/policy/configuration_policy_store_interface.h
index e690242..b7f2f05 100644
--- a/chrome/browser/policy/configuration_policy_store_interface.h
+++ b/chrome/browser/policy/configuration_policy_store_interface.h
@@ -27,6 +27,7 @@ enum ConfigurationPolicyType {
   kPolicyDefaultSearchProviderEncodings,
   kPolicyDisableSpdy,
   kPolicyProxyMode,
+  kPolicyProxyServerMode,
   kPolicyProxyServer,
   kPolicyProxyPacUrl,
   kPolicyProxyBypassList,
@@ -80,11 +81,11 @@ enum PolicyProxyModeType {
   // Disable Proxy, connect directly.
   kPolicyNoProxyServerMode = 0,
   // Auto detect proxy or use specific PAC script if given.
-  kPolicyAutoDetectProxyMode = 1,
+  kPolicyAutoDetectProxyServerMode = 1,
   // Use manually configured proxy servers (fixed servers).
-  kPolicyManuallyConfiguredProxyMode = 2,
+  kPolicyManuallyConfiguredProxyServerMode = 2,
   // Use system proxy server.
-  kPolicyUseSystemProxyMode = 3,
+  kPolicyUseSystemProxyServerMode = 3,
 
   MODE_COUNT
 };