[uber] Redoing the homepage selection UI

chrome/browser/prefs/session_startup_pref.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 "chrome/browser/prefs/session_startup_pref.h"
 
 #include <string>
 
 #include "base/string_piece.h"
+#include "base/values.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 
 #ifdef OS_MACOSX
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/ui/cocoa/window_restore_utils.h"
 #endif
 
 namespace {
 
 // For historical reasons the enum and value registered in the prefs don't line
 // up. These are the values registered in prefs.
-const int kPrefValueDefault = 0;
+
+// kPrefValueHomePage is deprecated, and means the user wants to load the
+// homepage on startup. If this is set, we automatically migrate the to
+// SessionStartupPref::URLS where the list of URLs consists of the user's
+// homepage only.
+const int kPrefValueHomePage = 0;

[csilv, 2012/02/01 20:37:29]

Let's save this longer explanation for where the prefs migration is done.  All
we need here is a // Deprecated command at the end of line 33.

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Done.

 const int kPrefValueLast = 1;
 const int kPrefValueURLs = 4;
+const int kPrefValueNewTab = 5;
 
 // Converts a SessionStartupPref::Type to an integer written to prefs.
 int TypeToPrefValue(SessionStartupPref::Type type) {
   switch (type) {
-    case SessionStartupPref::LAST:   return kPrefValueLast;
-    case SessionStartupPref::URLS:   return kPrefValueURLs;
-    default:                         return kPrefValueDefault;
+    case SessionStartupPref::HOMEPAGE: return kPrefValueHomePage;

[csilv, 2012/02/01 20:37:29]

This line is unneeded since we won't ever write this value.

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Done.

+    case SessionStartupPref::LAST:     return kPrefValueLast;
+    case SessionStartupPref::URLS:     return kPrefValueURLs;
+    default:                           return kPrefValueNewTab;
   }
 }
 
 // Converts an integer pref value to a SessionStartupPref::Type.
 SessionStartupPref::Type PrefValueToType(int pref_value) {
   switch (pref_value) {
-    case kPrefValueLast:  return SessionStartupPref::LAST;
-    case kPrefValueURLs:  return SessionStartupPref::URLS;
-    default:              return SessionStartupPref::DEFAULT;
+    case kPrefValueLast:     return SessionStartupPref::LAST;
+    case kPrefValueURLs:     return SessionStartupPref::URLS;
+    case kPrefValueHomePage: return SessionStartupPref::HOMEPAGE;
+    default:                 return SessionStartupPref::NEWTAB;
   }
 }
 
+// Sets the list of URLs to display at startup to a list consisting of only the
+// user's home page.
+void SetNewUrlList(PrefService * prefs) {

[csilv, 2012/02/01 20:37:29]

get rid of space before '*'

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Done.

+  ListValue* new_url_pref_list = new ListValue();

[csilv, 2012/02/01 20:37:29]

This leaks.  I would recommend instead:
  ListValue new_url_pref_list;

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Done.

+  StringValue* homePage = new StringValue(prefs->GetString(prefs::kHomePage));
+  new_url_pref_list->Append(static_cast<Value*>(homePage));

[csilv, 2012/02/01 20:37:29]

I don't think you need the static_cast here

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Thanks, I think I was misreading the compiler errors. Done.

+  const ListValue* const_list = const_cast<ListValue*>(new_url_pref_list);
+  prefs->Set(prefs::kURLsToRestoreOnStartup, *const_list);

[csilv, 2012/02/01 20:37:29]

kill line 64, change 64 to:
  prefs->Set(prefs::kURLsToRestoreOnStartup, new_url_pref_list);

[Tyler Breisacher (Chromium), 2012/02/01 21:49:38]

Done.

+}
+

[csilv, 2012/02/01 20:37:29]

After making the above changes, the function may be trivial enough to just
incorporate into GetStartupPref()

 }  // namespace
 
 // static
 void SessionStartupPref::RegisterUserPrefs(PrefService* prefs) {
   SessionStartupPref::Type type = browser_defaults::kDefaultSessionStartupType;
 
 #ifdef OS_MACOSX
   // During first run the calling code relies on |DEFAULT| session preference
   // value to avoid session restore.  That is respected here.
   if (!first_run::IsChromeFirstRun()) {
@@ skipping 49 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 (pref.type == SessionStartupPref::HOMEPAGE) {
+    prefs->SetInteger(prefs::kRestoreOnStartup, kPrefValueURLs);
+    pref.type = SessionStartupPref::URLS;
+    SetNewUrlList(prefs);
+  }
+
   // 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_pref_list = prefs->GetList(
       prefs::kURLsToRestoreOnStartup);
   if (url_pref_list) {
     for (size_t i = 0; i < url_pref_list->GetSize(); ++i) {
       Value* value = NULL;
       if (url_pref_list->Get(i, &value)) {
         std::string url_text;
         if (value->GetAsString(&url_text)) {
@@ skipping 17 matching lines @@
 
 // static
 bool SessionStartupPref::URLsAreManaged(PrefService* prefs) {
   DCHECK(prefs);
   const PrefService::Preference* pref_urls =
       prefs->FindPreference(prefs::kURLsToRestoreOnStartup);
   DCHECK(pref_urls);
   return pref_urls->IsManaged();
 }
 
-SessionStartupPref::SessionStartupPref() : type(DEFAULT) {}
-
 SessionStartupPref::SessionStartupPref(Type type) : type(type) {}
 
 SessionStartupPref::~SessionStartupPref() {}