Fix homepage migration for users who never changed their settings.

chrome/browser/prefs/session_startup_pref.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/prefs/session_startup_pref.h"
 
 #include <string>
 
 #include "base/values.h"
 #include "chrome/browser/net/url_fixer_upper.h"
@@ skipping 114 matching lines @@
   DCHECK(profile);
   return GetStartupPref(profile->GetPrefs());
 }
 
 // static
 SessionStartupPref SessionStartupPref::GetStartupPref(PrefService* prefs) {
   DCHECK(prefs);
   SessionStartupPref pref(
       PrefValueToType(prefs->GetInteger(prefs::kRestoreOnStartup)));
 
-  // Migrate from "Open the home page" to "Open the following URLs". If the user
-  // had the "Open the homepage" option selected, then we need to switch them to
-  // "Open the following URLs" and set the list of URLs to a list containing
-  // just the user's homepage.
-  if (pref.type == SessionStartupPref::HOMEPAGE) {
+  // Migrate from "Open the home page" to "Open the following URLs". If the
+  // user had the "Open the home page" option selected, switch them to "Open
+  // the following URLs" and set the list of URLs to a one-item list containing
+  // their homepage. If the kRestoreOnStartup pref is blank, it could be a user
+  // coming from Chrome 18 or lower, who had "open the homepage" selected
+  // (since that was the default) or it could be a user who originally
+  // installed Chrome 19 or higher. We distinguish between these cases using
+  // the kHomePageIsNewTabPage pref: A Chrome 18 user wanting to open a
+  // homepage other than the NTP on startup would have set
+  // kHomePageIsNewTabPage to false. If kHomePageIsNewTabPage is true, there is
+  // no need to do the migration; the new default behavior is to open the NTP.
+  if (pref.type == SessionStartupPref::HOMEPAGE ||
+      (!prefs->HasPrefPath(prefs::kRestoreOnStartup) &&
+       !prefs->GetBoolean(prefs::kHomePageIsNewTabPage))) {

[Mattias Nissler (ping if slow), 2012/04/16 09:21:46]

This is likely to break in case there is policy present that affects these
prefs. For example, if policy forces prefs::kRestoreOnStartup to HOMEPAGE,
you'll run this migration code every time, because the
SetInteger(prefs::kRestoreOnStartup) below will be masked by policy.

     prefs->SetInteger(prefs::kRestoreOnStartup, kPrefValueURLs);
     pref.type = SessionStartupPref::URLS;
     SetNewURLList(prefs);
+  } else if (!prefs->HasPrefPath(prefs::kRestoreOnStartup)) {

[Mattias Nissler (ping if slow), 2012/04/16 09:21:46]

Setting a pref to the default is bad. This breaks situations where the user
hasn't made any decision and the admin wants to change the default through
recommended policy (which is lower priority than user prefs, so your forced
default will mask the admin-provided default).

+    // If we didn't do the migration, but the kRestoreOnStartup pref is blank,
+    // then set it explicitly to the default value, so that we won't do the
+    // migration the next time this function is called.
+    prefs->SetInteger(prefs::kRestoreOnStartup,
+                      TypeToPrefValue(GetDefaultStartupType()));
   }
 
   // Always load the urls, even if the pref type isn't URLS. This way the
   // preferences panels can show the user their last choice.
   const ListValue* url_list = prefs->GetList(prefs::kURLsToRestoreOnStartup);
   URLListToPref(url_list, &pref);
 
   return pref;
 }
 
@@ skipping 56 matching lines @@
   CHECK(prefs_watcher->GetBackupForPref(
             prefs::kURLsToRestoreOnStartup)->GetAsList(&url_list));
   URLListToPref(url_list, &backup_pref);
 
   return backup_pref;
 }
 
 SessionStartupPref::SessionStartupPref(Type type) : type(type) {}
 
 SessionStartupPref::~SessionStartupPref() {}