Implement a policy to specify which pages to restore at startup. The admin can choose no restore, last opened windows, or specific pages to be opened.

xib changes: Added an enabled binding to the policy-controlled "Restore on Startup" radiobox group in the "Basic" preference pane.  They can be disabled when their value is overridden by policy. The methods for the bindings are in the preference window controller.

BUG=49297
TEST=ConfigurationPolicyPrefStoreTest.* and ConfigurationPolicyProviderWinTest.*  Manual tests: Set the policies externally.  Check that appropriate restore action is done at startup.  Check that the Restore on Startup buttons are disabled.

Review URL: http://codereview.chromium.org/3115021
Patch from Jean-Luc Brouillet <jeanluc@google.com>.

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

7 files changed, 106 insertions, 33 deletions

diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index b702ca8..b78a56b 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -91,6 +91,10 @@ const ConfigurationPolicyPrefStore::PolicyToPreferenceMapEntry
   { Value::TYPE_STRING, kPolicyHomePage,  prefs::kHomePage },
   { Value::TYPE_BOOLEAN, kPolicyHomepageIsNewTabPage,
       prefs::kHomePageIsNewTabPage },
+  { Value::TYPE_INTEGER, kPolicyRestoreOnStartup,
+      prefs::kRestoreOnStartup},
+  { Value::TYPE_LIST, kPolicyURLsToRestoreOnStartup,
+      prefs::kURLsToRestoreOnStartup },
   { Value::TYPE_BOOLEAN, kPolicyAlternateErrorPagesEnabled,
       prefs::kAlternateErrorPagesEnabled },
   { Value::TYPE_BOOLEAN, kPolicySearchSuggestEnabled,
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index 3f74266..986cf4d 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -19,6 +19,17 @@ class ConfigurationPolicyPrefStoreTest : public testing::Test {
       ConfigurationPolicyStore::PolicyType type,
       const char* policy_value);
 
+  // The following three methods test a policy which controls a preference
+  // that is a list of strings.
+  // Checks that by default, it's an empty list.
+  void TestListPolicyGetDefault(const char* pref_name);
+  // Checks that values can be set.
+  void TestListPolicySetValue(const char* pref_name,
+                              ConfigurationPolicyStore::PolicyType type);
+  // A wrapper that calls the above two methods.
+  void TestListPolicy(const char* pref_name,
+                      ConfigurationPolicyStore::PolicyType type);
+
   // The following three methods test a policy which controls a string
   // preference.
   // Checks that by default, it's an empty string.
@@ -60,6 +71,42 @@ void ConfigurationPolicyPrefStoreTest::ApplyStringPolicyValue(
   store->Apply(type, Value::CreateStringValue(policy_value));
 }
 
+void ConfigurationPolicyPrefStoreTest::TestListPolicyGetDefault(
+    const char* pref_name) {
+  ConfigurationPolicyPrefStore store(NULL, NULL);
+  ListValue* list = NULL;
+  EXPECT_FALSE(store.prefs()->GetList(pref_name, &list));
+}
+
+void ConfigurationPolicyPrefStoreTest::TestListPolicySetValue(
+    const char* pref_name, ConfigurationPolicyStore::PolicyType type) {
+  ConfigurationPolicyPrefStore store(NULL, NULL);
+  ListValue* in_value = new ListValue();
+  in_value->Append(Value::CreateStringValue("test1"));
+  in_value->Append(Value::CreateStringValue("test2,"));
+  store.Apply(type, in_value);
+  ListValue* list = NULL;
+  EXPECT_TRUE(store.prefs()->GetList(pref_name, &list));
+  ListValue::const_iterator current(list->begin());
+  ListValue::const_iterator end(list->end());
+  ASSERT_TRUE(current != end);
+  std::string value;
+  (*current)->GetAsString(&value);
+  EXPECT_EQ("test1", value);
+  ++current;
+  ASSERT_TRUE(current != end);
+  (*current)->GetAsString(&value);
+  EXPECT_EQ("test2,", value);
+  ++current;
+  EXPECT_TRUE(current == end);
+}
+
+void ConfigurationPolicyPrefStoreTest::TestListPolicy(
+    const char* pref_name, ConfigurationPolicyStore::PolicyType type) {
+  TestListPolicyGetDefault(pref_name);
+  TestListPolicySetValue(pref_name, type);
+}
+
 void ConfigurationPolicyPrefStoreTest::TestStringPolicyGetDefault(
     const char* pref_name) {
   ConfigurationPolicyPrefStore store(NULL, NULL);
@@ -147,6 +194,13 @@ TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyHomepageIsNewTabPage) {
       ConfigurationPolicyPrefStore::kPolicyHomepageIsNewTabPage);
 }
 
+TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyRestoreOnStartup) {
+  TestIntegerPolicy(prefs::kRestoreOnStartup,
+      ConfigurationPolicyPrefStore::kPolicyRestoreOnStartup);
+  TestListPolicy(prefs::kURLsToRestoreOnStartup,
+      ConfigurationPolicyPrefStore::kPolicyURLsToRestoreOnStartup);
+}
+
 TEST_F(ConfigurationPolicyPrefStoreTest, TestPolicyAlternateErrorPagesEnabled) {
   TestBooleanPolicy(prefs::kAlternateErrorPagesEnabled,
       ConfigurationPolicyStore::kPolicyAlternateErrorPagesEnabled);
diff --git a/chrome/browser/policy/configuration_policy_provider.cc b/chrome/browser/policy/configuration_policy_provider.cc
index d24196b..342c011 100644
--- a/chrome/browser/policy/configuration_policy_provider.cc
+++ b/chrome/browser/policy/configuration_policy_provider.cc
@@ -24,6 +24,10 @@ const InternalPolicyValueMapEntry kPolicyValueMap[] = {
       Value::TYPE_STRING, policy::key::kHomepageLocation },
   { ConfigurationPolicyStore::kPolicyHomepageIsNewTabPage,
       Value::TYPE_BOOLEAN, policy::key::kHomepageIsNewTabPage },
+  { ConfigurationPolicyStore::kPolicyRestoreOnStartup,
+      Value::TYPE_INTEGER, policy::key::kRestoreOnStartup },
+  { ConfigurationPolicyStore::kPolicyURLsToRestoreOnStartup,
+      Value::TYPE_LIST, policy::key::kURLsToRestoreOnStartup },
   { ConfigurationPolicyStore::kPolicyProxyServerMode,
       Value::TYPE_INTEGER, policy::key::kProxyServerMode },
   { ConfigurationPolicyStore::kPolicyProxyServer,
diff --git a/chrome/browser/policy/configuration_policy_provider_win.cc b/chrome/browser/policy/configuration_policy_provider_win.cc
index 2e5904e..5d79da6 100644
--- a/chrome/browser/policy/configuration_policy_provider_win.cc
+++ b/chrome/browser/policy/configuration_policy_provider_win.cc
@@ -58,31 +58,29 @@ ConfigurationPolicyProviderWin::ConfigurationPolicyProviderWin() {
 }
 
 bool ConfigurationPolicyProviderWin::GetRegistryPolicyString(
-    const string16& name, int index, string16* result) {
-  DWORD value_size = 0;
-  DWORD key_type = 0;
-  scoped_array<uint8> buffer;
+    const string16& name, string16* result) {
+  string16 path = string16(policy::kRegistrySubKey);
   RegKey policy_key;
-  string16 location = string16(policy::kRegistrySubKey);
-  string16 value_name = name;
-
-  if (index > 0) {
-    // This is a list value, treat |name| as a subkey.
-    location += ASCIIToUTF16("\\") + name;
-    value_name = base::IntToString16(index);
-  }
-
   // First try the global policy.
-  if (!policy_key.Open(HKEY_LOCAL_MACHINE, location.c_str()) ||
-      !policy_key.ReadValue(value_name.c_str(), 0, &value_size, &key_type)) {
+  if (policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str())) {
+    if (ReadRegistryStringValue(&policy_key, name, result))
+      return true;
     policy_key.Close();
-    // Fall back on user-specific policy.
-    if (!policy_key.Open(HKEY_CURRENT_USER, location.c_str()) ||
-        !policy_key.ReadValue(value_name.c_str(), 0, &value_size, &key_type)) {
-      return false;
-    }
   }
+  // Fall back on user-specific policy.
+  if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str()))
+    return false;
+  return ReadRegistryStringValue(&policy_key, name, result);
+}
 
+bool ConfigurationPolicyProviderWin::ReadRegistryStringValue(
+    RegKey* key, const string16& name, string16* result) {
+  DWORD value_size = 0;
+  DWORD key_type = 0;
+  scoped_array<uint8> buffer;
+
+  if (!key->ReadValue(name.c_str(), 0, &value_size, &key_type))
+    return false;
   if (key_type != REG_SZ)
     return false;
 
@@ -92,18 +90,28 @@ bool ConfigurationPolicyProviderWin::GetRegistryPolicyString(
   // the 0-termination.
   buffer.reset(new uint8[value_size + 2]);
   memset(buffer.get(), 0, value_size + 2);
-  policy_key.ReadValue(value_name.c_str(), buffer.get(), &value_size);
+  key->ReadValue(name.c_str(), buffer.get(), &value_size);
   result->assign(reinterpret_cast<const wchar_t*>(buffer.get()));
   return true;
 }
 
-
 bool ConfigurationPolicyProviderWin::GetRegistryPolicyStringList(
     const string16& key, ListValue* result) {
-  int index = 0;
+  string16 path = string16(policy::kRegistrySubKey);
+  path += ASCIIToUTF16("\\") + key;
+  RegKey policy_key;
+  if (!policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str())) {
+    policy_key.Close();
+    // Fall back on user-specific policy.
+    if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str()))
+      return false;
+  }
   string16 policy_string;
