Refactoring of master preferences parsing before adding a new preference.

Currently we are parsing master preferences file three time on startup. Since we only return an int bit mask flag after parsing preferences, it can not handle any preference other than boolean and we end up reading it again for first run tabs and ping delay. This change refactors all the preferences parsing logic to directly pass around DictionaryValue object around in Chrome as well as installer.

No functional change but this will make adding a new preference for new icon more logical since we will not read the preferences file once again.

BUG=12701
TEST=Make sure all the distribution preferences still work as before.

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

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

3 files changed, 101 insertions, 103 deletions

diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 2a3bc3a..ba62f0c 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -7,24 +7,21 @@
 
 #include "chrome/installer/setup/install.h"
 
-#include "base/command_line.h"
 #include "base/file_util.h"
+#include "base/logging.h"
 #include "base/path_service.h"
 #include "base/registry.h"
 #include "base/scoped_ptr.h"
-#include "base/string_util.h"
-#include "chrome/common/chrome_constants.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
+#include "chrome/installer/util/master_preferences.h"
 #include "chrome/installer/util/set_reg_value_work_item.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/util_constants.h"
-#include "chrome/installer/util/work_item.h"
-#include "chrome/installer/util/version.h"
 #include "chrome/installer/util/work_item_list.h"
 
 // Build-time generated include file.
@@ -160,11 +157,12 @@ void AddUninstallShortcutWorkItems(HKEY reg_root,
 // If the shortcuts do not exist, the function does not recreate them during
 // update.
 bool CreateOrUpdateChromeShortcuts(const std::wstring& exe_path,
-                                   int options,
-                                   installer_util::InstallStatus install_status,
                                    const std::wstring& install_path,
-                                   const std::wstring& new_version) {
-  bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
+                                   const std::wstring& new_version,
+                                   installer_util::InstallStatus install_status,
+                                   bool system_install,
+                                   bool create_all_shortcut,
+                                   bool alt_shortcut) {
   FilePath shortcut_path;
   int dir_enum = (system_install) ? base::DIR_COMMON_START_MENU :
                                     base::DIR_START_MENU;
@@ -242,20 +240,18 @@ bool CreateOrUpdateChromeShortcuts(const std::wstring& exe_path,
   // Update Desktop and Quick Launch shortcuts. If --create-new-shortcuts
   // is specified we want to create them, otherwise we update them only if
   // they exist.
-  bool create = (options & installer_util::CREATE_ALL_SHORTCUTS) != 0;
-  // In some cases the main desktop shortcut has an alternate name.
-  bool alt_shortcut = (options & installer_util::ALT_DESKTOP_SHORTCUT) != 0;
-
   if (system_install) {
     ret = ret && ShellUtil::CreateChromeDesktopShortcut(chrome_exe,
-        product_desc, ShellUtil::SYSTEM_LEVEL, alt_shortcut, create);
+        product_desc, ShellUtil::SYSTEM_LEVEL, alt_shortcut,
+        create_all_shortcut);
     ret = ret && ShellUtil::CreateChromeQuickLaunchShortcut(chrome_exe,
-        ShellUtil::CURRENT_USER | ShellUtil::SYSTEM_LEVEL, create);
+        ShellUtil::CURRENT_USER | ShellUtil::SYSTEM_LEVEL, create_all_shortcut);
   } else {
     ret = ret && ShellUtil::CreateChromeDesktopShortcut(chrome_exe,
-        product_desc, ShellUtil::CURRENT_USER, alt_shortcut, create);
+        product_desc, ShellUtil::CURRENT_USER, alt_shortcut,
+        create_all_shortcut);
     ret = ret && ShellUtil::CreateChromeQuickLaunchShortcut(chrome_exe,
-        ShellUtil::CURRENT_USER, create);
+        ShellUtil::CURRENT_USER, create_all_shortcut);
   }
 
   return ret;
@@ -279,8 +275,9 @@ bool Is64bit() {
   return false;
 }
 
