Update RestoreOnStartupPolicyHandler to translate to the correct preferences

chrome/browser/policy/configuration_policy_handler.cc

 // Copyright (c) 2012 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 "chrome/browser/policy/configuration_policy_handler.h"
 
 #include <string>
 
 #include "base/file_path.h"
 #include "base/json/json_writer.h"
@@ skipping 948 matching lines @@
 
 RestoreOnStartupPolicyHandler::RestoreOnStartupPolicyHandler()
     : SimplePolicyHandler(key::kRestoreOnStartup,
                           prefs::kRestoreOnStartup,
                           Value::TYPE_INTEGER) {
 }
 
 RestoreOnStartupPolicyHandler::~RestoreOnStartupPolicyHandler() {
 }
 
+void RestoreOnStartupPolicyHandler::ApplyPolicySettings(
+    const PolicyMap& policies,
+    PrefValueMap* prefs) {
+  const Value* restore_on_startup_value = policies.GetValue(policy_name());
+  int restore_on_startup = -1;
+  if (restore_on_startup_value)
+    restore_on_startup_value->GetAsInteger(&restore_on_startup);
+
+  if (restore_on_startup == SessionStartupPref::kPrefValueHomePage) {
+    ApplyPolicySettingsFromHomePage(policies, prefs);
+  } else {
+    SimplePolicyHandler::ApplyPolicySettings(policies, prefs);

[Mattias Nissler (ping if slow), 2012/04/19 13:15:55]

This would be just prefs->SetValue() if you don't derive from
SimplePolicyHandler, which I think would be easier to follow.

[Tyler Breisacher (Chromium), 2012/04/19 19:07:56]

Done.

+  }
+}
+
+void RestoreOnStartupPolicyHandler::ApplyPolicySettingsFromHomePage(
+    const PolicyMap& policies,
+    PrefValueMap* prefs) {
+
+  const base::Value* homepage_is_new_tab_page_value =
+      policies.GetValue(key::kHomepageIsNewTabPage);
+  if (!homepage_is_new_tab_page_value) {
+    // The policy is enforcing 'open the homepage on startup' but not
+    // enforcing what the homepage should be. Don't set any prefs.

[Mattias Nissler (ping if slow), 2012/04/19 13:15:55]

So the policy doesn't have the homepage_is_new_tab_page setting. In that case,
shouldn't we fall back to setting the default (which would be that the homepage
is the new tab page IIRC)?

[Tyler Breisacher (Chromium), 2012/04/19 19:07:56]

In Chrome 18, if a policy has RestoreOnStartup=0 and doesn't have
HomePageIsNewTab set at all, the user would still be free to choose their
homepage. Then presumably the admin setting the policy would notice that and
explicitly set a HomePageIsNewTab value (and probably a HomepageLocation value
too) so this would be a non-issue for most people. But in case they didn't, I
think we probably want the policy to have the same effect it had in the previous
version.

[Mattias Nissler (ping if slow), 2012/04/20 09:20:24]

You're right.

+    return;
+  }
+
+  bool homepage_is_new_tab_page;
+  bool result = homepage_is_new_tab_page_value->GetAsBoolean(
+      &homepage_is_new_tab_page);
+  DCHECK(result);

[Mattias Nissler (ping if slow), 2012/04/19 13:15:55]

This DCHECK is inappropriate. The Value you get here comes directly from the
policy provider, which cannot enforce proper Value types in all cases. Thus, you
should handle this situation gracefully, probably by just ignoring the value.

[Tyler Breisacher (Chromium), 2012/04/19 19:07:56]

Done. (Also did the same thing above for homepage_is_new_tab_page_value)

+
+  if (homepage_is_new_tab_page) {
+    prefs->SetInteger(
+        prefs::kRestoreOnStartup,
+        SessionStartupPref::kPrefValueNewTab);
+  } else {
+    const base::Value* homepage_value =
+        policies.GetValue(key::kHomepageLocation);
+    if (!homepage_value) {
+      // The policy is enforcing 'open the homepage on startup' but not
+      // enforcing what the homepage should be. Don't set any prefs.
+      return;
+    }
+    ListValue* url_list = new ListValue();
+    url_list->Append(homepage_value->DeepCopy());
+    prefs->SetInteger(
+        prefs::kRestoreOnStartup,
+        SessionStartupPref::kPrefValueURLs);
+    prefs->SetValue(prefs::kURLsToRestoreOnStartup, url_list);
+  }
+}
+
 bool RestoreOnStartupPolicyHandler::CheckPolicySettings(
     const PolicyMap& policies,
     PolicyErrorMap* errors) {
   if (!SimplePolicyHandler::CheckPolicySettings(policies, errors))
     return false;
 

[Mattias Nissler (ping if slow), 2012/04/19 13:15:55]

Ideally, you should add a check here that marks the key::kRestoreOnStartup
policy value deprecated. There already is IDS_POLICY_DEPRECATED, but that is for
the case when the entire policy is deprecated, so you might want to add another
constant IDS_POLICY_VALUE_DEPRECATED for the case that the policy is still
appropriate, but the specific value is deprecated.

[Tyler Breisacher (Chromium), 2012/04/19 19:07:56]

Done, but I'm pretty sure we'll want to merge this to the m19 branch. Is it too
late to add new strings?

[Mattias Nissler (ping if slow), 2012/04/20 09:20:24]

Yes, it is too late. I'm OK with merging an adjusted patch that doesn't have the
additional error reporting.

   // If the restore urls at start up policy is set, session cookies are treated
   // as permanent cookies and site data needed to restore the session is not
   // cleared so we have to warn the user in that case.
   const base::Value* restore_policy = policies.GetValue(key::kRestoreOnStartup);
 
   if (restore_policy) {
     int restore_value;
     if (restore_policy->GetAsInteger(&restore_value) &&
-        SessionStartupPref::PrefValueToType(restore_value) ==
-            SessionStartupPref::LAST) {
+        restore_value == SessionStartupPref::kPrefValueLast) {
 
       const base::Value* cookies_policy =
           policies.GetValue(key::kCookiesSessionOnlyForUrls);
       const base::ListValue *cookies_value;
       if (cookies_policy && cookies_policy->GetAsList(&cookies_value) &&
           !cookies_value->empty()) {
         errors->AddError(key::kCookiesSessionOnlyForUrls,
                          IDS_POLICY_OVERRIDDEN,
                          key::kRestoreOnStartup);
       }
 
       const base::Value* exit_policy =
           policies.GetValue(key::kClearSiteDataOnExit);
       bool exit_value;
       if (exit_policy && exit_policy->GetAsBoolean(&exit_value) && exit_value) {
         errors->AddError(key::kClearSiteDataOnExit,
                          IDS_POLICY_OVERRIDDEN,
                          key::kRestoreOnStartup);
       }
     }
   }
   return true;
 }
 
 }  // namespace policy