Add support for a "program_files_dir" field in the initial preferences.

The field "program_files_dir" will accept the full path to the system's
Program Files or Program Files (x86) directory (typically found in C:\).
This field will be linked to the MSI installer's PROGRAMFILESDIR
property. This new field will only be used for fresh system-level Chrome
installations and will be ignored for other kind of installations and
updates.

Bug: 1165921
Change-Id: Id688133c2b2740a953569c9453b808f2aeb7d1e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439784
Commit-Queue: Yann Dago <ydago@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845267}

chrome/installer/setup/installer_state.cc

@@ -80,7 +80,8 @@ void InstallerState::Initialize(const base::CommandLine& command_line,
 
   const bool is_uninstall = command_line.HasSwitch(switches::kUninstall);
 
-  target_path_ = GetChromeInstallPath(system_install());
+  target_path_ = GetChromeInstallPathWithPrefs(system_install(), prefs);
+
   state_key_ = install_static::GetClientStateKeyPath();
 
   VLOG(1) << (is_uninstall ? "Uninstall Chrome" : "Install Chrome");

chrome/installer/util/helper.cc

@@ -7,11 +7,43 @@
 #include "base/path_service.h"
 #include "base/win/registry.h"
 #include "chrome/install_static/install_util.h"
+#include "chrome/installer/util/initial_preferences.h"
+#include "chrome/installer/util/initial_preferences_constants.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/util_constants.h"
 
 namespace {
 
+// Returns a valid file path with the proper casing from the system if
+// |target_dir| is a valid target for the browser's installation and
+// |system_install| is true. Returns an empty file path otherwise.
+base::FilePath GetInstallationDirFromPrefs(
+    const installer::InitialPreferences& prefs,
+    bool system_install) {
+  base::FilePath program_files_dir;
+  if (system_install) {
+    prefs.GetPath(installer::initial_preferences::kProgramFilesDir,
+                  &program_files_dir);
+  }
+  if (program_files_dir.empty())
+    return program_files_dir;
+
+  base::FilePath expected_dir;
+  bool valid_program_files_path =
+      ((base::PathService::Get(base::DIR_PROGRAM_FILES, &expected_dir) &&
+        base::FilePath::CompareEqualIgnoreCase(program_files_dir.value(),
+                                               expected_dir.value())) ||
+       (base::PathService::Get(base::DIR_PROGRAM_FILESX86, &expected_dir) &&
+        base::FilePath::CompareEqualIgnoreCase(program_files_dir.value(),
+                                               expected_dir.value())));
+
+  return valid_program_files_path
+             ? expected_dir
+                   .Append(install_static::GetChromeInstallSubDirectory())
+                   .Append(installer::kInstallBinaryDir)
+             : base::FilePath();
+}
+
 base::FilePath GetDefaultChromeInstallPath(bool system_install) {
   base::FilePath install_path;
   int key = system_install ? base::DIR_PROGRAM_FILES : base::DIR_LOCAL_APP_DATA;
@@ -58,4 +90,17 @@ base::FilePath GetChromeInstallPath(bool system_install) {
   return install_path;
 }
 
+base::FilePath GetChromeInstallPathWithPrefs(bool system_install,
+                                             const InitialPreferences& prefs) {
+  base::FilePath install_path =
+      GetCurrentInstallPathFromRegistry(system_install);
+  if (!install_path.empty())
+    return install_path;
+
+  install_path = GetInstallationDirFromPrefs(prefs, system_install);
+  if (install_path.empty())
+    install_path = GetDefaultChromeInstallPath(system_install);
+  return install_path;
+}
+
 }  // namespace installer.

chrome/installer/util/helper.h

@@ -11,6 +11,8 @@
 
 namespace installer {
 
+class InitialPreferences;
+
 // This function returns the install path for Chrome depending on whether it's
 // a system wide install or a user specific install.
 // Returns the install path stored at
@@ -21,6 +23,14 @@ namespace installer {
 // (%LOCALAPPDATA%\[Company\]Product\Application).
 base::FilePath GetChromeInstallPath(bool system_install);
 
+// Returns a path to the directory holding chrome.exe for either a system wide
+// or user specific install. The returned path will be one of:
+// - The path to the current installation at |system_install|, if there is one.
+// - The desired path for a new installation based on the "program_files_dir"
+//   initial preference in the "distribution" dict of |prefs|, if set.
+// - The default path for a new installation based on the binary's bitness.
+base::FilePath GetChromeInstallPathWithPrefs(bool system_install,
+                                             const InitialPreferences& prefs);
 }  // namespace installer
 
 #endif  // CHROME_INSTALLER_UTIL_HELPER_H_

chrome/installer/util/helper_unittest.cc

@@ -10,24 +10,36 @@
 #include "base/path_service.h"
 #include "base/test/scoped_path_override.h"
 #include "base/test/test_reg_util_win.h"
+#include "base/values.h"
 #include "base/version.h"
 #include "base/win/registry.h"
 #include "chrome/install_static/install_util.h"
 #include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/initial_preferences.h"
 #include "chrome/installer/util/util_constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace installer {
 
-// Tests GetChromeInstallPath with a boolean parameter which is |true| if the
-// test must use system-level values or |false| it the test must use user-level
-// values.
-class GetChromeInstallPathTest : public testing::TestWithParam<bool> {
+struct Params {
+  Params(bool system_level, base::Optional<int> target_dir_key)
+      : system_level(system_level), target_dir_key(target_dir_key) {}
+  bool system_level;
+  base::Optional<int> target_dir_key;
+};
+
+// Tests GetChromeInstallPath with a params object that contains a boolean
+// |system_level| which is |true| if the test must use system-level values or
+// |false| it the test must use user-level values, and an optional
+// |target_dir_key| in which the installation should be made. When no value is
+// set for |target_dir_key|, assume an empty path.
+class GetChromeInstallPathTest : public testing::TestWithParam<Params> {
  public:
   GetChromeInstallPathTest() = default;
 
   void SetUp() override {
     ASSERT_TRUE(program_files_.CreateUniqueTempDir());
+    ASSERT_TRUE(program_files_x86_.CreateUniqueTempDir());
     ASSERT_TRUE(random_.CreateUniqueTempDir());
     ASSERT_TRUE(local_app_data_.CreateUniqueTempDir());
     ASSERT_NO_FATAL_FAILURE(
@@ -36,16 +48,21 @@ class GetChromeInstallPathTest : public testing::TestWithParam<bool> {
         registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
     program_files_override_.emplace(base::DIR_PROGRAM_FILES,
                                     program_files_path());
+    program_files_x86_override_.emplace(base::DIR_PROGRAM_FILESX86,
+                                        program_files_x86_path());
     local_data_app_override_.emplace(base::DIR_LOCAL_APP_DATA,
                                      local_app_data_path());
   }
 
   base::FilePath random_path() const { return random_.GetPath(); }
   base::FilePath program_files_path() const { return program_files_.GetPath(); }
+  base::FilePath program_files_x86_path() const {
+    return program_files_x86_.GetPath();
+  }
   base::FilePath local_app_data_path() const {
     return local_app_data_.GetPath();
   }
-  static bool is_system_level() { return GetParam(); }
+  static bool is_system_level() { return GetParam().system_level; }
 
   base::FilePath GetExpectedPath(bool system_level) {
     auto path = system_level ? program_files_path() : local_app_data_path();
@@ -69,10 +86,12 @@ class GetChromeInstallPathTest : public testing::TestWithParam<bool> {
 
  private:
   base::ScopedTempDir program_files_;
+  base::ScopedTempDir program_files_x86_;
   base::ScopedTempDir random_;
   base::ScopedTempDir local_app_data_;
   registry_util::RegistryOverrideManager registry_override_manager_;
   base::Optional<base::ScopedPathOverride> program_files_override_;
+  base::Optional<base::ScopedPathOverride> program_files_x86_override_;
   base::Optional<base::ScopedPathOverride> local_data_app_override_;
 };
 
@@ -138,9 +157,148 @@ TEST_P(GetChromeInstallPathTest, RegistryValueSetNoProductVersion) {
 
 INSTANTIATE_TEST_SUITE_P(UserLevelTest,
                          GetChromeInstallPathTest,
-                         testing::Values(false));
+                         testing::Values<Params>({false, base::nullopt}));
 INSTANTIATE_TEST_SUITE_P(SystemLevelTest,
                          GetChromeInstallPathTest,
-                         testing::Values(true));
+                         testing::Values<Params>({true, base::nullopt}));
+
+// Tests GetChromeInstallPath with a params object that contains a boolean
+// |system_level| which is |true| if the test must use system-level values or
+// |false| it the test must use user-level values, and a |target_dir| path in
+// which the installation should be made.
+class GetChromeInstallPathWithPrefsTest : public GetChromeInstallPathTest {
+ public:
+  GetChromeInstallPathWithPrefsTest() = default;
+
+  base::FilePath GetExpectedPathForSetup(bool system_level,
+                                         base::FilePath target_dir) {
+    if (system_level && !target_dir.empty() &&
+        (target_dir == program_files_path() ||
+         target_dir == program_files_x86_path())) {
+      return target_dir.Append(install_static::GetChromeInstallSubDirectory())
+          .Append(kInstallBinaryDir);
+    }
+    return GetExpectedPath(system_level);
+  }
+
+  static base::FilePath target_dir() {
+    base::FilePath result;
+    if (GetParam().target_dir_key.has_value())
+      base::PathService::Get(GetParam().target_dir_key.value(), &result);
+    return result;
+  }
+
+  static base::DictionaryValue prefs_json() {
+    base::FilePath result;
+    if (GetParam().target_dir_key.has_value())
+      base::PathService::Get(GetParam().target_dir_key.value(), &result);
+    base::DictionaryValue distribution;
+    distribution.SetStringPath("distribution.program_files_dir",
+                               result.AsUTF8Unsafe());
+
+    return distribution;
+  }
+};
+
+TEST_P(GetChromeInstallPathWithPrefsTest, NoRegistryValue) {
+  EXPECT_EQ(GetChromeInstallPathWithPrefs(is_system_level(),
+                                          InitialPreferences(prefs_json())),
+            GetExpectedPathForSetup(is_system_level(), target_dir()));
+}
+
+TEST_P(GetChromeInstallPathWithPrefsTest, RegistryValueSet) {
+  base::win::RegKey client_state_key(GetClientStateRegKey());
+  ASSERT_EQ(client_state_key.WriteValue(
+                kUninstallStringField,
+                random_path()
+                    .Append(install_static::GetChromeInstallSubDirectory())
+                    .Append(kInstallBinaryDir)
+                    .AppendASCII("1.0.0.0\\Installer\\setup.exe")
+                    .value()
+                    .c_str()),
+            ERROR_SUCCESS);
+
+  base::win::RegKey client_key(GetClientsRegKey());
+  ASSERT_EQ(client_key.WriteValue(google_update::kRegVersionField, L"1.0.0.0"),
+            ERROR_SUCCESS);
+  EXPECT_EQ(GetChromeInstallPathWithPrefs(is_system_level(),
+                                          InitialPreferences(prefs_json())),
+            random_path()
+                .Append(install_static::GetChromeInstallSubDirectory())
+                .Append(kInstallBinaryDir));
+}
+
+TEST_P(GetChromeInstallPathWithPrefsTest, RegistryValueSetWrongScope) {
+  base::win::RegKey client_state_key(GetClientStateRegKey());
+  ASSERT_EQ(client_state_key.WriteValue(
+                kUninstallStringField,
+                random_path()
+                    .Append(install_static::GetChromeInstallSubDirectory())
+                    .Append(kInstallBinaryDir)
+                    .AppendASCII("1.0.0.0\\Installer\\setup.exe")
+                    .value()
+                    .c_str()),
+            ERROR_SUCCESS);
+
+  base::win::RegKey client_key(GetClientsRegKey());
+  ASSERT_EQ(client_key.WriteValue(google_update::kRegVersionField, L"1.0.0.0"),
+            ERROR_SUCCESS);
+  EXPECT_EQ(GetChromeInstallPathWithPrefs(!is_system_level(),
+                                          InitialPreferences(prefs_json())),
+            GetExpectedPathForSetup(!is_system_level(), target_dir()));
+}
+
+TEST_P(GetChromeInstallPathWithPrefsTest, RegistryValueSetNoProductVersion) {
+  base::win::RegKey client_state_key(GetClientStateRegKey());
+  ASSERT_EQ(client_state_key.WriteValue(
+                kUninstallStringField,
+                random_path()
+                    .Append(install_static::GetChromeInstallSubDirectory())
+                    .Append(kInstallBinaryDir)
+                    .AppendASCII("1.0.0.0\\Installer\\setup.exe")
+                    .value()
+                    .c_str()),
+            ERROR_SUCCESS);
+  EXPECT_EQ(GetChromeInstallPathWithPrefs(is_system_level(),
+                                          InitialPreferences(prefs_json())),
+            GetExpectedPathForSetup(is_system_level(), target_dir()));
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    UserLevelX86SetupTest,
+    GetChromeInstallPathWithPrefsTest,
+    testing::Values<Params>(Params(false, base::DIR_PROGRAM_FILESX86)));
+
+INSTANTIATE_TEST_SUITE_P(
+    UserLevelX64SetupTest,
+    GetChromeInstallPathWithPrefsTest,
+    testing::Values<Params>(Params(false, base::DIR_PROGRAM_FILES)));
+
+INSTANTIATE_TEST_SUITE_P(UserLevelUnsupportedPathSetupTest,
+                         GetChromeInstallPathWithPrefsTest,
+                         testing::Values<Params>(Params(false,
+                                                        base::DIR_HOME)));
+
+INSTANTIATE_TEST_SUITE_P(UserLevelEmptyPathSetupTest,
+                         GetChromeInstallPathWithPrefsTest,
+                         testing::Values<Params>(Params(false, base::nullopt)));
+
+INSTANTIATE_TEST_SUITE_P(
+    MachineLevelX86SetupTest,
+    GetChromeInstallPathWithPrefsTest,
+    testing::Values<Params>(Params(true, base::DIR_PROGRAM_FILESX86)));
+
+INSTANTIATE_TEST_SUITE_P(
+    MachineLevelX64SetupTest,
+    GetChromeInstallPathWithPrefsTest,
+    testing::Values<Params>(Params(true, base::DIR_PROGRAM_FILES)));
+
+INSTANTIATE_TEST_SUITE_P(MachineLevelUnsupportedPathSetupTest,
+                         GetChromeInstallPathWithPrefsTest,
+                         testing::Values<Params>(Params(true, base::DIR_HOME)));
+
+INSTANTIATE_TEST_SUITE_P(MachineLevelEmptyPathSetupTest,
+                         GetChromeInstallPathWithPrefsTest,
+                         testing::Values<Params>(Params(true, base::nullopt)));
 
 }  // namespace installer

chrome/installer/util/initial_preferences.cc

@@ -14,6 +14,7 @@
 #include "base/notreached.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/pref_names.h"
@@ -93,6 +94,15 @@ InitialPreferences::InitialPreferences(const std::string& prefs) {
   InitializeFromString(prefs);
 }
 
+InitialPreferences::InitialPreferences(const base::DictionaryValue& prefs)
+    : initial_dictionary_(prefs.CreateDeepCopy()) {
+  // Cache a pointer to the distribution dictionary.
+  initial_dictionary_->GetDictionary(
+      installer::initial_preferences::kDistroDict, &distribution_);
+
+  EnforceLegacyPreferences();
+}
+
 InitialPreferences::~InitialPreferences() = default;
 
 void InitialPreferences::InitializeFromCommandLine(
@@ -277,6 +287,15 @@ bool InitialPreferences::GetString(const std::string& name,
   return ret;
 }
 
+bool InitialPreferences::GetPath(const std::string& name,
+                                 base::FilePath* value) const {
+  std::string string_value;
+  if (!GetString(name, &string_value))
+    return false;
+  *value = base::FilePath::FromUTF8Unsafe(string_value);
+  return true;
+}
+
 std::vector<std::string> InitialPreferences::GetFirstRunTabs() const {
   return GetNamedList(kFirstRunTabs, initial_dictionary_.get());
 }

chrome/installer/util/initial_preferences.h

@@ -93,6 +93,10 @@ class InitialPreferences {
   // line switches with the distribution dictionary.
   explicit InitialPreferences(const std::string& prefs);
 
+  // Parses a preferences directly from |prefs| and does not merge any command
+  // line switches with the distribution dictionary.
+  explicit InitialPreferences(const base::DictionaryValue& prefs);
+
   ~InitialPreferences();
 
   // Each of the Get methods below returns true if the named value was found in
@@ -101,6 +105,7 @@ class InitialPreferences {
   bool GetBool(const std::string& name, bool* value) const;
   bool GetInt(const std::string& name, int* value) const;
   bool GetString(const std::string& name, std::string* value) const;
+  bool GetPath(const std::string& name, base::FilePath* value) const;
 
   // As part of the initial preferences an optional section indicates the tabs
   // to open during first run. An example is the following:

chrome/installer/util/initial_preferences_constants.cc

@@ -25,6 +25,7 @@ const char kMakeChromeDefault[] = "make_chrome_default";
 const char kMakeChromeDefaultForUser[] = "make_chrome_default_for_user";
 const char kMsi[] = "msi";
 const char kMsiProductId[] = "msi_product_id";
+const char kProgramFilesDir[] = "program_files_dir";
 const char kRequireEula[] = "require_eula";
 const char kSystemLevel[] = "system_level";
 const char kVerboseLogging[] = "verbose_logging";

chrome/installer/util/initial_preferences_constants.h

@@ -53,6 +53,11 @@ extern const char kMsi[];
 // String. The MSI Product ID under which the MSI stores its information.  This
 // is used to update the DisplayVersion to match Chrome's version number.
 extern const char kMsiProductId[];
+// Installs Chrome in the location specified by the msi PROGRAMFILESDIR custom
+// property. The only accepted values are the values mapped to
+// CSIDL_PROGRAM_FILESX86 and CSIDL_PROGRAM_FILES. This property will only be
+// applied on fresh system installs.
+extern const char kProgramFilesDir[];
 // Boolean. Show EULA dialog before install.
 extern const char kRequireEula[];
 // Boolean. Install Chrome to system wise location. Cmd line override present.

chrome/installer/util/initial_preferences_unittest.cc

@@ -79,6 +79,7 @@ TEST_F(InitialPreferencesTest, ParseDistroParams) {
       "     \"do_not_launch_chrome\": true,\n"
       "     \"make_chrome_default\": true,\n"
       "     \"make_chrome_default_for_user\": true,\n"
+      "     \"program_files_dir\": \"c:\\\\bar\",\n"
       "     \"system_level\": true,\n"
       "     \"verbose_logging\": true,\n"
       "     \"require_eula\": true\n"
@@ -117,6 +118,11 @@ TEST_F(InitialPreferencesTest, ParseDistroParams) {
       installer::initial_preferences::kDistroImportBookmarksFromFilePref,
       &str_value));
   EXPECT_STREQ("c:\\foo", str_value.c_str());
+
+  base::FilePath path;
+  EXPECT_TRUE(
+      prefs.GetPath(installer::initial_preferences::kProgramFilesDir, &path));
+  EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("c:\\bar")), path);
 }
 
 TEST_F(InitialPreferencesTest, ParseMissingDistroParams) {