Converted IncognitoForced boolean policy into IncognitoModeAvailability enum policy.

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 3f74a0137462f5aa2b69d064884410bdb46f64ba..8ccbf37447dc048abea7b8ab1994f7e742f1dc7d 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 #include "chrome/browser/policy/policy_path_parser.h"
+#include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prefs/pref_value_map.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/browser/search_engines/search_terms_data.h"
@@ -106,6 +107,11 @@ class ConfigurationPolicyPrefKeeper
   // ApplyDefaultSearchPolicy takes ownership of |value|.
   bool ApplyDefaultSearchPolicy(ConfigurationPolicyType policy, Value* value);
 
+  // Processes incognito mode availability related policies. Returns true if the
+  // specified policy is pertinent to incognito mode availability. In that case,
+  // the function takes ownership of |value|.
+  bool ApplyIncognitoModePolicy(ConfigurationPolicyType policy, Value* value);
+
   // Make sure that the |path| if present in |prefs_|.  If not, set it to
   // a blank string.
   void EnsureStringPrefExists(const std::string& path);
@@ -121,6 +127,11 @@ class ConfigurationPolicyPrefKeeper
   // respective values in |prefs_|.
   void FinalizeProxyPolicySettings();
 
+  // If the required entries for the Incognito mode availability settings
+  // are specified and valid, finalizes the policy-specified configuration
+  // by initializing the respective values in |prefs_|.
+  void FinalizeIncognitoModeSettings();
+
   // 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.
@@ -136,6 +147,11 @@ class ConfigurationPolicyPrefKeeper
   // is called.
   std::map<ConfigurationPolicyType, Value*> proxy_policies_;
 
+  // Saved state of the deprecated kPolicyIncognitoEnabled. If is still used for

[Mattias Nissler (ping if slow), 2011/08/01 11:13:40]

s/If/It/

[rustema, 2011/08/01 23:55:33]

Done.

+  // backward compatibility to set the new kIncognitoAvailabilityMode pref in
+  // case the corresponding policy for the latter is not specified.
+  bool deprecated_incognito_enabled_;
+
   PrefValueMap prefs_;
 
   static const PolicyToPreferenceMapEntry kSimplePolicyMap[];
@@ -191,10 +207,6 @@ const ConfigurationPolicyPrefKeeper::PolicyToPreferenceMapEntry
     prefs::kShowHomeButton },
   { Value::TYPE_BOOLEAN, kPolicyJavascriptEnabled,
     prefs::kWebKitJavascriptEnabled },