-  while (GetRegistryPolicyString(key, ++index,  &policy_string))
+  int index = 0;
+  while (ReadRegistryStringValue(&policy_key, base::IntToString16(++index),
+                                 &policy_string)) {
     result->Append(Value::CreateStringValue(policy_string));
+  }
   return true;
 }
 
@@ -151,7 +159,7 @@ bool ConfigurationPolicyProviderWin::Provide(
     switch (current->value_type) {
       case Value::TYPE_STRING: {
         std::wstring string_value;
-        if (GetRegistryPolicyString(name.c_str(), -1, &string_value)) {
+        if (GetRegistryPolicyString(name.c_str(), &string_value)) {
           store->Apply(current->policy_type,
                        Value::CreateStringValue(string_value));
         }
diff --git a/chrome/browser/policy/configuration_policy_provider_win.h b/chrome/browser/policy/configuration_policy_provider_win.h
index e197e9d..59a2770 100644
--- a/chrome/browser/policy/configuration_policy_provider_win.h
+++ b/chrome/browser/policy/configuration_policy_provider_win.h
@@ -12,6 +12,8 @@
 #include "chrome/browser/policy/configuration_policy_store.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 
+class RegKey;
+
 // An implementation of |ConfigurationPolicyProvider| using the
 // mechanism provided by Windows Groups Policy. Policy decisions are
 // stored as values in a special section of the Windows Registry.
@@ -54,15 +56,12 @@ class ConfigurationPolicyProviderWin : public ConfigurationPolicyProvider {
   // Methods to perform type-specific policy lookups in the registry.
   // HKLM is checked first, then HKCU.
 
-  // Reads a string registry value |name| and puts the resulting string in
-  // |result|. If |index| > 0, |name| is the name of a subkey and the value
-  // read is named |index|. Note: A subkey is used for lists to work around
-  // a problem with the Group Policy Editor, where one list value overwrites
-  // another if they appear under the same key (even if they have different
-  // names).
-  bool GetRegistryPolicyString(const string16& name,
-                               int index,
+  // Reads a string registry value |name| at the specified |key| and puts the
+  // resulting string in |result|.
+  bool ReadRegistryStringValue(RegKey* key, const string16& name,
                                string16* result);
+
+  bool GetRegistryPolicyString(const string16& name, string16* result);
   // Gets a list value contained under |key| one level below the policy root.
   bool GetRegistryPolicyStringList(const string16& key, ListValue* result);
   bool GetRegistryPolicyBoolean(const string16& value_name, bool* result);
diff --git a/chrome/browser/policy/configuration_policy_store.h b/chrome/browser/policy/configuration_policy_store.h
index e74fe6c..2612c4d 100644
--- a/chrome/browser/policy/configuration_policy_store.h
+++ b/chrome/browser/policy/configuration_policy_store.h
@@ -20,6 +20,8 @@ class ConfigurationPolicyStore {
   enum PolicyType {
     kPolicyHomePage,
     kPolicyHomepageIsNewTabPage,
+    kPolicyRestoreOnStartup,
+    kPolicyURLsToRestoreOnStartup,
     kPolicyProxyServerMode,
     kPolicyProxyServer,
     kPolicyProxyPacUrl,
diff --git a/chrome/browser/policy/managed_prefs_banner_base.cc b/chrome/browser/policy/managed_prefs_banner_base.cc
index fc334f8..b9d4d83 100644
--- a/chrome/browser/policy/managed_prefs_banner_base.cc
+++ b/chrome/browser/policy/managed_prefs_banner_base.cc
@@ -52,6 +52,8 @@ void ManagedPrefsBannerBase::Init(PrefService* local_state,
       AddUserPref(prefs::kHomePage);
       AddUserPref(prefs::kHomePageIsNewTabPage);
       AddUserPref(prefs::kShowHomeButton);
+      AddUserPref(prefs::kRestoreOnStartup);
+      AddUserPref(prefs::kURLsToRestoreOnStartup);
       break;
     case OPTIONS_PAGE_CONTENT:
       AddUserPref(prefs::kSyncManaged);