Revert "Install 64-bit browser versions under "C:\Program Files" by default"
This reverts commit 2001eb11359fae01107202c73ffb657f0d918d70.
Reason for revert: Suspected of breaking mini_installer_tests on Win7; see crbug.com/1092356
Original change's description:
> Install 64-bit browser versions under "C:\Program Files" by default
>
> Browsers installed under "C:\Program Files (x86)" remain in that
> directory and will continue to be updated. They must be uninstalled
> first to be reinstalled under "C:\Program Files".
>
> Bug: 380177
> Change-Id: I45b8898ddf39dd7a5fe7e6e0ce258da313ea880f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204420
> Commit-Queue: Yann Dago <ydago@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#775810}
TBR=grt@chromium.org,ydago@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 380177, 1092356
Change-Id: Ic63cc4adcf2285f7e41fa8dc35d7260cf86bc64f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2232949
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776007}
diff --git a/chrome/installer/setup/installer_state.cc b/chrome/installer/setup/installer_state.cc
index 6883665..b5e6e41b 100644
--- a/chrome/installer/setup/installer_state.cc
+++ b/chrome/installer/setup/installer_state.cc
@@ -79,6 +79,8 @@
const bool is_uninstall = command_line.HasSwitch(switches::kUninstall);
+ // TODO(grt): Infer target_path_ from an existing install in support of
+ // varying install locations; see https://crbug.com/380177.
target_path_ = GetChromeInstallPath(system_install());
state_key_ = install_static::GetClientStateKeyPath();
diff --git a/chrome/installer/util/BUILD.gn b/chrome/installer/util/BUILD.gn
index 0b770ce..8010633 100644
--- a/chrome/installer/util/BUILD.gn
+++ b/chrome/installer/util/BUILD.gn
@@ -310,7 +310,6 @@
"experiment_storage_unittest.cc",
"experiment_unittest.cc",
"google_update_settings_unittest.cc",
- "helper_unittest.cc",
"install_util_unittest.cc",
"l10n_string_util_unittest.cc",
"logging_installer_unittest.cc",
diff --git a/chrome/installer/util/helper.cc b/chrome/installer/util/helper.cc
index 1190b20..9019ba9 100644
--- a/chrome/installer/util/helper.cc
+++ b/chrome/installer/util/helper.cc
@@ -5,56 +5,26 @@
#include "chrome/installer/util/helper.h"
#include "base/path_service.h"
-#include "base/win/registry.h"
#include "chrome/install_static/install_util.h"
-#include "chrome/installer/util/install_util.h"
#include "chrome/installer/util/util_constants.h"
-namespace {
-
-base::FilePath GetDefaultChromeInstallPath(bool system_install) {
- base::FilePath install_path;
- int key = system_install ? base::DIR_PROGRAM_FILES : base::DIR_LOCAL_APP_DATA;
- if (base::PathService::Get(key, &install_path)) {
- install_path =
- install_path.Append(install_static::GetChromeInstallSubDirectory());
- install_path = install_path.Append(installer::kInstallBinaryDir);
- }
- return install_path;
-}
-
-base::FilePath GetCurrentInstallPathFromRegistry(bool system_install) {
- base::FilePath install_path;
- if (!InstallUtil::GetChromeVersion(system_install).IsValid())
- return install_path;
-
- std::wstring uninstall_string;
- base::win::RegKey key(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
- install_static::GetClientStateKeyPath().c_str(),
- KEY_QUERY_VALUE | KEY_WOW64_32KEY);
- key.ReadValue(installer::kUninstallStringField, &uninstall_string);
-
- // The UninstallString has the format
- // [InstallPath]/[version]/Installer/setup.exe. In order to get the
- // [InstallPath], the full path must be pruned of the last 3 components.
- if (!uninstall_string.empty()) {
- install_path = base::FilePath(std::move(uninstall_string))
- .DirName()
- .DirName()
- .DirName();
- }
- return install_path;
-}
-
-} // namespace
-
namespace installer {
base::FilePath GetChromeInstallPath(bool system_install) {
- base::FilePath install_path =
- GetCurrentInstallPathFromRegistry(system_install);
- if (install_path.empty())
- install_path = GetDefaultChromeInstallPath(system_install);
+ base::FilePath install_path;
+#if defined(_WIN64)
+ // TODO(wfh): Place Chrome binaries into DIR_PROGRAM_FILESX86 until the code
+ // to support moving the binaries is added.
+ int key =
+ system_install ? base::DIR_PROGRAM_FILESX86 : base::DIR_LOCAL_APP_DATA;
+#else
+ int key = system_install ? base::DIR_PROGRAM_FILES : base::DIR_LOCAL_APP_DATA;
+#endif
+ if (base::PathService::Get(key, &install_path)) {
+ install_path =
+ install_path.Append(install_static::GetChromeInstallSubDirectory());
+ install_path = install_path.Append(kInstallBinaryDir);
+ }
return install_path;
}
diff --git a/chrome/installer/util/helper.h b/chrome/installer/util/helper.h
index 16c4bf7..5dadc32 100644
--- a/chrome/installer/util/helper.h
+++ b/chrome/installer/util/helper.h
@@ -11,14 +11,11 @@
namespace installer {
-// 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
-// Software\Google\Update\ClientState\{appguid}\UninstallString
-// under HKLM if |system_install| is true, HKCU otherwise. If no path was stored
-// in the registry, returns (%ProgramFiles%\[Company\]Product\Application) if
-// |system_install| is true, otherwise returns user specific location
-// (%LOCALAPPDATA%\[Company\]Product\Application).
+// This function returns the install path for Chrome depending on whether its
+// system wide install or user specific install.
+// system_install: if true, the function returns system wide location
+// (ProgramFiles\Google). Otherwise it returns user specific
+// location (Document And Settings\<user>\Local Settings...)
base::FilePath GetChromeInstallPath(bool system_install);
} // namespace installer
diff --git a/chrome/installer/util/helper_unittest.cc b/chrome/installer/util/helper_unittest.cc
deleted file mode 100644
index f02e9b8..0000000
--- a/chrome/installer/util/helper_unittest.cc
+++ /dev/null
@@ -1,146 +0,0 @@
-// Copyright 2020 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/installer/util/helper.h"
-
-#include "base/files/file_path.h"
-#include "base/files/scoped_temp_dir.h"
-#include "base/optional.h"
-#include "base/path_service.h"
-#include "base/test/scoped_path_override.h"
-#include "base/test/test_reg_util_win.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/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> {
- public:
- GetChromeInstallPathTest() = default;
-
- void SetUp() override {
- ASSERT_TRUE(program_files_.CreateUniqueTempDir());
- ASSERT_TRUE(random_.CreateUniqueTempDir());
- ASSERT_TRUE(local_app_data_.CreateUniqueTempDir());
- ASSERT_NO_FATAL_FAILURE(
- registry_override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE));
- ASSERT_NO_FATAL_FAILURE(
- registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
- program_files_override_.emplace(base::DIR_PROGRAM_FILES,
- program_files_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 local_app_data_path() const {
- return local_app_data_.GetPath();
- }
- static bool is_system_level() { return GetParam(); }
-
- base::FilePath GetExpectedPath(bool system_level) {
- auto path = system_level ? program_files_path() : local_app_data_path();
- return path.Append(install_static::GetChromeInstallSubDirectory())
- .Append(kInstallBinaryDir);
- }
-
- static base::win::RegKey GetClientsRegKey() {
- return base::win::RegKey(
- is_system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
- install_static::GetClientsKeyPath().c_str(),
- KEY_SET_VALUE | KEY_WOW64_32KEY);
- }
-
- static base::win::RegKey GetClientStateRegKey() {
- return base::win::RegKey(
- is_system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
- install_static::GetClientStateKeyPath().c_str(),
- KEY_SET_VALUE | KEY_WOW64_32KEY);
- }
-
- private:
- base::ScopedTempDir program_files_;
- 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> local_data_app_override_;
-};
-
-TEST_P(GetChromeInstallPathTest, NoRegistryValue) {
- EXPECT_EQ(GetChromeInstallPath(is_system_level()),
- GetExpectedPath(is_system_level()));
-}
-
-TEST_P(GetChromeInstallPathTest, 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(GetChromeInstallPath(is_system_level()),
- random_path()
- .Append(install_static::GetChromeInstallSubDirectory())
- .Append(kInstallBinaryDir));
-}
-
-TEST_P(GetChromeInstallPathTest, 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(GetChromeInstallPath(!is_system_level()),
- GetExpectedPath(!is_system_level()));
-}
-
-TEST_P(GetChromeInstallPathTest, 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(GetChromeInstallPath(is_system_level()),
- GetExpectedPath(is_system_level()));
-}
-
-INSTANTIATE_TEST_SUITE_P(UserLevelTest,
- GetChromeInstallPathTest,
- testing::Values(false));
-INSTANTIATE_TEST_SUITE_P(SystemLevelTest,
- GetChromeInstallPathTest,
- testing::Values(true));
-
-} // namespace installer
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index 51f0092..8c53f45 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -872,8 +872,11 @@
base::string16 GetInstallationSuffixForModeAtLevel(
const install_static::InstallConstants& mode,
bool system_install) {
- // Search based on the existing install location. If no existing install
- // found, uses the default install location for the mode.
+ // Search based on the default install location for the mode. If we ever
+ // support customizing the install location (https://crbug.com/113987,
+ // https://crbug.com/302491) this will have to change to something else, such
+ // as probing the Omaha keys in the registry to see where the mode is
+ // installed.
const base::FilePath chrome_exe =
installer::GetChromeInstallPath(system_install)
.Append(installer::kChromeExe);
diff --git a/chrome/test/mini_installer/variable_expander.py b/chrome/test/mini_installer/variable_expander.py
index 04a6dba..f9be0137 100644
--- a/chrome/test/mini_installer/variable_expander.py
+++ b/chrome/test/mini_installer/variable_expander.py
@@ -157,46 +157,31 @@
previous_version_mini_installer_path)
windows_major_ver, windows_minor_ver, _, _, _ = win32api.GetVersionEx()
self._variable_mapping = {
- 'CHROMEDRIVER_PATH':
- chromedriver_path,
- 'QUIET':
- '-q' if quiet else '',
- 'OUTPUT_DIR':
- '"--output-dir=%s"' % output_dir if output_dir else '',
- 'LOCAL_APPDATA':
- shell.SHGetFolderPath(0, shellcon.CSIDL_LOCAL_APPDATA, None, 0),
- 'LOG_FILE':
- '',
- 'MINI_INSTALLER':
- mini_installer_abspath,
- 'MINI_INSTALLER_FILE_VERSION':
- _GetFileVersion(mini_installer_abspath),
- 'MINI_INSTALLER_BITNESS':
- _GetFileBitness(mini_installer_abspath),
+ 'CHROMEDRIVER_PATH': chromedriver_path,
+ 'QUIET': '-q' if quiet else '',
+ 'OUTPUT_DIR': '"--output-dir=%s"' % output_dir if output_dir else '',
+ 'LOCAL_APPDATA': shell.SHGetFolderPath(0, shellcon.CSIDL_LOCAL_APPDATA,
+ None, 0),
+ 'LOG_FILE': '',
+ 'MINI_INSTALLER': mini_installer_abspath,
+ 'MINI_INSTALLER_FILE_VERSION': _GetFileVersion(mini_installer_abspath),
+ 'MINI_INSTALLER_BITNESS': _GetFileBitness(mini_installer_abspath),
'PREVIOUS_VERSION_MINI_INSTALLER':
- previous_version_mini_installer_abspath,
- 'PREVIOUS_VERSION_MINI_INSTALLER_FILE_VERSION':
- _GetFileVersion(previous_version_mini_installer_abspath),
- 'PROGRAM_FILES':
- shell.SHGetFolderPath(0, shellcon.CSIDL_PROGRAM_FILES, None, 0),
- 'USER_SPECIFIC_REGISTRY_SUFFIX':
- _GetUserSpecificRegistrySuffix(),
- 'VERSION_SERVER_2003':
- '(5, 2)',
- 'VERSION_VISTA':
- '(6, 0)',
- 'VERSION_WIN10':
- '(10, 0)',
- 'VERSION_WIN7':
- '(6, 1)',
- 'VERSION_WIN8':
- '(6, 2)',
- 'VERSION_WIN8_1':
- '(6, 3)',
- 'VERSION_XP':
- '(5, 1)',
- 'WINDOWS_VERSION':
- '(%s, %s)' % (windows_major_ver, windows_minor_ver)
+ previous_version_mini_installer_abspath,
+ 'PREVIOUS_VERSION_MINI_INSTALLER_FILE_VERSION': _GetFileVersion(
+ previous_version_mini_installer_abspath),
+ 'PROGRAM_FILES': shell.SHGetFolderPath(0,
+ shellcon.CSIDL_PROGRAM_FILESX86,
+ None, 0),
+ 'USER_SPECIFIC_REGISTRY_SUFFIX': _GetUserSpecificRegistrySuffix(),
+ 'VERSION_SERVER_2003': '(5, 2)',
+ 'VERSION_VISTA': '(6, 0)',
+ 'VERSION_WIN10': '(10, 0)',
+ 'VERSION_WIN7': '(6, 1)',
+ 'VERSION_WIN8': '(6, 2)',
+ 'VERSION_WIN8_1': '(6, 3)',
+ 'VERSION_XP': '(5, 1)',
+ 'WINDOWS_VERSION': '(%s, %s)' % (windows_major_ver, windows_minor_ver)
}
mini_installer_product_name = _GetProductName(mini_installer_abspath)
