First part of the pre-set bookmarks feature

- master prefs format (just url)
- defer actual bookmark fu, because early on the bookmarkmodel and other services are not yet running.
- cleanup master prefs test, too much code duplication
- cleanup ProcessMasterPreferences it had too many params

BUG=32728
TEST=unit test included

Review URL: http://codereview.chromium.org/660116

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

7 files changed, 148 insertions, 78 deletions

diff --git a/chrome/browser/browser_init.h b/chrome/browser/browser_init.h
index c427741..28653f1 100644
--- a/chrome/browser/browser_init.h
+++ b/chrome/browser/browser_init.h
@@ -31,6 +31,13 @@ class BrowserInit {
     first_run_tabs_.push_back(url);
   }
 
+  // Adds a url that will become a bookmark during first run, just as if the
+  // user had done it. A network request is generated. Icon and title will
+  // be provided by the server.
+  void AddDefaultBookmark(const GURL& url) {
+    default_bookmarks_.push_back(url);
+  }
+
   // This function is equivalent to ProcessCommandLine but should only be
   // called during actual process startup.
   bool Start(const CommandLine& cmd_line, const std::wstring& cur_dir,
@@ -150,6 +157,8 @@ class BrowserInit {
 
   // Additional tabs to open during first run.
   std::vector<GURL> first_run_tabs_;
+  // Additional bookmarks to set during first run.
+  std::vector<GURL> default_bookmarks_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserInit);
 };
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index a74c1d2..5814816 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -310,11 +310,25 @@ void AddFirstRunNewTabs(BrowserInit* browser_init,
       browser_init->AddFirstRunTab(url);
   }
 }
+
+void AddDefaultBookmarks(BrowserInit* browser_init,
+                         const std::vector<std::wstring>& bookmarks) {
+  for (std::vector<std::wstring>::const_iterator it = bookmarks.begin();
+       it != bookmarks.end(); ++it) {
+    GURL url(*it);
+    if (url.is_valid())
+      browser_init->AddDefaultBookmark(url);
+  }
+}
 #else
 // TODO(cpu): implement first run experience for other platforms.
 void AddFirstRunNewTabs(BrowserInit* browser_init,
                         const std::vector<std::wstring>& new_tabs) {
 }
+
+void AddDefaultBookmarks(BrowserInit* browser_init,
+                         const std::vector<std::wstring>& bookmarks) {
+}
 #endif
 
 #if defined(USE_LINUX_BREAKPAD)
@@ -606,25 +620,22 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   BrowserInit browser_init;
 