-  { Value::TYPE_BOOLEAN, kPolicyIncognitoEnabled,
-    prefs::kIncognitoEnabled },
-  { Value::TYPE_BOOLEAN, kPolicyIncognitoForced,
-    prefs::kIncognitoForced },
   { Value::TYPE_BOOLEAN, kPolicySavingBrowserHistoryDisabled,
     prefs::kSavingBrowserHistoryDisabled },
   { Value::TYPE_BOOLEAN, kPolicyClearSiteDataOnExit,
@@ -313,10 +325,12 @@ const ConfigurationPolicyPrefKeeper::PolicyToPreferenceMapEntry
 
 ConfigurationPolicyPrefKeeper::ConfigurationPolicyPrefKeeper(
     ConfigurationPolicyProvider* provider) {
+  deprecated_incognito_enabled_ = true;
   if (!provider->Provide(this))
     LOG(WARNING) << "Failed to get policy from provider.";
   FinalizeProxyPolicySettings();
   FinalizeDefaultSearchPolicySettings();
+  FinalizeIncognitoModeSettings();
 }
 
 ConfigurationPolicyPrefKeeper::~ConfigurationPolicyPrefKeeper() {
@@ -369,6 +383,9 @@ void ConfigurationPolicyPrefKeeper::Apply(ConfigurationPolicyType policy,
   if (ApplyDefaultSearchPolicy(policy, value))
     return;
 
+  if (ApplyIncognitoModePolicy(policy, value))
+    return;
+
   if (ApplyPolicyFromMap(policy, value, kSimplePolicyMap,
                          arraysize(kSimplePolicyMap)))
     return;
@@ -556,6 +573,32 @@ bool ConfigurationPolicyPrefKeeper::ApplyDefaultSearchPolicy(
   return false;
 }
 
+bool ConfigurationPolicyPrefKeeper::ApplyIncognitoModePolicy(
+    ConfigurationPolicyType policy,
+    Value* value) {
+  if (policy == kPolicyIncognitoModeAvailability) {
+    int availability = IncognitoModePrefs::ENABLED;
+    bool result = value->GetAsInteger(&availability);
+    DCHECK(result);
+    delete value;
+    IncognitoModePrefs::Availability availability_enum_value;
+    result = IncognitoModePrefs::IntToAvailability(availability,
+                                                   &availability_enum_value);
+    DCHECK(result);
+    prefs_.SetValue(prefs::kIncognitoModeAvailability,
+                    Value::CreateIntegerValue(availability_enum_value));
+    return true;
+  }
+  if (policy == kPolicyIncognitoEnabled) {

[Mattias Nissler (ping if slow), 2011/08/01 11:13:40]

else if

[rustema, 2011/08/01 23:55:33]

But there's a return statement in the previous if block. I thought having if
blocks with return statements was cleaner. Do you insist that I change it to
"else if"?

[Mattias Nissler (ping if slow), 2011/08/03 09:16:08]

Yes, right, sorry for the noise.

+    bool result = value->GetAsBoolean(&deprecated_incognito_enabled_);
+    DCHECK(result);
+    delete value;
+    return true;
+  }
+  // The policy is not relevant to incognito.
+  return false;
+}
+
 void ConfigurationPolicyPrefKeeper::EnsureStringPrefExists(
     const std::string& path) {
   std::string value;
@@ -637,6 +680,19 @@ void ConfigurationPolicyPrefKeeper::FinalizeDefaultSearchPolicySettings() {
                          arraysize(kDefaultSearchPolicyMap));
 }
 
+void ConfigurationPolicyPrefKeeper::FinalizeIncognitoModeSettings() {
+  int int_value;
+  if (!prefs_.GetInteger(prefs::kIncognitoModeAvailability, &int_value)) {
+    // If kPolicyIncognitoModeAvailability is not specified, check the obsolete
+    // kPolicyIncognitoEnabled.
+    if (!deprecated_incognito_enabled_) {

[Mattias Nissler (ping if slow), 2011/08/01 11:13:40]

It's not correct to treat the cases "not set" and "set to true" the same. If the
legacy policy is set to true, you should set the pref below, but you don't. This
is important s.t. client code asking IsManaged() on the pref will get the right
value.

To accomplish this, I suggest you keep a scoped_ptr<Value> to save the value of
the deprecated policy in instead of the  boolean flag you have at the moment.

[rustema, 2011/08/01 23:55:33]

Done.

+      // If the obsolete policy says that incognito is disabled,
+      prefs_.SetInteger(prefs::kIncognitoModeAvailability,
+                        IncognitoModePrefs::DISABLED);
+    }
+  }
+}
+
 void ConfigurationPolicyPrefKeeper::FinalizeProxyPolicySettings() {
   if (CheckProxySettings())
     ApplyProxySettings();
@@ -999,7 +1055,8 @@ ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList() {
     { kPolicyPrintingEnabled, Value::TYPE_BOOLEAN, key::kPrintingEnabled },
     { kPolicyJavascriptEnabled, Value::TYPE_BOOLEAN, key::kJavascriptEnabled },
     { kPolicyIncognitoEnabled, Value::TYPE_BOOLEAN, key::kIncognitoEnabled },
-    { kPolicyIncognitoForced, Value::TYPE_BOOLEAN, key::kIncognitoForced },
+    { kPolicyIncognitoModeAvailability, Value::TYPE_INTEGER,
+      key::kIncognitoModeAvailability },
     { kPolicySavingBrowserHistoryDisabled, Value::TYPE_BOOLEAN,
       key::kSavingBrowserHistoryDisabled },
     { kPolicyClearSiteDataOnExit, Value::TYPE_BOOLEAN,