-void RegisterChromeOnMachine(const std::wstring& install_path, int options) {
-  bool system_level = (options & installer_util::SYSTEM_LEVEL) != 0;
+void RegisterChromeOnMachine(const std::wstring& install_path,
+                             bool system_level,
+                             bool make_chrome_default) {
   // Try to add Chrome to Media Player shim inclusion list. We don't do any
   // error checking here because this operation will fail if user doesn't
   // have admin rights and we want to ignore the error.
@@ -291,7 +288,7 @@ void RegisterChromeOnMachine(const std::wstring& install_path, int options) {
   std::wstring chrome_exe(install_path);
   file_util::AppendToPath(&chrome_exe, installer_util::kChromeExe);
   LOG(INFO) << "Registering Chrome as browser";
-  if (options & installer_util::MAKE_CHROME_DEFAULT) {
+  if (make_chrome_default) {
     int level = ShellUtil::CURRENT_USER;
     if (system_level)
       level = level | ShellUtil::SYSTEM_LEVEL;
@@ -483,9 +480,10 @@ bool installer::InstallNewVersion(const std::wstring& exe_path,
 
 installer_util::InstallStatus installer::InstallOrUpdateChrome(
     const std::wstring& exe_path, const std::wstring& archive_path,
-    const std::wstring& install_temp_path, int options,
+    const std::wstring& install_temp_path, const DictionaryValue* prefs,
     const Version& new_version, const Version* installed_version) {
-  bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
+  bool system_install = installer_util::GetDistroBooleanPreference(prefs,
+      installer_util::master_preferences::kSystemLevel);
   std::wstring install_path(GetChromeInstallPath(system_install));
   if (install_path.empty()) {
     LOG(ERROR) << "Could not get installation destination path.";
@@ -526,13 +524,21 @@ installer_util::InstallStatus installer::InstallOrUpdateChrome(
       result = installer_util::NEW_VERSION_UPDATED;
     }
 
-    if (!CreateOrUpdateChromeShortcuts(exe_path, options, result,
-                                       install_path, new_version.GetString()))
+    bool create_all_shortcut = installer_util::GetDistroBooleanPreference(prefs,
+        installer_util::master_preferences::kCreateAllShortcuts);
+    bool alt_shortcut = installer_util::GetDistroBooleanPreference(prefs,
+        installer_util::master_preferences::kAltShortcutText);
+    if (!CreateOrUpdateChromeShortcuts(exe_path, install_path,
+                                       new_version.GetString(), result,
+                                       system_install, create_all_shortcut,
+                                       alt_shortcut))
       LOG(WARNING) << "Failed to create/update start menu shortcut.";
 
     RemoveOldVersionDirs(install_path, new_version.GetString());
 
-    RegisterChromeOnMachine(install_path, options);
+    bool make_chrome_default = installer_util::GetDistroBooleanPreference(prefs,
+        installer_util::master_preferences::kMakeChromeDefault);
+    RegisterChromeOnMachine(install_path, system_install, make_chrome_default);
   }
 
   return result;
diff --git a/chrome/installer/setup/install.h b/chrome/installer/setup/install.h
index 6613e6fb..de468b3 100644
--- a/chrome/installer/setup/install.h
+++ b/chrome/installer/setup/install.h
@@ -7,8 +7,7 @@
 #ifndef CHROME_INSTALLER_SETUP_INSTALL_H_
 #define CHROME_INSTALLER_SETUP_INSTALL_H_
 
-#include <string>
-#include <windows.h>
+#include <base/values.h>
 
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/version.h"
@@ -29,7 +28,7 @@ std::wstring GetInstallerPathUnderChrome(const std::wstring& install_path,
 // install_temp_path: working directory used during install/update. It should
 //                    also has a sub dir source that contains a complete
 //                    and unpacked Chrome package.
-// options: install options. See chrome/installer/util/util_constants.h.
+// prefs: master preferences. See chrome/installer/util/master_preferences.h.
 // new_version: new Chrome version that needs to be installed
 // installed_version: currently installed version of Chrome, if any, or
 //                    NULL otherwise
@@ -38,7 +37,7 @@ std::wstring GetInstallerPathUnderChrome(const std::wstring& install_path,
 // is responsible for cleaning up install_temp_path.
 installer_util::InstallStatus InstallOrUpdateChrome(
     const std::wstring& exe_path, const std::wstring& archive_path,
-    const std::wstring& install_temp_path, int options,
+    const std::wstring& install_temp_path, const DictionaryValue* prefs,
     const Version& new_version, const Version* installed_version);
 
 // This function installs a new version of Chrome to the specified location.
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index d73e4db..eb2e8ba 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -206,74 +206,63 @@ installer_util::InstallStatus RenameChromeExecutables(bool system_install) {
 
 // Parse command line and read master profile, if present, to get distribution
 // related install options.
-int GetInstallOptions(const CommandLine& cmd_line) {
-  int options = 0;
-  int preferences = 0;
+DictionaryValue* GetInstallPreferences(const CommandLine& cmd_line) {
+  DictionaryValue* prefs = NULL;
 
   if (cmd_line.HasSwitch(installer_util::switches::kInstallerData)) {
-    std::wstring prefs_path = cmd_line.GetSwitchValue(
-        installer_util::switches::kInstallerData);
-    preferences = installer_util::ParseDistributionPreferences(prefs_path);
-    if ((preferences & installer_util::MASTER_PROFILE_NOT_FOUND) == 0) {
-      options |= installer_util::MASTER_PROFILE_PRESENT;
-      if ((preferences & installer_util::MASTER_PROFILE_ERROR) == 0)
-        options |= installer_util::MASTER_PROFILE_VALID;
-    }
-    // While there is a --show-eula command line flag, we don't process
-    // it in this function because it requires special handling.
-    if (preferences & installer_util::MASTER_PROFILE_REQUIRE_EULA)
-      options |= installer_util::SHOW_EULA_DIALOG;
+    FilePath prefs_path(
+        cmd_line.GetSwitchValue(installer_util::switches::kInstallerData));
+    prefs = installer_util::ParseDistributionPreferences(prefs_path);
   }
 
-  if (preferences & installer_util::MASTER_PROFILE_CREATE_ALL_SHORTCUTS ||
-      cmd_line.HasSwitch(installer_util::switches::kCreateAllShortcuts))
-    options |= installer_util::CREATE_ALL_SHORTCUTS;
+  if (!prefs)
+    prefs = new DictionaryValue();
 
-  if (preferences & installer_util::MASTER_PROFILE_DO_NOT_LAUNCH_CHROME ||
-      cmd_line.HasSwitch(installer_util::switches::kDoNotLaunchChrome))
-    options |= installer_util::DO_NOT_LAUNCH_CHROME;
+  if (cmd_line.HasSwitch(installer_util::switches::kCreateAllShortcuts))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kCreateAllShortcuts, true);
 
-  if (preferences & installer_util::MASTER_PROFILE_MAKE_CHROME_DEFAULT ||
-      cmd_line.HasSwitch(installer_util::switches::kMakeChromeDefault))
-    options |= installer_util::MAKE_CHROME_DEFAULT;
+  if (cmd_line.HasSwitch(installer_util::switches::kDoNotLaunchChrome))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kDoNotLaunchChrome, true);
 
-  if (preferences & installer_util::MASTER_PROFILE_SYSTEM_LEVEL ||
-      cmd_line.HasSwitch(installer_util::switches::kSystemLevel))
-    options |= installer_util::SYSTEM_LEVEL;
+  if (cmd_line.HasSwitch(installer_util::switches::kMakeChromeDefault))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kMakeChromeDefault, true);
 
-  if (preferences & installer_util::MASTER_PROFILE_VERBOSE_LOGGING ||
-      cmd_line.HasSwitch(installer_util::switches::kVerboseLogging))
-    options |= installer_util::VERBOSE_LOGGING;
+  if (cmd_line.HasSwitch(installer_util::switches::kSystemLevel))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kSystemLevel, true);
 
-  if (preferences & installer_util::MASTER_PROFILE_ALT_SHORTCUT_TXT ||
-      cmd_line.HasSwitch(installer_util::switches::kAltDesktopShortcut))
-    options |= installer_util::ALT_DESKTOP_SHORTCUT;
+  if (cmd_line.HasSwitch(installer_util::switches::kVerboseLogging))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kVerboseLogging, true);
 
-  return options;
+  if (cmd_line.HasSwitch(installer_util::switches::kAltDesktopShortcut))
+    installer_util::SetDistroBooleanPreference(
+        prefs, installer_util::master_preferences::kAltShortcutText, true);
+
+  return prefs;
 }
 
-// Copy master preference file if provided to installer to the same path
-// of chrome.exe so Chrome first run can find it.
-// This function will be called only when Chrome is launched the first time.
-void CopyPreferenceFileForFirstRun(int options, const CommandLine& cmd_line) {
-  if (options & installer_util::MASTER_PROFILE_VALID) {
-    std::wstring prefs_source_path = cmd_line.GetSwitchValue(
-        installer_util::switches::kInstallerData);
-    bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
-    std::wstring prefs_dest_path(
-        installer::GetChromeInstallPath(system_install));
-    file_util::AppendToPath(&prefs_dest_path,
-                            installer_util::kDefaultMasterPrefs);
-    if (!file_util::CopyFile(prefs_source_path, prefs_dest_path))
-      LOG(ERROR) << "failed copying master profile";
-  }
+// Copy master preferences file provided to installer, in the same folder
+// as chrome.exe so Chrome first run can find it. This function will be called
+// only on the first install of Chrome.
+void CopyPreferenceFileForFirstRun(bool system_level,
+                                   const CommandLine& cmd_line) {
+  std::wstring prefs_source_path = cmd_line.GetSwitchValue(
+      installer_util::switches::kInstallerData);
+  std::wstring prefs_dest_path(
+      installer::GetChromeInstallPath(system_level));
+  file_util::AppendToPath(&prefs_dest_path,
+                          installer_util::kDefaultMasterPrefs);
+  if (!file_util::CopyFile(prefs_source_path, prefs_dest_path))
+    LOG(ERROR) << "failed copying master profile";
 }
 
 bool CheckPreInstallConditions(const installer::Version* installed_version,
-                               int options,
+                               bool system_install,
                                installer_util::InstallStatus& status) {
-  bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
-
   // Check to avoid simultaneous per-user and per-machine installs.
   scoped_ptr<installer::Version>
       chrome_version(InstallUtil::GetChromeVersion(!system_install));
@@ -308,12 +297,14 @@ bool CheckPreInstallConditions(const installer::Version* installed_version,
 }
 
 installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
-    const installer::Version* installed_version, int options) {
+    const installer::Version* installed_version, const DictionaryValue* prefs) {
+  bool system_level = installer_util::GetDistroBooleanPreference(prefs,
+      installer_util::master_preferences::kSystemLevel);
   installer_util::InstallStatus install_status = installer_util::UNKNOWN_STATUS;
-  if (!CheckPreInstallConditions(installed_version, options, install_status))
+  if (!CheckPreInstallConditions(installed_version,
+                                 system_level, install_status))
     return install_status;
 
-  bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
   // For install the default location for chrome.packed.7z is in current
   // folder, so get that value first.
   std::wstring archive = file_util::GetDirectoryFromPath(cmd_line.program());
@@ -332,7 +323,7 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
   if (!file_util::CreateNewTempDirectory(std::wstring(L"chrome_"),
                                          &temp_path)) {
     LOG(ERROR) << "Could not create temporary path.";
-    InstallUtil::WriteInstallerResult(system_install,
+    InstallUtil::WriteInstallerResult(system_level,
                                       installer_util::TEMP_DIR_FAILED,
                                       IDS_INSTALL_TEMP_DIR_FAILED_BASE,
                                       NULL);
@@ -345,10 +336,10 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
   file_util::AppendToPath(&unpack_path,
                           std::wstring(installer::kInstallSourceDir));
   bool incremental_install = false;
-  if (UnPackArchive(archive, system_install, installed_version,
+  if (UnPackArchive(archive, system_level, installed_version,
                     temp_path, unpack_path, incremental_install)) {
     install_status = installer_util::UNCOMPRESSION_FAILED;
-    InstallUtil::WriteInstallerResult(system_install, install_status,
+    InstallUtil::WriteInstallerResult(system_level, install_status,
                                       IDS_INSTALL_UNCOMPRESSION_FAILED_BASE,
                                       NULL);
   } else {
@@ -361,7 +352,7 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
     if (!installer_version.get()) {
       LOG(ERROR) << "Did not find any valid version in installer.";
       install_status = installer_util::INVALID_ARCHIVE;
-      InstallUtil::WriteInstallerResult(system_install, install_status,
+      InstallUtil::WriteInstallerResult(system_level, install_status,
                                         IDS_INSTALL_INVALID_ARCHIVE_BASE, NULL);
     } else {
       LOG(INFO) << "version to install: " << installer_version->GetString();
@@ -369,7 +360,7 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
           installed_version->IsHigherThan(installer_version.get())) {
         LOG(ERROR) << "Higher version is already installed.";
         install_status = installer_util::HIGHER_VERSION_EXISTS;
-        InstallUtil::WriteInstallerResult(system_install, install_status,
+        InstallUtil::WriteInstallerResult(system_level, install_status,
                                           IDS_INSTALL_HIGHER_VERSION_BASE,
                                           NULL);
       } else {
@@ -379,13 +370,13 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
         file_util::AppendToPath(&archive_to_copy,
                                 std::wstring(installer::kChromeArchive));
         install_status = installer::InstallOrUpdateChrome(
-            cmd_line.program(), archive_to_copy, temp_path, options,
+            cmd_line.program(), archive_to_copy, temp_path, prefs,
             *installer_version, installed_version);
 
         int install_msg_base = IDS_INSTALL_FAILED_BASE;
         std::wstring chrome_exe;
         if (install_status != installer_util::INSTALL_FAILED) {
-          chrome_exe = installer::GetChromeInstallPath(system_install);
+          chrome_exe = installer::GetChromeInstallPath(system_level);
           if (chrome_exe.empty()) {
             // If we failed to construct install path, it means the OS call to
             // get %ProgramFiles% or %AppData% failed. Report this as failure.
@@ -397,15 +388,15 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
             install_msg_base = 0;
           }
         }
-        InstallUtil::WriteInstallerResult(system_install, install_status,
+        InstallUtil::WriteInstallerResult(system_level, install_status,
                                           install_msg_base, &chrome_exe);
         if (install_status == installer_util::FIRST_INSTALL_SUCCESS) {
           LOG(INFO) << "First install successful.";
-          CopyPreferenceFileForFirstRun(options, cmd_line);
+          CopyPreferenceFileForFirstRun(system_level, cmd_line);
           // We never want to launch Chrome in system level install mode.
-          if (!(options & installer_util::DO_NOT_LAUNCH_CHROME) &&
-              !(options & installer_util::SYSTEM_LEVEL))
-            installer::LaunchChrome(system_install);
+          if (!system_level && !installer_util::GetDistroBooleanPreference(prefs,
+              installer_util::master_preferences::kDoNotLaunchChrome))
+            installer::LaunchChrome(system_level);
         }
       }
     }
@@ -414,7 +405,7 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
     // the case we would not do that directly at this point but in another
     //  instance of setup.exe
     dist->LaunchUserExperiment(install_status, *installer_version,
-                               system_install, options);
+                               system_level);
   }
 
   // Delete temporary files. These include install temporary directory
@@ -422,14 +413,14 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
   scoped_ptr<WorkItemList> cleanup_list(WorkItem::CreateWorkItemList());
   LOG(INFO) << "Deleting temporary directory " << temp_path;
   cleanup_list->AddDeleteTreeWorkItem(temp_path, std::wstring());
-  if (options & installer_util::MASTER_PROFILE_PRESENT) {
+  if (cmd_line.HasSwitch(installer_util::switches::kInstallerData)) {
     std::wstring prefs_path = cmd_line.GetSwitchValue(
         installer_util::switches::kInstallerData);
     cleanup_list->AddDeleteTreeWorkItem(prefs_path, std::wstring());
   }
   cleanup_list->Do();
 
-  dist->UpdateDiffInstallStatus(system_install, incremental_install,
+  dist->UpdateDiffInstallStatus(system_level, incremental_install,
                                 install_status);
   return install_status;
 }
@@ -620,11 +611,13 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
   CommandLine::Init(0, NULL);
   const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
   installer::InitInstallerLogging(parsed_command_line);
-  int options = GetInstallOptions(parsed_command_line);
-  if (options & installer_util::VERBOSE_LOGGING)
+  scoped_ptr<DictionaryValue> prefs(GetInstallPreferences(parsed_command_line));
+  if (installer_util::GetDistroBooleanPreference(prefs.get(),
+      installer_util::master_preferences::kVerboseLogging))
     logging::SetMinLogLevel(logging::LOG_INFO);
 
-  bool system_install = (options & installer_util::SYSTEM_LEVEL) != 0;
+  bool system_install = installer_util::GetDistroBooleanPreference(prefs.get(),
+      installer_util::master_preferences::kSystemLevel);
   LOG(INFO) << "system install is " << system_install;
 
   // Check to make sure current system is WinXP or later. If not, log
@@ -691,7 +684,7 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
   } else {
     install_status = InstallChrome(parsed_command_line,
                                    installed_version.get(),
-                                   options);
+                                   prefs.get());
   }
 
   CoUninitialize();