[uber] Redoing the homepage selection UI

chrome/browser/prefs/session_startup_pref.cc

Index: chrome/browser/prefs/session_startup_pref.cc
diff --git a/chrome/browser/prefs/session_startup_pref.cc b/chrome/browser/prefs/session_startup_pref.cc
index a441a4ccbdc402a0268e94b17b42a130cb82d222..658f32733da09c4824942cc7062c4d02ccd3459d 100644
--- a/chrome/browser/prefs/session_startup_pref.cc
+++ b/chrome/browser/prefs/session_startup_pref.cc
@@ -7,6 +7,7 @@
 #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"
@@ -24,28 +25,46 @@ 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
@@ -115,6 +134,13 @@ SessionStartupPref SessionStartupPref::GetStartupPref(PrefService* 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(
@@ -152,8 +178,6 @@ bool SessionStartupPref::URLsAreManaged(PrefService* prefs) {
   return pref_urls->IsManaged();
 }
 
-SessionStartupPref::SessionStartupPref() : type(DEFAULT) {}
-
 SessionStartupPref::SessionStartupPref(Type type) : type(type) {}
 
 SessionStartupPref::~SessionStartupPref() {}