Converted IncognitoForced boolean policy into IncognitoModeAvailability enum policy.

chrome/browser/policy/configuration_policy_pref_store_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/policy/configuration_policy_pref_store.h"
 #include "chrome/browser/policy/mock_configuration_policy_provider.h"
+#include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_store_observer_mock.h"
 #include "content/common/notification_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
 using testing::Mock;
 
@@ skipping 174 matching lines @@
         TypeAndName(kPolicyPasswordManagerEnabled,
                     prefs::kPasswordManagerEnabled),
         TypeAndName(kPolicyPasswordManagerAllowShowPasswords,
                     prefs::kPasswordManagerAllowShowPasswords),
         TypeAndName(kPolicyShowHomeButton,
                     prefs::kShowHomeButton),
         TypeAndName(kPolicyPrintingEnabled,
                     prefs::kPrintingEnabled),
         TypeAndName(kPolicyJavascriptEnabled,
                     prefs::kWebKitJavascriptEnabled),
-        TypeAndName(kPolicyIncognitoEnabled,
-                    prefs::kIncognitoEnabled),
-        TypeAndName(kPolicyIncognitoForced,
-                    prefs::kIncognitoForced),
         TypeAndName(kPolicyRemoteAccessClientFirewallTraversal,
                     prefs::kRemoteAccessClientFirewallTraversal),
         TypeAndName(kPolicyRemoteAccessHostFirewallTraversal,
                     prefs::kRemoteAccessHostFirewallTraversal),
         TypeAndName(kPolicyCloudPrintProxyEnabled,
                     prefs::kCloudPrintProxyEnabled),
         TypeAndName(kPolicySavingBrowserHistoryDisabled,
                     prefs::kSavingBrowserHistoryDisabled),
         TypeAndName(kPolicySavingBrowserHistoryDisabled,
                     prefs::kSavingBrowserHistoryDisabled),
@@ skipping 473 matching lines @@
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store->GetValue(prefs::kDefaultSearchProviderKeyword, NULL));
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store->GetValue(prefs::kDefaultSearchProviderSuggestURL, NULL));
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store->GetValue(prefs::kDefaultSearchProviderIconURL, NULL));
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store->GetValue(prefs::kDefaultSearchProviderEncodings, NULL));
 }
 
+// Tests Incognito mode availability preference setting.
+class ConfigurationPolicyPrefStoreIncognitoModeTest : public testing::Test {
+ protected:
+  void SetPolicies(bool incognito_enabled, int availability) {
+    provider_.AddPolicy(kPolicyIncognitoEnabled,
+                        Value::CreateBooleanValue(incognito_enabled));
+    if (availability >= 0)
+      provider_.AddPolicy(kPolicyIncognitoModeAvailability,
+                          Value::CreateIntegerValue(availability));
+    store_ = new ConfigurationPolicyPrefStore(&provider_);
+  }

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

newline

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

Done.

+  void VerifyValues(IncognitoModePrefs::Availability availability) {
+    const Value* value = NULL;
+    EXPECT_EQ(PrefStore::READ_OK,
+              store_->GetValue(prefs::kIncognitoModeAvailability, &value));
+    EXPECT_TRUE(Value::CreateIntegerValue(availability)->Equals(value));

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

You are leaking the created Value here. Just use
FundamentalValue(availability).Equals(value)

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

Nuts! Chrome tests do not link in heap checker?!

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

We do have valgrind and the tcmalloc heap checker. They run on the continuous
build and for local testing, see tools/heapcheck/chrome_tests.sh and
tools/valgrind/chrome_tests.sh

[rustema, 2011/08/04 07:00:49]

Cool! Thanks for the pointers!

+  }

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

newline

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

Done.

+  MockConfigurationPolicyProvider provider_;
+  scoped_refptr<ConfigurationPolicyPrefStore> store_;
+};
+
+// The following three testcases verify that if the obsolete IncognitoEnabled
+// policy is set to true, the IncognitoModeAvailability values should be copied
+// from policy to pref "as is".

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

Shouldn't it rather check that if the new-style policy is set, the old value
gets ignored?

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

Updated.

+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       DefaultObsoletePolicyAndIncognitoEnabled) {
+  SetPolicies(true, IncognitoModePrefs::ENABLED);
+  VerifyValues(IncognitoModePrefs::ENABLED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       DefaultObsoletePolicyAndIncognitoDisabled) {
+  SetPolicies(true, IncognitoModePrefs::DISABLED);
+  VerifyValues(IncognitoModePrefs::DISABLED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       DefaultObsoletePolicyAndIncognitoForced) {
+  SetPolicies(true, IncognitoModePrefs::FORCED);
+  VerifyValues(IncognitoModePrefs::FORCED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       DefaultObsoletePolicyAndIncognitoAvailabilityNotSet) {
+  SetPolicies(true, -1);
+    const Value* value = NULL;
+  EXPECT_EQ(PrefStore::READ_NO_VALUE,

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

In this case, it should return READ_OK with IncognitoModePrefs::ENABLED.

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

Nope, -1 here means no IncognitoModeAvailability specified. I redefined it as a
constant.

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

I was actually referring to the fact that you set kPolicyIncognitoEnabled to
true. That should reflect as READ_OK with IncognitoModePrefs::ENABLED.

[rustema, 2011/08/04 07:00:49]

Yep, ObsoletePolicySetsPreferenceToEnabled verifies that.

+            store_->GetValue(prefs::kIncognitoModeAvailability, &value));
+}
+
+// Checks that if the obsolete IncognitoEnabled policy is set to false,
+// IncognitoModeAvailability is overridden to DISABLED only in case it's
+// originally was not specified.

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

grammar

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

Updated.

+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       ObsoletePolicyDoesNotAffectAvailabilityEnabled) {
+  SetPolicies(false, IncognitoModePrefs::ENABLED);
+  VerifyValues(IncognitoModePrefs::ENABLED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       ObsoletePolicyDoesNotAffectAvailabilityDisabled) {
+  SetPolicies(false, IncognitoModePrefs::DISABLED);
+  VerifyValues(IncognitoModePrefs::DISABLED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       ObsoletePolicyDoesNotAffectAvailabilityForced) {
+  SetPolicies(false, IncognitoModePrefs::FORCED);
+  VerifyValues(IncognitoModePrefs::FORCED);
+}
+
+TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest,
+       DefaultObsoletePolicyDisablesAvailabilityWhenLatterNotSpecified) {

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

If the policy isn't specified correctly, shouldn't we rather not set the pref at
all?

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

-1 means "not specified".

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

Right, sorry for the confusion.

+  SetPolicies(false, -1);
+  VerifyValues(IncognitoModePrefs::DISABLED);
+}
+
 // Test cases for the Sync policy setting.
 class ConfigurationPolicyPrefStoreSyncTest
     : public ConfigurationPolicyPrefStoreTestBase<testing::Test> {
 };
 
 TEST_F(ConfigurationPolicyPrefStoreSyncTest, Default) {
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store_->GetValue(prefs::kSyncManaged, NULL));
 }
 
@@ skipping 161 matching lines @@
 
   provider_.SetInitializationComplete(true);
   EXPECT_FALSE(store_->IsInitializationComplete());
 
   store_->OnUpdatePolicy();
   Mock::VerifyAndClearExpectations(&observer_);
   EXPECT_TRUE(store_->IsInitializationComplete());
 }
 
 }  // namespace policy