-#if defined(OS_WIN)
-  int rlz_ping_delay = 0;
-#endif
-  bool homepage_defined = false;
-  int import_items = 0;
-  int dont_import_items = 0;
+  FirstRun::MasterPrefs master_prefs = {0};
   bool first_run_ui_bypass = false;
   if (is_first_run) {
 #if defined(OS_WIN)
     // On first run, we need to process the master preferences before the
     // browser's profile_manager object is created, but after ResourceBundle
     // is initialized.
-    std::vector<std::wstring> first_run_tabs;
-    first_run_ui_bypass = !FirstRun::ProcessMasterPreferences(user_data_dir,
-        FilePath(), &first_run_tabs, &rlz_ping_delay, &homepage_defined,
-        &import_items, &dont_import_items);
+    first_run_ui_bypass =
+        !FirstRun::ProcessMasterPreferences(user_data_dir,
+                                            FilePath(), &master_prefs);
     // The master prefs might specify a set of urls to display.
-    if (first_run_tabs.size())
-      AddFirstRunNewTabs(&browser_init, first_run_tabs);
+    if (master_prefs.new_tabs.size())
+      AddFirstRunNewTabs(&browser_init, master_prefs.new_tabs);
+    // The master prefs might specify a set of initial bookmarks.
+    if (master_prefs.bookmarks.size())
+      AddDefaultBookmarks(&browser_init, master_prefs.bookmarks);
 #endif  // OS_WIN
 
     // If we are running in App mode, we do not want to show the importer
@@ -837,8 +848,11 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // touches reads preferences.
   if (is_first_run) {
     if (!first_run_ui_bypass) {
-      if (!OpenFirstRunDialog(profile, homepage_defined, import_items,
-                              dont_import_items, &process_singleton)) {
+      if (!OpenFirstRunDialog(profile,
+                              master_prefs.homepage_defined,
+                              master_prefs.do_import_items,
+                              master_prefs.dont_import_items,
+                              &process_singleton)) {
         // The user cancelled the first run dialog box, we should exit Chrome.
         return ResultCodes::NORMAL_EXIT;
       }
@@ -852,6 +866,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
         local_state->SetBoolean(prefs::kMetricsReportingEnabled, true);
 #endif  // OS_POSIX
     }
+
     Browser::SetNewHomePagePrefs(user_prefs);
   }
 
@@ -876,7 +891,8 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // Init the RLZ library. This just binds the dll and schedules a task on the
   // file thread to be run sometime later. If this is the first run we record
   // the installation event.
-  RLZTracker::InitRlzDelayed(base::DIR_MODULE, is_first_run, rlz_ping_delay);
+  RLZTracker::InitRlzDelayed(base::DIR_MODULE, is_first_run,
+                             master_prefs.ping_delay);
 #endif
 
   // Configure the network module so it has access to resources.
diff --git a/chrome/browser/first_run.h b/chrome/browser/first_run.h
index 3b2d996..08a101a4 100644
--- a/chrome/browser/first_run.h
+++ b/chrome/browser/first_run.h
@@ -28,6 +28,15 @@ class ProcessSingleton;
 // install work for this user. After that the sentinel file is created.
 class FirstRun {
  public:
+  // See ProcessMasterPreferences for more info about this structure.
+  struct MasterPrefs {
+    int ping_delay;
+    bool homepage_defined;
+    int do_import_items;
+    int dont_import_items;
+    std::vector<std::wstring> new_tabs;
+    std::vector<std::wstring> bookmarks;
+  };
 #if defined(OS_WIN)
   // Creates the desktop shortcut to chrome for the current user. Returns
   // false if it fails. It will overwrite the shortcut if it exists.
@@ -40,6 +49,7 @@ class FirstRun {
   // FirstRun::ImportSettings(). This function might or might not show
   // a visible UI depending on the cmdline parameters.
   static int ImportNow(Profile* profile, const CommandLine& cmdline);
+
   // The master preferences is a JSON file with the same entries as the
   // 'Default\Preferences' file. This function locates this file from
   // master_pref_path or if that path is empty from the default location
@@ -57,11 +67,7 @@ class FirstRun {
   // 'master_preferences' file.
   static bool ProcessMasterPreferences(const FilePath& user_data_dir,
                                        const FilePath& master_prefs_path,
-                                       std::vector<std::wstring>* new_tabs,
-                                       int* ping_delay,
-                                       bool* homepage_defined,
-                                       int* do_import_items,
-                                       int* dont_import_items);
+                                       MasterPrefs* out_prefs);
 #endif  // OS_WIN
 
   // Returns true if this is the first time chrome is run for this user.
diff --git a/chrome/browser/first_run_win.cc b/chrome/browser/first_run_win.cc
index 5a3078f..faaef63 100644
--- a/chrome/browser/first_run_win.cc
+++ b/chrome/browser/first_run_win.cc
@@ -177,11 +177,7 @@ bool FirstRun::CreateChromeQuickLaunchShortcut() {
 
 bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
                                         const FilePath& master_prefs_path,
-                                        std::vector<std::wstring>* new_tabs,
-                                        int* ping_delay,
-                                        bool* homepage_defined,
-                                        int* do_import_items,
-                                        int* dont_import_items) {
+                                        MasterPrefs* out_prefs) {
   DCHECK(!user_data_dir.empty());
   FilePath master_prefs = master_prefs_path;
   if (master_prefs.empty()) {
@@ -197,21 +193,19 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
   if (!prefs.get())
     return true;
 
-  if (new_tabs)
-    *new_tabs = installer_util::GetFirstRunTabs(prefs.get());
-  if (ping_delay) {
-    if (!installer_util::GetDistroIntegerPreference(prefs.get(),
-        installer_util::master_preferences::kDistroPingDelay, ping_delay)) {
-      // 90 seconds is the default that we want to use in case master
-      // preferences is missing, corrupt or ping_delay is missing.
-      *ping_delay = 90;
-    }
-  }
-  if (homepage_defined) {
-    std::string not_used;
-    *homepage_defined = prefs->GetString(prefs::kHomePage, &not_used);
+  out_prefs->new_tabs = installer_util::GetFirstRunTabs(prefs.get());
+
+  if (!installer_util::GetDistroIntegerPreference(prefs.get(),
+      installer_util::master_preferences::kDistroPingDelay,
+      &out_prefs->ping_delay)) {
+    // 90 seconds is the default that we want to use in case master
+    // preferences is missing, corrupt or ping_delay is missing.
+    out_prefs->ping_delay = 90;
   }
 
+  std::string not_used;
+  out_prefs->homepage_defined = prefs->GetString(prefs::kHomePage, &not_used);
+
   bool value = false;
   if (installer_util::GetDistroBooleanPreference(prefs.get(),
       installer_util::master_preferences::kRequireEula, &value) && value) {
@@ -262,10 +256,9 @@ bool FirstRun::ProcessMasterPreferences(const FilePath& user_data_dir,
       installer_util::master_preferences::kDistroImportSearchPref, &value)) {
     if (value) {
       import_items += SEARCH_ENGINES;
-      if (do_import_items)
-        *do_import_items += SEARCH_ENGINES;
-    } else if (dont_import_items) {
-        *dont_import_items += SEARCH_ENGINES;
+      out_prefs->do_import_items += SEARCH_ENGINES;
+    } else {
+      out_prefs->dont_import_items += SEARCH_ENGINES;
     }
   }
 
diff --git a/chrome/installer/util/master_preferences.cc b/chrome/installer/util/master_preferences.cc
index 176c2fb..cf26c3e 100644
--- a/chrome/installer/util/master_preferences.cc
+++ b/chrome/installer/util/master_preferences.cc
@@ -11,6 +11,27 @@
 
 namespace {
 const wchar_t* kDistroDict = L"distribution";
+
+std::vector<std::wstring> GetNamedList(const wchar_t* name,
+                                       const DictionaryValue* prefs) {
+  std::vector<std::wstring> list;
+  if (!prefs)
+    return list;
+  ListValue* value_list = NULL;
+  if (!prefs->GetList(name, &value_list))
+    return list;
+  for (size_t i = 0; i < value_list->GetSize(); ++i) {
+    Value* entry;
+    std::wstring str_entry;
+    if (!value_list->Get(i, &entry) || !entry->GetAsString(&str_entry)) {
+      NOTREACHED();
+      break;
+    }
+    list.push_back(str_entry);
+  }
+  return list;
+}
+
 }
 
 namespace installer_util {
@@ -92,22 +113,11 @@ DictionaryValue* ParseDistributionPreferences(
 }
 
 std::vector<std::wstring> GetFirstRunTabs(const DictionaryValue* prefs) {
-  std::vector<std::wstring> launch_tabs;
-  if (!prefs)
-    return launch_tabs;
-  ListValue* tabs_list = NULL;
-  if (!prefs->GetList(L"first_run_tabs", &tabs_list))
-    return launch_tabs;
-  for (size_t i = 0; i < tabs_list->GetSize(); ++i) {
-    Value* entry;
-    std::wstring tab_entry;
-    if (!tabs_list->Get(i, &entry) || !entry->GetAsString(&tab_entry)) {
-      NOTREACHED();
-      break;
-    }
-    launch_tabs.push_back(tab_entry);
-  }
-  return launch_tabs;
+  return GetNamedList(L"first_run_tabs", prefs);
+}
+
+std::vector<std::wstring> GetDefaultBookmarks(const DictionaryValue* prefs) {
+  return GetNamedList(L"default_bookmarks", prefs);
 }
 
 bool SetDistroBooleanPreference(DictionaryValue* prefs,
diff --git a/chrome/installer/util/master_preferences.h b/chrome/installer/util/master_preferences.h
index a809c97..ee750a4 100644
--- a/chrome/installer/util/master_preferences.h
+++ b/chrome/installer/util/master_preferences.h
@@ -140,6 +140,22 @@ DictionaryValue* ParseDistributionPreferences(
 // preferences file does not contain such list the vector is empty.
 std::vector<std::wstring> GetFirstRunTabs(const DictionaryValue* prefs);
 
+// As part of the master preferences an optional section indicates the
+// pre-installed bookmarks. An example is the following:
+//
+//  {
+//    "default_bookmarks": [
+//       "http://google.com/b1",
+//       "https://google.com/b2"
+//    ]
+//  }
+//
+// Note that the entries need to be urls.
+//
+// This function retuns the list as a vector of strings. If the master
+// preferences file does not contain such list the vector is empty.
+std::vector<std::wstring> GetDefaultBookmarks(const DictionaryValue* prefs);
+
 // Sets the value of given boolean preference |name| in "distribution"
 // dictionary inside |prefs| dictionary.
 bool SetDistroBooleanPreference(DictionaryValue* prefs,
diff --git a/chrome/installer/util/master_preferences_unittest.cc b/chrome/installer/util/master_preferences_unittest.cc
index ad827eb..18e560f 100644
--- a/chrome/installer/util/master_preferences_unittest.cc
+++ b/chrome/installer/util/master_preferences_unittest.cc
@@ -14,18 +14,21 @@ namespace {
 class MasterPreferencesTest : public testing::Test {
  protected:
   virtual void SetUp() {
-    // Currently no setup required.
+    ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file_));
   }
 
   virtual void TearDown() {
-    // Currently no tear down required.
+    EXPECT_TRUE(file_util::Delete(prefs_file_, false));
   }
+
+  const FilePath& prefs_file() const { return prefs_file_; }
+
+ private:
+  FilePath prefs_file_;
 };
 }  // namespace
 
-TEST(MasterPreferencesTest, ParseDistroParams) {
-  FilePath prefs_file;
-  ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file));
+TEST_F(MasterPreferencesTest, ParseDistroParams) {
   const char text[] =
     "{ \n"
     "  \"distribution\": { \n"
@@ -52,9 +55,9 @@ TEST(MasterPreferencesTest, ParseDistroParams) {
     "  }\n"
     "} \n";
 
-  EXPECT_TRUE(file_util::WriteFile(prefs_file, text, sizeof(text)));
+  EXPECT_TRUE(file_util::WriteFile(prefs_file(), text, sizeof(text)));
   scoped_ptr<DictionaryValue> prefs(
-      installer_util::ParseDistributionPreferences(prefs_file));
+      installer_util::ParseDistributionPreferences(prefs_file()));
   EXPECT_TRUE(prefs.get() != NULL);
   bool value = true;
   EXPECT_TRUE(installer_util::GetDistroBooleanPreference(prefs.get(),
@@ -115,12 +118,9 @@ TEST(MasterPreferencesTest, ParseDistroParams) {
   EXPECT_TRUE(installer_util::GetDistroIntegerPreference(prefs.get(),
       installer_util::master_preferences::kDistroPingDelay, &ping_delay));
   EXPECT_EQ(ping_delay, 40);
-  EXPECT_TRUE(file_util::Delete(prefs_file, false));
 }
 
-TEST(MasterPreferencesTest, ParseMissingDistroParams) {
-  FilePath prefs_file;
-  ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file));
+TEST_F(MasterPreferencesTest, ParseMissingDistroParams) {
   const char text[] =
     "{ \n"
     "  \"distribution\": { \n"
@@ -133,9 +133,9 @@ TEST(MasterPreferencesTest, ParseMissingDistroParams) {
     "  }\n"
     "} \n";
 
-  EXPECT_TRUE(file_util::WriteFile(prefs_file, text, sizeof(text)));
+  EXPECT_TRUE(file_util::WriteFile(prefs_file(), text, sizeof(text)));
   scoped_ptr<DictionaryValue> prefs(
-      installer_util::ParseDistributionPreferences(prefs_file));
+      installer_util::ParseDistributionPreferences(prefs_file()));
   EXPECT_TRUE(prefs.get() != NULL);
   bool value = false;
   EXPECT_TRUE(installer_util::GetDistroBooleanPreference(prefs.get(),
@@ -178,12 +178,9 @@ TEST(MasterPreferencesTest, ParseMissingDistroParams) {
   EXPECT_FALSE(installer_util::GetDistroIntegerPreference(prefs.get(),
       installer_util::master_preferences::kDistroPingDelay, &ping_delay));
   EXPECT_EQ(ping_delay, 90);
-  EXPECT_TRUE(file_util::Delete(prefs_file, false));
 }
 
-TEST(MasterPreferencesTest, FirstRunTabs) {
-  FilePath prefs_file;
-  ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file));
+TEST_F(MasterPreferencesTest, FirstRunTabs) {
   const char text[] =
     "{ \n"
     "  \"distribution\": { \n"
@@ -196,9 +193,9 @@ TEST(MasterPreferencesTest, FirstRunTabs) {
     "  ]\n"
     "} \n";
 
-  EXPECT_TRUE(file_util::WriteFile(prefs_file, text, sizeof(text)));
+  EXPECT_TRUE(file_util::WriteFile(prefs_file(), text, sizeof(text)));
   scoped_ptr<DictionaryValue> prefs(
-      installer_util::ParseDistributionPreferences(prefs_file));
+      installer_util::ParseDistributionPreferences(prefs_file()));
   EXPECT_TRUE(prefs.get() != NULL);
 
   typedef std::vector<std::wstring> TabsVector;
@@ -207,5 +204,28 @@ TEST(MasterPreferencesTest, FirstRunTabs) {
   EXPECT_EQ(L"http://google.com/f1", tabs[0]);
   EXPECT_EQ(L"https://google.com/f2", tabs[1]);
   EXPECT_EQ(L"new_tab_page", tabs[2]);
-  EXPECT_TRUE(file_util::Delete(prefs_file, false));
+}
+
+TEST_F(MasterPreferencesTest, FirstRunBookMarks) {
+  const char text[] =
+    "{ \n"
+    "  \"distribution\": { \n"
+    "     \"something here\": true\n"
+    "  },\n"
+    "  \"default_bookmarks\": [\n"
+    "     \"http://google.com/b1\",\n"
+    "     \"https://google.com/b2\"\n"
+    "  ]\n"
+    "} \n";
+
+  EXPECT_TRUE(file_util::WriteFile(prefs_file(), text, sizeof(text)));
+  scoped_ptr<DictionaryValue> prefs(
+      installer_util::ParseDistributionPreferences(prefs_file()));
+  EXPECT_TRUE(prefs.get() != NULL);
+
+  typedef std::vector<std::wstring> BookmarksVector;
+  BookmarksVector bookmarks = installer_util::GetDefaultBookmarks(prefs.get());
+  ASSERT_EQ(2, bookmarks.size());
+  EXPECT_EQ(L"http://google.com/b1", bookmarks[0]);
+  EXPECT_EQ(L"https://google.com/b2", bookmarks[1]);
 }