From f6d82f88e62b6b888d9bd41f3dd87bba1bf25d48 Mon Sep 17 00:00:00 2001 From: Mila Green Date: Fri, 5 Mar 2021 22:14:29 +0000 Subject: [PATCH] Updater: Parameterize IntegrationTest. Bug: 1096654 Change-Id: Ia86a3a4f1355812cd32d89bcc34f5a220b5673ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2642930 Commit-Queue: Mila Green Reviewed-by: Sorin Jianu Reviewed-by: Joshua Pawlicki Cr-Commit-Position: refs/heads/master@{#860393} --- chrome/updater/BUILD.gn | 59 +- chrome/updater/activity.cc | 18 +- chrome/updater/activity.h | 5 +- chrome/updater/activity_impl.h | 6 +- chrome/updater/activity_impl_mac.cc | 28 +- chrome/updater/activity_impl_win.cc | 65 ++- chrome/updater/app/app.cc | 7 +- chrome/updater/app/app.h | 6 + chrome/updater/app/app_install.cc | 2 +- chrome/updater/app/app_install_mac.mm | 3 +- chrome/updater/app/app_server.cc | 3 + chrome/updater/app/app_server_unittest.cc | 14 +- chrome/updater/app/app_uninstall.cc | 6 +- chrome/updater/app/app_update.cc | 3 +- chrome/updater/app/server/mac/server.mm | 4 +- chrome/updater/app/server/win/server.cc | 16 +- chrome/updater/configurator.cc | 4 +- chrome/updater/crash_client.cc | 7 +- chrome/updater/crash_reporter.cc | 7 +- .../updater/device_management/dm_storage.cc | 8 +- chrome/updater/external_constants_builder.cc | 7 +- .../external_constants_builder_unittest.cc | 8 +- chrome/updater/external_constants_override.cc | 7 +- chrome/updater/installer.cc | 41 +- chrome/updater/installer.h | 3 +- chrome/updater/launchd_util.cc | 19 +- chrome/updater/launchd_util.h | 4 +- chrome/updater/mac/BUILD.gn | 17 - chrome/updater/mac/mac_util.h | 71 +++ chrome/updater/mac/mac_util.mm | 183 ++++++ chrome/updater/mac/setup/setup.h | 13 +- chrome/updater/mac/setup/setup.mm | 276 +++++---- .../mac/update_service_internal_proxy.h | 4 +- .../mac/update_service_internal_proxy.mm | 8 +- chrome/updater/mac/update_service_proxy.h | 4 +- chrome/updater/mac/update_service_proxy.mm | 8 +- .../updater/mac/update_service_proxy_test.mm | 4 +- chrome/updater/mac/util.h | 52 -- chrome/updater/mac/util.mm | 112 ---- chrome/updater/prefs.cc | 15 +- chrome/updater/prefs_win.cc | 13 +- chrome/updater/service_factory_mac.mm | 2 +- chrome/updater/service_factory_win.cc | 2 +- chrome/updater/service_scope.h | 30 - chrome/updater/setup.h | 3 +- chrome/updater/setup_mac.mm | 14 +- chrome/updater/setup_win.cc | 9 +- chrome/updater/test/integration_tests.cc | 541 ++++++++++++++---- chrome/updater/test/integration_tests.h | 98 +++- .../updater/test/integration_tests_helper.cc | 235 ++++++++ chrome/updater/test/integration_tests_mac.mm | 248 +++++--- chrome/updater/test/integration_tests_win.cc | 92 ++- chrome/updater/test/test_app/test_app_mac.mm | 6 + chrome/updater/update_service.h | 9 - chrome/updater/updater.cc | 10 +- chrome/updater/updater_scope.h | 41 ++ ..._unittest.cc => updater_scope_unittest.cc} | 8 +- chrome/updater/util.cc | 76 ++- chrome/updater/util.h | 11 +- chrome/updater/win/setup/setup.cc | 31 +- chrome/updater/win/setup/setup.h | 4 +- chrome/updater/win/setup/uninstall.cc | 21 +- chrome/updater/win/setup/uninstall.h | 6 +- chrome/updater/win/task_scheduler_unittest.cc | 4 +- chrome/updater/win/ui/ui.cc | 3 +- chrome/updater/win/ui/ui.h | 7 +- chrome/updater/win/ui/ui_displayed_event.cc | 15 +- chrome/updater/win/ui/ui_displayed_event.h | 7 +- .../win/update_service_internal_proxy.cc | 3 +- .../win/update_service_internal_proxy.h | 4 +- chrome/updater/win/update_service_proxy.cc | 5 +- chrome/updater/win/update_service_proxy.h | 4 +- chrome/updater/win/util.cc | 31 +- chrome/updater/win/util.h | 7 +- 74 files changed, 1876 insertions(+), 851 deletions(-) create mode 100644 chrome/updater/mac/mac_util.h create mode 100644 chrome/updater/mac/mac_util.mm delete mode 100644 chrome/updater/mac/util.h delete mode 100644 chrome/updater/mac/util.mm delete mode 100644 chrome/updater/service_scope.h create mode 100644 chrome/updater/test/integration_tests_helper.cc create mode 100644 chrome/updater/updater_scope.h rename chrome/updater/{service_scope_unittest.cc => updater_scope_unittest.cc} (76%) diff --git a/chrome/updater/BUILD.gn b/chrome/updater/BUILD.gn index f668df995e13e5..29ab8763b0350b 100644 --- a/chrome/updater/BUILD.gn +++ b/chrome/updater/BUILD.gn @@ -64,13 +64,13 @@ if (is_win || is_mac) { "policy_manager.h", "registration_data.cc", "registration_data.h", - "service_scope.h", "splash_screen.h", "unzipper.cc", "unzipper.h", "update_service.cc", "update_service.h", "update_service_internal.h", + "updater_scope.h", "util.cc", "util.h", ] @@ -89,6 +89,17 @@ if (is_win || is_mac) { "//third_party/zlib/google:zip", "//url", ] + + if (is_mac) { + sources += [ + "mac/mac_util.h", + "mac/mac_util.mm", + ] + + deps += [ "//chrome/common/mac:launchd" ] + + frameworks = [ "Foundation.framework" ] + } } # Use this source set for code which has platform-specific modules. @@ -224,7 +235,6 @@ if (is_win || is_mac) { "//chrome/updater/mac:installer_sources", "//chrome/updater/mac:network_fetcher_sources", "//chrome/updater/mac:updater_setup_sources", - "//chrome/updater/mac:util", "//chrome/updater/mac:xpc_names", ] } @@ -326,10 +336,8 @@ if (is_win || is_mac) { ] } - # These tests are run serially since they mutate system state. - test("updater_tests") { + source_set("updater_test_sources") { testonly = true - sources = [ "app/app_server_unittest.cc", "enum_traits_unittest.cc", @@ -340,8 +348,6 @@ if (is_win || is_mac) { "policy_manager_unittest.cc", "policy_service_unittest.cc", "prefs_unittest.cc", - "run_all_unittests.cc", - "service_scope_unittest.cc", "tag_unittest.cc", "test/integration_tests.cc", "test/integration_tests.h", @@ -349,6 +355,7 @@ if (is_win || is_mac) { "test/server.h", "unittest_util_unittest.cc", "update_service_unittest.cc", + "updater_scope_unittest.cc", "updater_unittest.cc", ] @@ -407,7 +414,6 @@ if (is_win || is_mac) { "//chrome/updater/mac:updater_bundle", "//chrome/updater/mac:updater_setup_tests", "//chrome/updater/mac:updater_tests", - "//chrome/updater/mac:util", "//chrome/updater/mac:xpc_names", "//third_party/ocmock", ] @@ -419,6 +425,43 @@ if (is_win || is_mac) { } } + # These tests are run serially since they mutate system state. + test("updater_tests") { + testonly = true + + sources = [ "run_all_unittests.cc" ] + + deps = [ + ":updater_test_sources", + "//base/test:test_support", + "//chrome/common:constants", + "//testing/gtest", + ] + + data_deps = [ ":updater_integration_tests_helper" ] + + if (is_mac) { + data_deps += [ + "//chrome/updater/mac:updater_bundle", + "//chrome/updater/test/test_app", + ] + } + } + + executable("updater_integration_tests_helper") { + testonly = true + + sources = [ "test/integration_tests_helper.cc" ] + + deps = [ + ":base", + ":lib", + ":updater_test_sources", + "//base", + "//url", + ] + } + process_version("branding_header_py") { process_only = true diff --git a/chrome/updater/activity.cc b/chrome/updater/activity.cc index e6ad40bd980a6e..3622a51789e45c 100644 --- a/chrome/updater/activity.cc +++ b/chrome/updater/activity.cc @@ -14,14 +14,14 @@ #include "base/task/task_traits.h" #include "base/task/thread_pool.h" #include "chrome/updater/activity_impl.h" +#include "chrome/updater/updater_scope.h" namespace updater { namespace { constexpr int kDaysUnknown = -2; } -ActivityDataService::ActivityDataService(bool is_machine) - : is_machine_(is_machine) {} +ActivityDataService::ActivityDataService(UpdaterScope scope) : scope_(scope) {} void ActivityDataService::GetActiveBits( const std::vector& ids, @@ -29,15 +29,15 @@ void ActivityDataService::GetActiveBits( base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce( - [](const std::vector& ids, bool is_machine) { + [](UpdaterScope scope, const std::vector& ids) { std::set result; for (const auto& id : ids) { - if (GetActiveBit(id, is_machine)) + if (GetActiveBit(scope, id)) result.insert(id); } return result; }, - ids, is_machine_), + scope_, ids), std::move(callback)); } @@ -47,16 +47,16 @@ void ActivityDataService::GetAndClearActiveBits( base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce( - [](const std::vector& ids, bool is_machine) { + [](UpdaterScope scope, const std::vector& ids) { std::set result; for (const auto& id : ids) { - if (GetActiveBit(id, is_machine)) + if (GetActiveBit(scope, id)) result.insert(id); - ClearActiveBit(id, is_machine); + ClearActiveBit(scope, id); } return result; }, - ids, is_machine_), + scope_, ids), std::move(callback)); } diff --git a/chrome/updater/activity.h b/chrome/updater/activity.h index 4afe9a4fbc6663..1ea607ac8e16c5 100644 --- a/chrome/updater/activity.h +++ b/chrome/updater/activity.h @@ -10,13 +10,14 @@ #include #include "base/callback_forward.h" +#include "chrome/updater/updater_scope.h" #include "components/update_client/activity_data_service.h" namespace updater { class ActivityDataService final : public update_client::ActivityDataService { public: - explicit ActivityDataService(bool is_machine); + explicit ActivityDataService(UpdaterScope scope); ActivityDataService(const ActivityDataService&) = delete; ActivityDataService& operator=(const ActivityDataService&) = delete; ~ActivityDataService() override = default; @@ -35,7 +36,7 @@ class ActivityDataService final : public update_client::ActivityDataService { int GetDaysSinceLastRollCall(const std::string& id) const override; private: - bool is_machine_; + UpdaterScope scope_; }; } // namespace updater diff --git a/chrome/updater/activity_impl.h b/chrome/updater/activity_impl.h index 11e4ca527dcab1..a424fb6f725751 100644 --- a/chrome/updater/activity_impl.h +++ b/chrome/updater/activity_impl.h @@ -7,11 +7,13 @@ #include +#include "chrome/updater/updater_scope.h" + namespace updater { -bool GetActiveBit(const std::string& id, bool is_machine_); +bool GetActiveBit(UpdaterScope scope, const std::string& id); -void ClearActiveBit(const std::string& id, bool is_machine_); +void ClearActiveBit(UpdaterScope scope, const std::string& id); } // namespace updater diff --git a/chrome/updater/activity_impl_mac.cc b/chrome/updater/activity_impl_mac.cc index 4a00dcae07b9e3..b7202953dd7ea1 100644 --- a/chrome/updater/activity_impl_mac.cc +++ b/chrome/updater/activity_impl_mac.cc @@ -8,12 +8,15 @@ #include "base/files/file_util.h" #include "base/logging.h" +#include "chrome/updater/mac/mac_util.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" namespace updater { namespace { -base::FilePath GetActiveFile(const std::string& id) { +base::FilePath GetActiveFile(UpdaterScope scope, const std::string& id) { + // TODO(crbug.com/1096654): Add support for UpdaterScope::kSystem. return base::GetHomeDir() .AppendASCII("Library") .AppendASCII(COMPANY_SHORTNAME_STRING) @@ -24,25 +27,14 @@ base::FilePath GetActiveFile(const std::string& id) { } // namespace -bool GetActiveBit(const std::string& id, bool is_machine_) { - if (is_machine_) { - // TODO(crbug.com/1096654): Add support for the machine case. Machine - // installs must look for values in each home dir. - return false; - } else { - base::FilePath path = GetActiveFile(id); - return base::PathExists(path) && base::PathIsWritable(path); - } +bool GetActiveBit(UpdaterScope scope, const std::string& id) { + const base::FilePath path = GetActiveFile(scope, id); + return base::PathExists(path) && base::PathIsWritable(path); } -void ClearActiveBit(const std::string& id, bool is_machine_) { - if (is_machine_) { - // TODO(crbug.com/1096654): Add support for the machine case. Machine - // installs must clear values in each home dir. - } else { - if (!base::DeleteFile(GetActiveFile(id))) - VLOG(2) << "Failed to clear activity bit at " << GetActiveFile(id); - } +void ClearActiveBit(UpdaterScope scope, const std::string& id) { + if (!base::DeleteFile(GetActiveFile(scope, id))) + VLOG(2) << "Failed to clear activity bit at " << GetActiveFile(scope, id); } } // namespace updater diff --git a/chrome/updater/activity_impl_win.cc b/chrome/updater/activity_impl_win.cc index 639bfd83f47bcb..90774dc14b6c8d 100644 --- a/chrome/updater/activity_impl_win.cc +++ b/chrome/updater/activity_impl_win.cc @@ -12,6 +12,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/win/registry.h" #include "base/win/windows_types.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/constants.h" namespace updater { @@ -24,41 +25,47 @@ std::wstring GetAppClientStateKey(const std::string& id) { } // namespace -bool GetActiveBit(const std::string& id, bool is_machine_) { - if (is_machine_) { - // TODO(crbug.com/1096654): Add support for the machine case. Machine - // installs must look for values under HKLM and under every HKU\. - return false; - } else { - // TODO(crbug/1159498): Standardize registry access. - base::win::RegKey key; - if (key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), - KEY_READ | KEY_WOW64_32KEY) == ERROR_SUCCESS) { - std::wstring value; - if (key.ReadValue(kDidRun, &value) == ERROR_SUCCESS && value == L"1") - return true; +bool GetActiveBit(UpdaterScope scope, const std::string& id) { + switch (scope) { + case UpdaterScope::kUser: { + // TODO(crbug/1159498): Standardize registry access. + base::win::RegKey key; + if (key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), + KEY_READ | KEY_WOW64_32KEY) == ERROR_SUCCESS) { + std::wstring value; + if (key.ReadValue(kDidRun, &value) == ERROR_SUCCESS && value == L"1") + return true; + } + return false; } - return false; + case UpdaterScope::kSystem: + // TODO(crbug.com/1096654): Add support for the machine case. Machine + // installs must look for values under HKLM and under every HKU\. + return false; } } -void ClearActiveBit(const std::string& id, bool is_machine_) { - if (is_machine_) { - // TODO(crbug.com/1096654): Add support for the machine case. Machine - // installs must clear values under HKLM and under every HKU\. - } else { - // TODO(crbug/1159498): Standardize registry access. - base::win::RegKey key; - if (key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), - KEY_WRITE | KEY_WOW64_32KEY) == ERROR_SUCCESS) { - const LONG result = key.WriteValue(kDidRun, L"0"); - if (result) { - VLOG(2) << "Failed to clear HKCU activity key for " << id << ": " - << result; +void ClearActiveBit(UpdaterScope scope, const std::string& id) { + switch (scope) { + case UpdaterScope::kUser: { + // TODO(crbug/1159498): Standardize registry access. + base::win::RegKey key; + if (key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), + KEY_WRITE | KEY_WOW64_32KEY) == ERROR_SUCCESS) { + const LONG result = key.WriteValue(kDidRun, L"0"); + if (result) { + VLOG(2) << "Failed to clear HKCU activity key for " << id << ": " + << result; + } + } else { + VLOG(2) << "Failed to open HKCU activity key with write for " << id; } - } else { - VLOG(2) << "Failed to open HKCU activity key with write for " << id; + break; } + case UpdaterScope::kSystem: + // TODO(crbug.com/1096654): Add support for the machine case. Machine + // installs must clear values under HKLM and under every HKU\. + break; } } diff --git a/chrome/updater/app/app.cc b/chrome/updater/app/app.cc index 1f32f0c24e1a16..e63fa73c96aa4f 100644 --- a/chrome/updater/app/app.cc +++ b/chrome/updater/app/app.cc @@ -11,12 +11,13 @@ #include "base/run_loop.h" #include "base/task/thread_pool/thread_pool_instance.h" #include "base/threading/thread_restrictions.h" +#include "chrome/updater/updater_scope.h" namespace updater { constexpr base::StringPiece App::kThreadPoolName; -App::App() = default; +App::App() : updater_scope_(GetProcessScope()) {} App::~App() = default; void App::InitializeThreadPool() { @@ -50,4 +51,8 @@ void App::Shutdown(int exit_code) { std::move(quit_).Run(exit_code); } +UpdaterScope App::updater_scope() const { + return updater_scope_; +} + } // namespace updater diff --git a/chrome/updater/app/app.h b/chrome/updater/app/app.h index 29494c8c1242c1..4e4c6bbf4a0d5a 100644 --- a/chrome/updater/app/app.h +++ b/chrome/updater/app/app.h @@ -9,6 +9,7 @@ #include "base/memory/ref_counted.h" #include "base/no_destructor.h" #include "base/strings/string_piece.h" +#include "chrome/updater/updater_scope.h" namespace updater { @@ -51,6 +52,8 @@ class App : public base::RefCountedThreadSafe { // will exit with the specified code. void Shutdown(int exit_code); + UpdaterScope updater_scope() const; + private: // Allows initialization of the thread pool for specific environments, in // cases where the thread pool must be started with different init parameters, @@ -68,6 +71,9 @@ class App : public base::RefCountedThreadSafe { // A callback that quits the main sequence runloop. base::OnceCallback quit_; + + // The scope in which the updater is running. + UpdaterScope updater_scope_; }; } // namespace updater diff --git a/chrome/updater/app/app_install.cc b/chrome/updater/app/app_install.cc index 7984761e35e371..3db76036593697 100644 --- a/chrome/updater/app/app_install.cc +++ b/chrome/updater/app/app_install.cc @@ -106,7 +106,7 @@ void AppInstall::GetVersionDone(scoped_refptr, } InstallCandidate( - false, + updater_scope(), base::BindOnce( [](SplashScreen* splash_screen, base::OnceCallback done, int result) { diff --git a/chrome/updater/app/app_install_mac.mm b/chrome/updater/app/app_install_mac.mm index e7496ef45da72b..50e18d31d4a693 100644 --- a/chrome/updater/app/app_install_mac.mm +++ b/chrome/updater/app/app_install_mac.mm @@ -9,13 +9,14 @@ #include "base/time/time.h" #include "chrome/updater/constants.h" #include "chrome/updater/launchd_util.h" +#import "chrome/updater/mac/mac_util.h" #include "chrome/updater/mac/xpc_service_names.h" namespace updater { void AppInstall::WakeCandidateDone() { PollLaunchctlList( - kUpdateServiceLaunchdName, LaunchctlPresence::kPresent, + updater_scope(), kUpdateServiceLaunchdName, LaunchctlPresence::kPresent, base::TimeDelta::FromSeconds(kWaitForLaunchctlUpdateSec), base::BindOnce([](scoped_refptr installer, bool unused) { installer->MaybeInstallApp(); }, diff --git a/chrome/updater/app/app_server.cc b/chrome/updater/app/app_server.cc index e584c6709a53c0..d48378ac676ec7 100644 --- a/chrome/updater/app/app_server.cc +++ b/chrome/updater/app/app_server.cc @@ -26,6 +26,7 @@ #include "chrome/updater/update_service_internal.h" #include "chrome/updater/update_service_internal_impl.h" #include "chrome/updater/update_service_internal_impl_inactive.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/updater_version.h" #include "components/prefs/pref_service.h" @@ -102,6 +103,8 @@ void AppServer::MaybeUninstall() { base::CommandLine command_line( base::CommandLine::ForCurrentProcess()->GetProgram()); command_line.AppendSwitch(kUninstallIfUnusedSwitch); + if (updater_scope() == UpdaterScope::kSystem) + command_line.AppendSwitch(kSystemSwitch); command_line.AppendSwitch("--enable-logging"); command_line.AppendSwitchASCII("--vmodule", "*/chrome/updater/*=2"); DVLOG(2) << "Launching uninstall command: " diff --git a/chrome/updater/app/app_server_unittest.cc b/chrome/updater/app/app_server_unittest.cc index 87b6ecb865ac04..ef4f072b52a988 100644 --- a/chrome/updater/app/app_server_unittest.cc +++ b/chrome/updater/app/app_server_unittest.cc @@ -10,6 +10,7 @@ #include "base/files/file_util.h" #include "base/memory/scoped_refptr.h" #include "base/message_loop/message_pump_type.h" +#include "base/optional.h" #include "base/task/single_thread_task_executor.h" #include "base/task/thread_pool/thread_pool_instance.h" #include "chrome/updater/constants.h" @@ -56,13 +57,12 @@ class AppServerTest : public AppServer { }; void ClearPrefs() { - base::FilePath prefs_dir; - ASSERT_TRUE(GetBaseDirectory(&prefs_dir)); - ASSERT_TRUE( - base::DeleteFile(prefs_dir.Append(FILE_PATH_LITERAL("prefs.json")))); - ASSERT_TRUE(GetVersionedDirectory(&prefs_dir)); - ASSERT_TRUE( - base::DeleteFile(prefs_dir.Append(FILE_PATH_LITERAL("prefs.json")))); + for (const base::Optional& path : + {GetBaseDirectory(), GetVersionedDirectory()}) { + ASSERT_TRUE(path); + ASSERT_TRUE( + base::DeleteFile(path->Append(FILE_PATH_LITERAL("prefs.json")))); + } } class AppServerTestCase : public testing::Test { diff --git a/chrome/updater/app/app_uninstall.cc b/chrome/updater/app/app_uninstall.cc index 4111065b1c0a1d..70414511b24259 100644 --- a/chrome/updater/app/app_uninstall.cc +++ b/chrome/updater/app/app_uninstall.cc @@ -58,7 +58,8 @@ void AppUninstall::FirstTaskRun() { // TODO(crbug.com/1114719): Implement --uninstall-self for Win. if (command_line->HasSwitch(kUninstallSelfSwitch)) { base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, base::BindOnce(&UninstallCandidate), + FROM_HERE, {base::MayBlock()}, + base::BindOnce(&UninstallCandidate, updater_scope()), base::BindOnce(&AppUninstall::Shutdown, this)); return; } @@ -79,7 +80,8 @@ void AppUninstall::FirstTaskRun() { if (can_uninstall) { base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, base::BindOnce(&Uninstall, false), + FROM_HERE, {base::MayBlock()}, + base::BindOnce(&Uninstall, updater_scope()), base::BindOnce(&AppUninstall::Shutdown, this)); return; } diff --git a/chrome/updater/app/app_update.cc b/chrome/updater/app/app_update.cc index e1678104bb98b2..4d488bbc00175b 100644 --- a/chrome/updater/app/app_update.cc +++ b/chrome/updater/app/app_update.cc @@ -41,7 +41,8 @@ void AppUpdate::Uninitialize() { } void AppUpdate::FirstTaskRun() { - InstallCandidate(false, base::BindOnce(&AppUpdate::SetupDone, this)); + InstallCandidate(updater_scope(), + base::BindOnce(&AppUpdate::SetupDone, this)); } void AppUpdate::SetupDone(int result) { diff --git a/chrome/updater/app/server/mac/server.mm b/chrome/updater/app/server/mac/server.mm index ba294d81aab87e..cc3d89ac3a9926 100644 --- a/chrome/updater/app/server/mac/server.mm +++ b/chrome/updater/app/server/mac/server.mm @@ -94,11 +94,11 @@ } void AppServerMac::UninstallSelf() { - UninstallCandidate(); + UninstallCandidate(updater_scope()); } bool AppServerMac::SwapRPCInterfaces() { - return PromoteCandidate() == setup_exit_codes::kSuccess; + return PromoteCandidate(updater_scope()) == setup_exit_codes::kSuccess; } void AppServerMac::TaskStarted() { diff --git a/chrome/updater/app/server/win/server.cc b/chrome/updater/app/server/win/server.cc index afa4869ae8de78..a9e5ceede0838b 100644 --- a/chrome/updater/app/server/win/server.cc +++ b/chrome/updater/app/server/win/server.cc @@ -14,6 +14,7 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/system/sys_info.h" #include "base/task/task_traits.h" #include "base/task/thread_pool.h" @@ -26,6 +27,7 @@ #include "chrome/updater/prefs.h" #include "chrome/updater/update_service.h" #include "chrome/updater/update_service_internal.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/util.h" #include "chrome/updater/win/constants.h" #include "chrome/updater/win/setup/setup_util.h" @@ -177,32 +179,32 @@ void ComServerApp::ActiveDuty( } void ComServerApp::UninstallSelf() { - // TODO(crbug.com/1096654): Add support for is_machine. - UninstallCandidate(false); + // TODO(crbug.com/1096654): Add support for UpdaterScope::kSystem. + UninstallCandidate(UpdaterScope::kUser); } bool ComServerApp::SwapRPCInterfaces() { std::unique_ptr list(WorkItem::CreateWorkItemList()); - base::FilePath versioned_directory; - if (!GetVersionedDirectory(&versioned_directory)) + base::Optional versioned_directory = GetVersionedDirectory(); + if (!versioned_directory) return false; for (const CLSID& clsid : GetActiveServers()) { // TODO(crbug.com/1096654): Use HKLM for system. AddInstallServerWorkItems( HKEY_CURRENT_USER, clsid, - versioned_directory.Append(FILE_PATH_LITERAL("updater.exe")), + versioned_directory->Append(FILE_PATH_LITERAL("updater.exe")), list.get()); } - // TODO(crbug.com/1096654): Add support for is_machine: A call to + // TODO(crbug.com/1096654): Add support for UpdaterScope::kSystem: A call to // AddComServiceWorkItems is needed. for (const GUID& iid : GetActiveInterfaces()) { // TODO(crbug.com/1096654): Use HKLM for system. AddInstallComInterfaceWorkItems( HKEY_CURRENT_USER, - versioned_directory.Append(FILE_PATH_LITERAL("updater.exe")), iid, + versioned_directory->Append(FILE_PATH_LITERAL("updater.exe")), iid, list.get()); } diff --git a/chrome/updater/configurator.cc b/chrome/updater/configurator.cc index 371ccd79c52b73..f11b0260b0feca 100644 --- a/chrome/updater/configurator.cc +++ b/chrome/updater/configurator.cc @@ -17,6 +17,7 @@ #include "chrome/updater/patcher.h" #include "chrome/updater/prefs.h" #include "chrome/updater/unzipper.h" +#include "chrome/updater/updater_scope.h" #include "components/prefs/pref_service.h" #include "components/update_client/network.h" #include "components/update_client/patcher.h" @@ -47,7 +48,8 @@ namespace updater { Configurator::Configurator(std::unique_ptr prefs) : prefs_(std::move(prefs)), external_constants_(CreateExternalConstants()), - activity_data_service_(std::make_unique(false)), + activity_data_service_( + std::make_unique(GetProcessScope())), unzip_factory_(base::MakeRefCounted()), patch_factory_(base::MakeRefCounted()) {} Configurator::~Configurator() = default; diff --git a/chrome/updater/crash_client.cc b/chrome/updater/crash_client.cc index 2757bec1e48fa0..30165bcbfeea73 100644 --- a/chrome/updater/crash_client.cc +++ b/chrome/updater/crash_client.cc @@ -10,6 +10,7 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/no_destructor.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/strings/string_util.h" #include "build/build_config.h" @@ -54,13 +55,13 @@ bool CrashClient::InitializeDatabaseOnly() { base::FilePath handler_path; base::PathService::Get(base::FILE_EXE, &handler_path); - base::FilePath database_path; - if (!GetVersionedDirectory(&database_path)) { + base::Optional database_path = GetVersionedDirectory(); + if (!database_path) { LOG(ERROR) << "Failed to get the database path."; return false; } - database_ = crashpad::CrashReportDatabase::Initialize(database_path); + database_ = crashpad::CrashReportDatabase::Initialize(*database_path); if (!database_) { LOG(ERROR) << "Failed to initialize Crashpad database."; return false; diff --git a/chrome/updater/crash_reporter.cc b/chrome/updater/crash_reporter.cc index c139f54dec74b3..622b4d26671947 100644 --- a/chrome/updater/crash_reporter.cc +++ b/chrome/updater/crash_reporter.cc @@ -11,6 +11,7 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/logging.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/stl_util.h" #include "base/strings/strcat.h" @@ -60,8 +61,8 @@ void StartCrashReporter(const std::string& version) { base::FilePath handler_path; base::PathService::Get(base::FILE_EXE, &handler_path); - base::FilePath database_path; - if (!GetVersionedDirectory(&database_path)) { + base::Optional database_path = GetVersionedDirectory(); + if (!database_path) { LOG(DFATAL) << "Failed to get the database path."; return; } @@ -75,7 +76,7 @@ void StartCrashReporter(const std::string& version) { // TODO(crbug.com/1163583): use the production front end instead of staging. crashpad::CrashpadClient* client = GetCrashpadClient(); - if (!client->StartHandler(handler_path, database_path, + if (!client->StartHandler(handler_path, *database_path, /*metrics_dir=*/base::FilePath(), CRASH_STAGING_UPLOAD_URL, annotations, arguments, /*restartable=*/true, diff --git a/chrome/updater/device_management/dm_storage.cc b/chrome/updater/device_management/dm_storage.cc index 498c632a8952a2..54557d5fbe1e34 100644 --- a/chrome/updater/device_management/dm_storage.cc +++ b/chrome/updater/device_management/dm_storage.cc @@ -11,6 +11,7 @@ #include "base/files/file_enumerator.h" #include "base/files/file_util.h" #include "base/files/important_file_writer.h" +#include "base/optional.h" #include "base/strings/string16.h" #include "base/strings/sys_string_conversions.h" #include "chrome/updater/device_management/dm_cached_policy_info.h" @@ -190,12 +191,13 @@ std::unique_ptr DMStorage::GetOmahaPolicyManager() { } scoped_refptr GetDefaultDMStorage() { - base::FilePath updater_versioned_path; - if (!GetVersionedDirectory(&updater_versioned_path)) + base::Optional updater_versioned_path = + GetVersionedDirectory(); + if (!updater_versioned_path) return nullptr; base::FilePath policy_cache_folder = - updater_versioned_path.AppendASCII(kPolicyCacheSubfolder); + updater_versioned_path->AppendASCII(kPolicyCacheSubfolder); return base::MakeRefCounted(policy_cache_folder); } diff --git a/chrome/updater/external_constants_builder.cc b/chrome/updater/external_constants_builder.cc index 1a8c4c5285eb31..e99093ac522a15 100644 --- a/chrome/updater/external_constants_builder.cc +++ b/chrome/updater/external_constants_builder.cc @@ -9,6 +9,7 @@ #include "base/json/json_file_value_serializer.h" #include "base/logging.h" +#include "base/optional.h" #include "chrome/updater/constants.h" #include "chrome/updater/util.h" @@ -71,13 +72,13 @@ ExternalConstantsBuilder::ClearServerKeepAliveSeconds() { } bool ExternalConstantsBuilder::Overwrite() { - base::FilePath base_path; - if (!GetBaseDirectory(&base_path)) { + base::Optional base_path = GetBaseDirectory(); + if (!base_path) { LOG(ERROR) << "Can't find base directory; can't save constant overrides."; return false; } const base::FilePath override_file_path = - base_path.AppendASCII(kDevOverrideFileName); + base_path.value().AppendASCII(kDevOverrideFileName); bool ok = JSONFileValueSerializer(override_file_path).Serialize(overrides_); written_ = written_ || ok; return ok; diff --git a/chrome/updater/external_constants_builder_unittest.cc b/chrome/updater/external_constants_builder_unittest.cc index b901bec011e99a..45f622d27dd7f7 100644 --- a/chrome/updater/external_constants_builder_unittest.cc +++ b/chrome/updater/external_constants_builder_unittest.cc @@ -11,6 +11,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" +#include "base/optional.h" #include "chrome/updater/constants.h" #include "chrome/updater/external_constants.h" #include "chrome/updater/external_constants_builder.h" @@ -24,11 +25,12 @@ namespace updater { namespace { void DeleteOverridesFile() { - base::FilePath target; - if (!GetBaseDirectory(&target)) { + base::Optional target = GetBaseDirectory(); + if (!target) { LOG(ERROR) << "Could not get base directory to clean out overrides file."; + return; } - if (!base::DeleteFile(target.AppendASCII(kDevOverrideFileName))) { + if (!base::DeleteFile(target->AppendASCII(kDevOverrideFileName))) { // Note: base::DeleteFile returns `true` if there is no such file, which // is what we want; "file already doesn't exist" is not an error here. LOG(ERROR) << "Could not delete override file."; diff --git a/chrome/updater/external_constants_override.cc b/chrome/updater/external_constants_override.cc index a1578e7571e628..085cfd67d8540e 100644 --- a/chrome/updater/external_constants_override.cc +++ b/chrome/updater/external_constants_override.cc @@ -13,6 +13,7 @@ #include "base/json/json_reader.h" #include "base/logging.h" #include "base/notreached.h" +#include "base/optional.h" #include "base/values.h" #include "build/build_config.h" #include "chrome/updater/constants.h" @@ -116,13 +117,13 @@ int ExternalConstantsOverrider::ServerKeepAliveSeconds() const { std::unique_ptr ExternalConstantsOverrider::FromDefaultJSONFile( std::unique_ptr next_provider) { - base::FilePath data_dir_path; - if (!GetBaseDirectory(&data_dir_path)) { + base::Optional data_dir_path = GetBaseDirectory(); + if (!data_dir_path) { LOG(ERROR) << "Cannot find app data path."; return nullptr; } const base::FilePath override_file_path = - data_dir_path.AppendASCII(kDevOverrideFileName); + data_dir_path->AppendASCII(kDevOverrideFileName); JSONFileValueDeserializer parser(override_file_path, base::JSON_ALLOW_TRAILING_COMMAS); diff --git a/chrome/updater/installer.cc b/chrome/updater/installer.cc index 7809cb19a6a8cf..0c721dd96ecd99 100644 --- a/chrome/updater/installer.cc +++ b/chrome/updater/installer.cc @@ -11,6 +11,7 @@ #include "base/files/file_enumerator.h" #include "base/files/file_util.h" #include "base/logging.h" +#include "base/optional.h" #include "base/task/post_task.h" #include "base/task/thread_pool.h" #include "base/threading/scoped_blocking_call.h" @@ -35,13 +36,12 @@ static constexpr base::TaskTraits kTaskTraitsBlockWithSyncPrimitives = { // Returns the full path to the installation directory for the application // identified by the |app_id|. -base::FilePath GetAppInstallDir(const std::string& app_id) { - base::FilePath app_install_dir; - if (GetBaseDirectory(&app_install_dir)) { - app_install_dir = app_install_dir.AppendASCII(kAppsDir); - app_install_dir = app_install_dir.AppendASCII(app_id); - } - return app_install_dir; +base::Optional GetAppInstallDir(const std::string& app_id) { + base::Optional app_install_dir = GetBaseDirectory(); + if (!app_install_dir) + return base::nullopt; + + return app_install_dir->AppendASCII(kAppsDir).AppendASCII(app_id); } } // namespace @@ -96,12 +96,13 @@ void Installer::DeleteOlderInstallPaths() { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::WILL_BLOCK); - const base::FilePath app_install_dir = GetAppInstallDir(app_id_); - if (app_install_dir.empty() || !base::PathExists(app_install_dir)) { + const base::Optional app_install_dir = + GetAppInstallDir(app_id_); + if (!app_install_dir || !base::PathExists(*app_install_dir)) { return; } - base::FileEnumerator file_enumerator(app_install_dir, false, + base::FileEnumerator file_enumerator(*app_install_dir, false, base::FileEnumerator::DIRECTORIES); for (auto path = file_enumerator.Next(); !path.value().empty(); path = file_enumerator.Next()) { @@ -138,14 +139,15 @@ Installer::Result Installer::InstallHelper( if (pv_.CompareTo(manifest_version) > 0) return Result(update_client::InstallError::VERSION_NOT_UPGRADED); - const base::FilePath app_install_dir = GetAppInstallDir(app_id_); - if (app_install_dir.empty()) + const base::Optional app_install_dir = + GetAppInstallDir(app_id_); + if (!app_install_dir) return Result(update_client::InstallError::NO_DIR_COMPONENT_USER); - if (!base::CreateDirectory(app_install_dir)) + if (!base::CreateDirectory(*app_install_dir)) return Result(kErrorCreateAppInstallDirectory); const auto versioned_install_dir = - app_install_dir.AppendASCII(manifest_version.GetString()); + app_install_dir->AppendASCII(manifest_version.GetString()); if (base::PathExists(versioned_install_dir)) { if (!base::DeletePathRecursively(versioned_install_dir)) return Result(update_client::InstallError::CLEAN_INSTALL_DIR_FAILED); @@ -226,10 +228,10 @@ bool Installer::GetInstalledFile(const std::string& file, return false; // No component has been installed yet. const auto install_dir = GetCurrentInstallDir(); - if (install_dir.empty()) + if (!install_dir) return false; - *installed_file = install_dir.AppendASCII(file); + *installed_file = install_dir->AppendASCII(file); return true; } @@ -237,10 +239,13 @@ bool Installer::Uninstall() { return false; } -base::FilePath Installer::GetCurrentInstallDir() const { +base::Optional Installer::GetCurrentInstallDir() const { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::WILL_BLOCK); - return GetAppInstallDir(app_id_).AppendASCII(pv_.GetString()); + base::Optional path = GetAppInstallDir(app_id_); + if (!path) + return base::nullopt; + return path->AppendASCII(pv_.GetString()); } #if defined(OS_LINUX) || defined(OS_CHROMEOS) diff --git a/chrome/updater/installer.h b/chrome/updater/installer.h index d42b324b043e50..ee8cb7ed7a57c8 100644 --- a/chrome/updater/installer.h +++ b/chrome/updater/installer.h @@ -11,6 +11,7 @@ #include "base/callback_forward.h" #include "base/files/file_path.h" #include "base/memory/scoped_refptr.h" +#include "base/optional.h" #include "base/sequence_checker.h" #include "base/values.h" #include "base/version.h" @@ -94,7 +95,7 @@ class Installer final : public update_client::CrxInstaller { void DeleteOlderInstallPaths(); // Returns an install directory matching the |pv_| version. - base::FilePath GetCurrentInstallDir() const; + base::Optional GetCurrentInstallDir() const; SEQUENCE_CHECKER(sequence_checker_); diff --git a/chrome/updater/launchd_util.cc b/chrome/updater/launchd_util.cc index 75a9745be2b7dd..27225e6bdf3c38 100644 --- a/chrome/updater/launchd_util.cc +++ b/chrome/updater/launchd_util.cc @@ -8,6 +8,7 @@ #include "base/callback.h" #include "base/command_line.h" #include "base/files/file_path.h" +#include "base/logging.h" #include "base/memory/ref_counted.h" #include "base/process/launch.h" #include "base/process/process.h" @@ -15,12 +16,16 @@ #include "base/task/thread_pool.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/time/time.h" +#import "chrome/updater/mac/mac_util.h" +#include "chrome/updater/updater_scope.h" +#import "chrome/updater/util.h" namespace updater { namespace { -void PollLaunchctlListImpl(const std::string& service, +void PollLaunchctlListImpl(UpdaterScope scope, + const std::string& service, LaunchctlPresence expectation, base::TimeTicks deadline, base::OnceCallback callback) { @@ -31,6 +36,9 @@ void PollLaunchctlListImpl(const std::string& service, base::CommandLine command_line(base::FilePath("/bin/launchctl")); command_line.AppendArg("list"); command_line.AppendArg(service); + if (scope == UpdaterScope::kSystem) + command_line = MakeElevated(command_line); + base::Process process = base::LaunchProcess(command_line, {}); if (!process.IsValid()) { VLOG(1) << "Failed to launch launchctl process."; @@ -50,21 +58,22 @@ void PollLaunchctlListImpl(const std::string& service, } base::ThreadPool::PostDelayedTask( FROM_HERE, {base::WithBaseSyncPrimitives()}, - base::BindOnce(&PollLaunchctlListImpl, service, expectation, deadline, - std::move(callback)), + base::BindOnce(&PollLaunchctlListImpl, scope, service, expectation, + deadline, std::move(callback)), base::TimeDelta::FromMilliseconds(500)); } } // namespace -void PollLaunchctlList(const std::string& service, +void PollLaunchctlList(UpdaterScope scope, + const std::string& service, LaunchctlPresence expectation, base::TimeDelta timeout, base::OnceCallback callback) { base::ThreadPool::PostTask( FROM_HERE, {base::WithBaseSyncPrimitives()}, base::BindOnce( - &PollLaunchctlListImpl, service, expectation, + &PollLaunchctlListImpl, scope, service, expectation, base::TimeTicks::Now() + timeout, base::BindOnce( [](scoped_refptr runner, diff --git a/chrome/updater/launchd_util.h b/chrome/updater/launchd_util.h index 4572474c10440d..440fe48d8dc90d 100644 --- a/chrome/updater/launchd_util.h +++ b/chrome/updater/launchd_util.h @@ -8,6 +8,7 @@ #include #include "base/callback_forward.h" +#include "chrome/updater/updater_scope.h" namespace base { class TimeDelta; @@ -25,7 +26,8 @@ enum class LaunchctlPresence { // reached. Calls `callback` with 'true' if the expectation is met, and false // if |timeout| is reached. Must be called in a sequence. The callback is // posted to the same sequence. -void PollLaunchctlList(const std::string& service, +void PollLaunchctlList(UpdaterScope scope, + const std::string& service, LaunchctlPresence expectation, base::TimeDelta timeout, base::OnceCallback callback); diff --git a/chrome/updater/mac/BUILD.gn b/chrome/updater/mac/BUILD.gn index 2efd1885fec0e3..19868a6028432f 100644 --- a/chrome/updater/mac/BUILD.gn +++ b/chrome/updater/mac/BUILD.gn @@ -81,22 +81,6 @@ source_set("xpc_names") { frameworks = [ "Foundation.framework" ] } -source_set("util") { - sources = [ - "util.h", - "util.mm", - ] - - deps = [ - "//base", - "//chrome/updater:base", - "//chrome/updater:branding_header", - "//chrome/updater:version_header", - ] - - frameworks = [ "Foundation.framework" ] -} - source_set("updater_setup_sources") { sources = [ "setup/setup.h", @@ -105,7 +89,6 @@ source_set("updater_setup_sources") { deps = [ ":network_fetcher_sources", - ":util", ":xpc_names", "//base", "//chrome/common/mac:launchd", diff --git a/chrome/updater/mac/mac_util.h b/chrome/updater/mac/mac_util.h new file mode 100644 index 00000000000000..c548c46a25e815 --- /dev/null +++ b/chrome/updater/mac/mac_util.h @@ -0,0 +1,71 @@ +// 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. + +#ifndef CHROME_UPDATER_MAC_MAC_UTIL_H_ +#define CHROME_UPDATER_MAC_MAC_UTIL_H_ + +#include "base/mac/scoped_cftyperef.h" +#include "base/optional.h" +#include "chrome/common/mac/launchd.h" +#include "chrome/updater/updater_scope.h" + +namespace base { +class FilePath; +class Version; +} // namespace base + +namespace updater { + +// For user installations returns: the "~/Library" for the logged in user. +// For system installations returns: "/Library". +base::Optional GetLibraryFolderPath(UpdaterScope scope); + +// For user installations sets `path` to: the "~/Library/Application Support" +// for the logged in user. For system installations sets `path` to: +// "/Library/Application Support". +base::Optional GetApplicationSupportDirectory( + UpdaterScope scope); + +// For user installations: +// ~/Library/Google/GoogleUpdater +// For system installations: +// /Library/Google/GoogleUpdater +base::Optional GetUpdaterFolderPath(UpdaterScope scope); + +// For user installations: +// ~/Library/Google/GoogleUpdater/88.0.4293.0 +// For system installations: +// /Library/Google/GoogleUpdater/88.0.4293.0 +base::Optional GetVersionedUpdaterFolderPathForVersion( + UpdaterScope scope, + const base::Version& version); + +// The same as GetVersionedUpdaterFolderPathForVersion, where the version is +// UPDATER_VERSION_STRING. +base::Optional GetVersionedUpdaterFolderPath( + UpdaterScope scope); + +// For user installations: +// ~/Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS/GoogleUpdater +// For system installations: +// /Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS/GoogleUpdater +base::Optional GetUpdaterExecutablePath(UpdaterScope scope); + +// For user installations: +// ~/Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS +// For system installations: +// /Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS +base::Optional GetExecutableFolderPathForVersion( + UpdaterScope scope, + const base::Version& version); + +// Removes the Launchd job with the given 'name'. +bool RemoveJobFromLaunchd(UpdaterScope scope, + Launchd::Domain domain, + Launchd::Type type, + base::ScopedCFTypeRef name); + +} // namespace updater + +#endif // CHROME_UPDATER_MAC_MAC_UTIL_H_ diff --git a/chrome/updater/mac/mac_util.mm b/chrome/updater/mac/mac_util.mm new file mode 100644 index 00000000000000..d4168f4a89c361 --- /dev/null +++ b/chrome/updater/mac/mac_util.mm @@ -0,0 +1,183 @@ +// Copyright 2021 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. + +#import "chrome/updater/mac/mac_util.h" + +#include +#include + +#include "base/files/file.h" +#include "base/files/file_path.h" +#include "base/logging.h" +#include "base/mac/foundation_util.h" +#include "base/mac/scoped_cftyperef.h" +#include "base/optional.h" +#include "base/process/launch.h" +#include "base/strings/sys_string_conversions.h" +#include "base/threading/scoped_blocking_call.h" +#include "base/version.h" +#include "chrome/common/mac/launchd.h" +#include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" +#include "chrome/updater/updater_version.h" +#include "chrome/updater/util.h" + +namespace updater { +namespace { + +base::FilePath GetUpdateFolderName() { + return base::FilePath(COMPANY_SHORTNAME_STRING) + .AppendASCII(PRODUCT_FULLNAME_STRING); +} + +base::FilePath ExecutableFolderPath() { + return base::FilePath(FILE_PATH_LITERAL(PRODUCT_FULLNAME_STRING ".app")) + .Append(FILE_PATH_LITERAL("Contents")) + .Append(FILE_PATH_LITERAL("MacOS")); +} + +} // namespace + +base::Optional GetLibraryFolderPath(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kUser: + return base::mac::GetUserLibraryPath(); + case UpdaterScope::kSystem: { + base::FilePath local_library_path; + if (!base::mac::GetLocalDirectory(NSLibraryDirectory, + &local_library_path)) { + VLOG(1) << "Could not get local library path"; + return base::nullopt; + } + return local_library_path; + } + } +} + +base::Optional GetApplicationSupportDirectory( + UpdaterScope scope) { + base::FilePath path; + switch (scope) { + case UpdaterScope::kUser: + if (base::mac::GetUserDirectory(NSApplicationSupportDirectory, &path)) + return path; + break; + case UpdaterScope::kSystem: + if (base::mac::GetLocalDirectory(NSApplicationSupportDirectory, &path)) + return path; + break; + } + + VLOG(1) << "Could not get applications support path"; + return base::nullopt; +} + +base::Optional GetUpdaterFolderPath(UpdaterScope scope) { + base::Optional path = GetLibraryFolderPath(scope); + if (!path) + return base::nullopt; + return path->Append(GetUpdateFolderName()); +} + +base::Optional GetVersionedUpdaterFolderPathForVersion( + UpdaterScope scope, + const base::Version& version) { + base::Optional path = GetUpdaterFolderPath(scope); + if (!path) + return base::nullopt; + return path->AppendASCII(version.GetString()); +} + +base::Optional GetVersionedUpdaterFolderPath( + UpdaterScope scope) { + base::Optional path = GetUpdaterFolderPath(scope); + if (!path) + return base::nullopt; + return path->AppendASCII(UPDATER_VERSION_STRING); +} + +base::Optional GetExecutableFolderPathForVersion( + UpdaterScope scope, + const base::Version& version) { + base::Optional path = + GetVersionedUpdaterFolderPathForVersion(scope, version); + if (!path) + return base::nullopt; + return path->Append(ExecutableFolderPath()); +} + +base::Optional GetUpdaterExecutablePath(UpdaterScope scope) { + base::Optional path = GetVersionedUpdaterFolderPath(scope); + if (!path) + return base::nullopt; + return path->Append(ExecutableFolderPath()) + .Append(FILE_PATH_LITERAL(PRODUCT_FULLNAME_STRING)); +} + +bool PathOwnedByUser(const base::FilePath& path) { + struct passwd* result = nullptr; + struct passwd user_info = {}; + char pwbuf[2048] = {}; + const uid_t user_uid = geteuid(); + + const int error = + getpwuid_r(user_uid, &user_info, pwbuf, sizeof(pwbuf), &result); + + if (error) { + VLOG(1) << "Failed to get user info."; + return true; + } + + if (result == nullptr) { + VLOG(1) << "No entry for user."; + return true; + } + + base::stat_wrapper_t stat_info = {}; + if (base::File::Lstat(path.value().c_str(), &stat_info) != 0) { + DPLOG(ERROR) << "Failed to get information on path " << path.value(); + return false; + } + + if (S_ISLNK(stat_info.st_mode)) { + DLOG(ERROR) << "Path " << path.value() << " is a symbolic link."; + return false; + } + + if (stat_info.st_uid != user_uid) { + DLOG(ERROR) << "Path " << path.value() << " is owned by the wrong user."; + return false; + } + + return true; +} + +bool RemoveJobFromLaunchd(UpdaterScope scope, + Launchd::Domain domain, + Launchd::Type type, + base::ScopedCFTypeRef name) { + // This may block while deleting the launchd plist file. + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); + + // If the job doesn't exist return true. + if (!Launchd::GetInstance()->PlistExists(domain, type, name)) + return true; + + if (!Launchd::GetInstance()->DeletePlist(domain, type, name)) + return false; + + base::CommandLine command_line(base::FilePath("/bin/launchctl")); + command_line.AppendArg("remove"); + command_line.AppendArg(base::SysCFStringRefToUTF8(name)); + if (scope == UpdaterScope::kSystem) + command_line = MakeElevated(command_line); + + int exit_code = -1; + std::string output; + base::GetAppOutputWithExitCode(command_line, &output, &exit_code); + return exit_code == 0; +} + +} // namespace updater diff --git a/chrome/updater/mac/setup/setup.h b/chrome/updater/mac/setup/setup.h index e10e7ac6a373b6..218bd8d5a7b79b 100644 --- a/chrome/updater/mac/setup/setup.h +++ b/chrome/updater/mac/setup/setup.h @@ -5,6 +5,8 @@ #ifndef CHROME_UPDATER_MAC_SETUP_SETUP_H_ #define CHROME_UPDATER_MAC_SETUP_SETUP_H_ +#include "chrome/updater/updater_scope.h" + namespace updater { namespace setup_exit_codes { @@ -21,6 +23,9 @@ constexpr int kFailedToDeleteFolder = 11; // Failed to delete the updater's data folder. constexpr int kFailedToDeleteDataFolder = 12; +// Failed to get versioned updater folder path. +constexpr int kFailedToGetVersionedUpdaterFolderPath = 13; + // Failed to remove the active(unversioned) update service job from Launchd. constexpr int kFailedToRemoveActiveUpdateServiceJobFromLaunchd = 20; @@ -66,17 +71,17 @@ constexpr int kFailedAwaitingLaunchdUpdateServiceInternalJob = 44; // Sets up the candidate updater by copying the bundle, creating launchd plists // for administration service and XPC service tasks, and start the corresponding // launchd jobs. -int Setup(); +int Setup(UpdaterScope scope); // Uninstalls this version of the updater. -int UninstallCandidate(); +int UninstallCandidate(UpdaterScope scope); // Sets up this version of the Updater as the active version. -int PromoteCandidate(); +int PromoteCandidate(UpdaterScope scope); // Removes the launchd plists for scheduled tasks and xpc service. Deletes the // updater bundle from its installed location. -int Uninstall(bool is_machine); +int Uninstall(UpdaterScope scope); } // namespace updater diff --git a/chrome/updater/mac/setup/setup.mm b/chrome/updater/mac/setup/setup.mm index 3a866e29ff383f..7bea12fc0c5012 100644 --- a/chrome/updater/mac/setup/setup.mm +++ b/chrome/updater/mac/setup/setup.mm @@ -15,6 +15,7 @@ #include "base/mac/bundle_locations.h" #include "base/mac/foundation_util.h" #include "base/mac/scoped_nsobject.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/process/launch.h" #include "base/process/process.h" @@ -27,9 +28,10 @@ #include "chrome/updater/constants.h" #include "chrome/updater/crash_client.h" #include "chrome/updater/crash_reporter.h" -#import "chrome/updater/mac/util.h" +#import "chrome/updater/mac/mac_util.h" #import "chrome/updater/mac/xpc_service_names.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/util.h" #include "components/crash/core/common/crash_key.h" @@ -55,16 +57,40 @@ .Append(GetUpdaterAppExecutablePath()); } -Launchd::Domain LaunchdDomain() { - return IsSystemInstall() ? Launchd::Domain::Local : Launchd::Domain::User; +Launchd::Domain LaunchdDomain(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return Launchd::Domain::Local; + case UpdaterScope::kUser: + return Launchd::Domain::User; + } } -Launchd::Type ServiceLaunchdType() { - return IsSystemInstall() ? Launchd::Type::Daemon : Launchd::Type::Agent; +Launchd::Type ServiceLaunchdType(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return Launchd::Type::Daemon; + case UpdaterScope::kUser: + return Launchd::Type::Agent; + } +} + +CFStringRef CFSessionType(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return CFSTR("System"); + case UpdaterScope::kUser: + return CFSTR("Aqua"); + } } -Launchd::Type ClientLaunchdType() { - return Launchd::Type::Agent; +NSString* NSStringSessionType(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return @"System"; + case UpdaterScope::kUser: + return @"Aqua"; + } } #pragma mark Setup @@ -95,22 +121,30 @@ bool CopyBundle(const base::FilePath& dest_path) { } base::ScopedCFTypeRef CreateServiceLaunchdPlist( + UpdaterScope scope, const base::FilePath& updater_path) { // See the man page for launchd.plist. + NSMutableArray* program_arguments = + [NSMutableArray array]; + [program_arguments addObjectsFromArray:@[ + base::SysUTF8ToNSString(updater_path.value()), + MakeProgramArgument(kServerSwitch), + MakeProgramArgumentWithValue(kServerServiceSwitch, + kServerUpdateServiceSwitchValue), + MakeProgramArgument(kEnableLoggingSwitch), + MakeProgramArgumentWithValue(kLoggingModuleSwitch, + kLoggingModuleSwitchValue) + + ]]; + if (scope == UpdaterScope::kSystem) + [program_arguments addObject:MakeProgramArgument(kSystemSwitch)]; + NSDictionary* launchd_plist = @{ @LAUNCH_JOBKEY_LABEL : GetUpdateServiceLaunchdLabel(), - @LAUNCH_JOBKEY_PROGRAMARGUMENTS : @[ - base::SysUTF8ToNSString(updater_path.value()), - MakeProgramArgument(kServerSwitch), - MakeProgramArgumentWithValue(kServerServiceSwitch, - kServerUpdateServiceSwitchValue), - MakeProgramArgument(kEnableLoggingSwitch), - MakeProgramArgumentWithValue(kLoggingModuleSwitch, - kLoggingModuleSwitchValue), - ], + @LAUNCH_JOBKEY_PROGRAMARGUMENTS : program_arguments, @LAUNCH_JOBKEY_MACHSERVICES : @{GetUpdateServiceMachName() : @YES}, @LAUNCH_JOBKEY_ABANDONPROCESSGROUP : @NO, - @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : @"Aqua" + @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : NSStringSessionType(scope) }; return base::ScopedCFTypeRef( @@ -119,6 +153,7 @@ bool CopyBundle(const base::FilePath& dest_path) { } base::ScopedCFTypeRef CreateWakeLaunchdPlist( + UpdaterScope scope, const base::FilePath& updater_path) { // See the man page for launchd.plist. NSMutableArray* program_arguments = @@ -127,7 +162,7 @@ bool CopyBundle(const base::FilePath& dest_path) { base::SysUTF8ToNSString(updater_path.value()), MakeProgramArgument(kWakeSwitch), MakeProgramArgument(kEnableLoggingSwitch) ]]; - if (IsSystemInstall()) + if (scope == UpdaterScope::kSystem) [program_arguments addObject:MakeProgramArgument(kSystemSwitch)]; NSDictionary* launchd_plist = @{ @@ -135,7 +170,7 @@ bool CopyBundle(const base::FilePath& dest_path) { @LAUNCH_JOBKEY_PROGRAMARGUMENTS : program_arguments, @LAUNCH_JOBKEY_STARTINTERVAL : @3600, @LAUNCH_JOBKEY_ABANDONPROCESSGROUP : @NO, - @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : @"Aqua" + @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : NSStringSessionType(scope) }; return base::ScopedCFTypeRef( @@ -144,22 +179,29 @@ bool CopyBundle(const base::FilePath& dest_path) { } base::ScopedCFTypeRef CreateUpdateServiceInternalLaunchdPlist( + UpdaterScope scope, const base::FilePath& updater_path) { // See the man page for launchd.plist. + NSMutableArray* program_arguments = + [NSMutableArray array]; + [program_arguments addObjectsFromArray:@[ + base::SysUTF8ToNSString(updater_path.value()), + MakeProgramArgument(kServerSwitch), + MakeProgramArgumentWithValue(kServerServiceSwitch, + kServerUpdateServiceInternalSwitchValue), + MakeProgramArgument(kEnableLoggingSwitch), + MakeProgramArgumentWithValue(kLoggingModuleSwitch, + kLoggingModuleSwitchValue) + ]]; + if (scope == UpdaterScope::kSystem) + [program_arguments addObject:MakeProgramArgument(kSystemSwitch)]; + NSDictionary* launchd_plist = @{ @LAUNCH_JOBKEY_LABEL : GetUpdateServiceInternalLaunchdLabel(), - @LAUNCH_JOBKEY_PROGRAMARGUMENTS : @[ - base::SysUTF8ToNSString(updater_path.value()), - MakeProgramArgument(kServerSwitch), - MakeProgramArgumentWithValue(kServerServiceSwitch, - kServerUpdateServiceInternalSwitchValue), - MakeProgramArgument(kEnableLoggingSwitch), - MakeProgramArgumentWithValue(kLoggingModuleSwitch, - kLoggingModuleSwitchValue), - ], + @LAUNCH_JOBKEY_PROGRAMARGUMENTS : program_arguments, @LAUNCH_JOBKEY_MACHSERVICES : @{GetUpdateServiceInternalMachName() : @YES}, @LAUNCH_JOBKEY_ABANDONPROCESSGROUP : @YES, - @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : @"Aqua" + @LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE : NSStringSessionType(scope) }; return base::ScopedCFTypeRef( @@ -167,194 +209,197 @@ bool CopyBundle(const base::FilePath& dest_path) { base::scoped_policy::RETAIN); } -bool CreateUpdateServiceLaunchdJobPlist(const base::FilePath& updater_path) { +bool CreateUpdateServiceLaunchdJobPlist(UpdaterScope scope, + const base::FilePath& updater_path) { // We're creating directories and writing a file. base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); base::ScopedCFTypeRef plist( - CreateServiceLaunchdPlist(updater_path)); + CreateServiceLaunchdPlist(scope, updater_path)); return Launchd::GetInstance()->WritePlistToFile( - LaunchdDomain(), ServiceLaunchdType(), CopyUpdateServiceLaunchdName(), - plist); + LaunchdDomain(scope), ServiceLaunchdType(scope), + CopyUpdateServiceLaunchdName(), plist); } -bool CreateWakeLaunchdJobPlist(const base::FilePath& updater_path) { +bool CreateWakeLaunchdJobPlist(UpdaterScope scope, + const base::FilePath& updater_path) { // We're creating directories and writing a file. base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); base::ScopedCFTypeRef plist( - CreateWakeLaunchdPlist(updater_path)); - return Launchd::GetInstance()->WritePlistToFile( - LaunchdDomain(), ServiceLaunchdType(), CopyWakeLaunchdName(), plist); + CreateWakeLaunchdPlist(scope, updater_path)); + return Launchd::GetInstance()->WritePlistToFile(LaunchdDomain(scope), + ServiceLaunchdType(scope), + CopyWakeLaunchdName(), plist); } bool CreateUpdateServiceInternalLaunchdJobPlist( + UpdaterScope scope, const base::FilePath& updater_path) { // We're creating directories and writing a file. base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); base::ScopedCFTypeRef plist( - CreateUpdateServiceInternalLaunchdPlist(updater_path)); + CreateUpdateServiceInternalLaunchdPlist(scope, updater_path)); return Launchd::GetInstance()->WritePlistToFile( - LaunchdDomain(), ServiceLaunchdType(), + LaunchdDomain(scope), ServiceLaunchdType(scope), CopyUpdateServiceInternalLaunchdName(), plist); } bool StartUpdateServiceVersionedLaunchdJob( + UpdaterScope scope, const base::ScopedCFTypeRef name) { - return Launchd::GetInstance()->RestartJob( - LaunchdDomain(), ServiceLaunchdType(), name, CFSTR("Aqua")); + return Launchd::GetInstance()->RestartJob(LaunchdDomain(scope), + ServiceLaunchdType(scope), name, + CFSessionType(scope)); } -bool StartUpdateWakeVersionedLaunchdJob() { +bool StartUpdateWakeVersionedLaunchdJob(UpdaterScope scope) { return Launchd::GetInstance()->RestartJob( - LaunchdDomain(), ServiceLaunchdType(), CopyWakeLaunchdName(), - CFSTR("Aqua")); + LaunchdDomain(scope), ServiceLaunchdType(scope), CopyWakeLaunchdName(), + CFSessionType(scope)); } -bool StartUpdateServiceInternalVersionedLaunchdJob() { +bool StartUpdateServiceInternalVersionedLaunchdJob(UpdaterScope scope) { return Launchd::GetInstance()->RestartJob( - LaunchdDomain(), ServiceLaunchdType(), - CopyUpdateServiceInternalLaunchdName(), CFSTR("Aqua")); + LaunchdDomain(scope), ServiceLaunchdType(scope), + CopyUpdateServiceInternalLaunchdName(), CFSessionType(scope)); } -bool StartLaunchdServiceJob() { - return StartUpdateServiceVersionedLaunchdJob(CopyUpdateServiceLaunchdName()); +bool StartLaunchdServiceJob(UpdaterScope scope) { + return StartUpdateServiceVersionedLaunchdJob(scope, + CopyUpdateServiceLaunchdName()); } -bool RemoveJobFromLaunchd(Launchd::Domain domain, - Launchd::Type type, - base::ScopedCFTypeRef name) { - // This may block while deleting the launchd plist file. - base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, - base::BlockingType::MAY_BLOCK); - - // If the job doesn't exist return true. - if (!Launchd::GetInstance()->PlistExists(domain, type, name)) - return true; - - if (!Launchd::GetInstance()->DeletePlist(domain, type, name)) - return false; - - return Launchd::GetInstance()->RemoveJob(base::SysCFStringRefToUTF8(name)); -} - -bool RemoveClientJobFromLaunchd(base::ScopedCFTypeRef name) { - return RemoveJobFromLaunchd(LaunchdDomain(), ClientLaunchdType(), name); -} - -bool RemoveServiceJobFromLaunchd(base::ScopedCFTypeRef name) { - return RemoveJobFromLaunchd(LaunchdDomain(), ServiceLaunchdType(), name); +bool RemoveServiceJobFromLaunchd(UpdaterScope scope, + base::ScopedCFTypeRef name) { + return RemoveJobFromLaunchd(scope, LaunchdDomain(scope), + ServiceLaunchdType(scope), name); } bool RemoveUpdateServiceJobFromLaunchd( + UpdaterScope scope, base::ScopedCFTypeRef name) { - return RemoveServiceJobFromLaunchd(name); + return RemoveServiceJobFromLaunchd(scope, name); } -bool RemoveUpdateServiceJobFromLaunchd() { - return RemoveUpdateServiceJobFromLaunchd(CopyUpdateServiceLaunchdName()); +bool RemoveUpdateServiceJobFromLaunchd(UpdaterScope scope) { + return RemoveUpdateServiceJobFromLaunchd(scope, + CopyUpdateServiceLaunchdName()); } -bool RemoveUpdateWakeJobFromLaunchd() { - return RemoveClientJobFromLaunchd(CopyWakeLaunchdName()); +bool RemoveUpdateWakeJobFromLaunchd(UpdaterScope scope) { + return RemoveServiceJobFromLaunchd(scope, CopyWakeLaunchdName()); } -bool RemoveUpdateServiceInternalJobFromLaunchd() { - return RemoveServiceJobFromLaunchd(CopyUpdateServiceInternalLaunchdName()); +bool RemoveUpdateServiceInternalJobFromLaunchd(UpdaterScope scope) { + return RemoveServiceJobFromLaunchd(scope, + CopyUpdateServiceInternalLaunchdName()); } -bool DeleteFolder(const base::FilePath& installed_path) { - if (!base::DeletePathRecursively(installed_path)) { +bool DeleteFolder(const base::Optional& installed_path) { + if (!installed_path) + return false; + if (!base::DeletePathRecursively(*installed_path)) { LOG(ERROR) << "Deleting " << installed_path << " failed"; return false; } return true; } -bool DeleteInstallFolder() { - return DeleteFolder(GetUpdaterFolderPath()); +bool DeleteInstallFolder(UpdaterScope scope) { + return DeleteFolder(GetUpdaterFolderPath(scope)); } -bool DeleteCandidateInstallFolder() { - return DeleteFolder(GetVersionedUpdaterFolderPath()); +bool DeleteCandidateInstallFolder(UpdaterScope scope) { + return DeleteFolder(GetVersionedUpdaterFolderPath(scope)); } bool DeleteDataFolder() { - base::FilePath data_path; - if (!GetBaseDirectory(&data_path)) - return false; - return DeleteFolder(data_path); + return DeleteFolder(GetBaseDirectory()); } } // namespace -int Setup() { - const base::FilePath dest_path = GetVersionedUpdaterFolderPath(); +int Setup(UpdaterScope scope) { + const base::Optional dest_path = + GetVersionedUpdaterFolderPath(scope); - if (!CopyBundle(dest_path)) + if (!dest_path) + return setup_exit_codes::kFailedToGetVersionedUpdaterFolderPath; + if (!CopyBundle(*dest_path)) return setup_exit_codes::kFailedToCopyBundle; const base::FilePath updater_executable_path = - dest_path.Append(GetUpdaterAppName()) + dest_path->Append(GetUpdaterAppName()) .Append(GetUpdaterAppExecutablePath()); - if (!CreateWakeLaunchdJobPlist(updater_executable_path)) + if (!CreateWakeLaunchdJobPlist(scope, updater_executable_path)) return setup_exit_codes::kFailedToCreateWakeLaunchdJobPlist; - if (!CreateUpdateServiceInternalLaunchdJobPlist(updater_executable_path)) + if (!CreateUpdateServiceInternalLaunchdJobPlist(scope, + updater_executable_path)) return setup_exit_codes:: kFailedToCreateUpdateServiceInternalLaunchdJobPlist; - if (!StartUpdateServiceInternalVersionedLaunchdJob()) + if (!StartUpdateServiceInternalVersionedLaunchdJob(scope)) return setup_exit_codes::kFailedToStartLaunchdUpdateServiceInternalJob; - if (!StartUpdateWakeVersionedLaunchdJob()) + if (!StartUpdateWakeVersionedLaunchdJob(scope)) return setup_exit_codes::kFailedToStartLaunchdWakeJob; return setup_exit_codes::kSuccess; } -int PromoteCandidate() { - const base::FilePath dest_path = GetVersionedUpdaterFolderPath(); +int PromoteCandidate(UpdaterScope scope) { + const base::Optional dest_path = + GetVersionedUpdaterFolderPath(scope); + if (!dest_path) + return setup_exit_codes::kFailedToGetVersionedUpdaterFolderPath; const base::FilePath updater_executable_path = - dest_path.Append(GetUpdaterAppName()) + dest_path->Append(GetUpdaterAppName()) .Append(GetUpdaterAppExecutablePath()); - if (!CreateUpdateServiceLaunchdJobPlist(updater_executable_path)) + if (!CreateUpdateServiceLaunchdJobPlist(scope, updater_executable_path)) return setup_exit_codes::kFailedToCreateUpdateServiceLaunchdJobPlist; - if (!StartLaunchdServiceJob()) + if (!StartLaunchdServiceJob(scope)) return setup_exit_codes::kFailedToStartLaunchdActiveServiceJob; return setup_exit_codes::kSuccess; } #pragma mark Uninstall -int UninstallCandidate() { - if (!DeleteCandidateInstallFolder()) +int UninstallCandidate(UpdaterScope scope) { + if (!DeleteCandidateInstallFolder(scope)) return setup_exit_codes::kFailedToDeleteFolder; - if (!RemoveUpdateWakeJobFromLaunchd()) + if (!RemoveUpdateWakeJobFromLaunchd(scope)) return setup_exit_codes::kFailedToRemoveWakeJobFromLaunchd; // Removing the Update Internal job has to be the last step because launchd is // likely to terminate the current process. Clients should expect the // connection to invalidate (possibly with an interruption beforehand) as a // result of service uninstallation. - if (!RemoveUpdateServiceInternalJobFromLaunchd()) + if (!RemoveUpdateServiceInternalJobFromLaunchd(scope)) return setup_exit_codes::kFailedToRemoveUpdateServiceInternalJobFromLaunchd; return setup_exit_codes::kSuccess; } -void UninstallOtherVersions() { - base::FileEnumerator file_enumerator(GetUpdaterFolderPath(), true, +void UninstallOtherVersions(UpdaterScope scope) { + const base::Optional path = + GetVersionedUpdaterFolderPath(scope); + if (!path) { + LOG(ERROR) << "Failed to get updater folder path."; + return; + } + base::FileEnumerator file_enumerator(*path, true, base::FileEnumerator::DIRECTORIES); for (base::FilePath version_folder_path = file_enumerator.Next(); !version_folder_path.empty() && - version_folder_path != GetVersionedUpdaterFolderPath(); + version_folder_path != GetVersionedUpdaterFolderPath(scope); version_folder_path = file_enumerator.Next()) { const base::FilePath version_executable_path = GetUpdaterExecutablePath(version_folder_path); @@ -362,6 +407,8 @@ void UninstallOtherVersions() { if (base::PathExists(version_executable_path)) { base::CommandLine command_line(version_executable_path); command_line.AppendSwitch(kUninstallSelfSwitch); + if (scope == UpdaterScope::kSystem) + command_line.AppendSwitch(kSystemSwitch); command_line.AppendSwitch("--enable-logging"); command_line.AppendSwitchASCII("--vmodule", "*/chrome/updater/*=2"); @@ -375,23 +422,22 @@ void UninstallOtherVersions() { } } -int Uninstall(bool is_machine) { - ALLOW_UNUSED_LOCAL(is_machine); +int Uninstall(UpdaterScope scope) { VLOG(1) << base::CommandLine::ForCurrentProcess()->GetCommandLineString() << " : " << __func__; - const int exit = UninstallCandidate(); + const int exit = UninstallCandidate(scope); if (exit != setup_exit_codes::kSuccess) return exit; - if (!RemoveUpdateServiceJobFromLaunchd()) + if (!RemoveUpdateServiceJobFromLaunchd(scope)) return setup_exit_codes::kFailedToRemoveActiveUpdateServiceJobFromLaunchd; - UninstallOtherVersions(); + UninstallOtherVersions(scope); if (!DeleteDataFolder()) return setup_exit_codes::kFailedToDeleteDataFolder; - if (!DeleteInstallFolder()) + if (!DeleteInstallFolder(scope)) return setup_exit_codes::kFailedToDeleteFolder; return setup_exit_codes::kSuccess; diff --git a/chrome/updater/mac/update_service_internal_proxy.h b/chrome/updater/mac/update_service_internal_proxy.h index 64f83deb8c75e3..b802591de8834c 100644 --- a/chrome/updater/mac/update_service_internal_proxy.h +++ b/chrome/updater/mac/update_service_internal_proxy.h @@ -11,8 +11,8 @@ #include "base/mac/scoped_nsobject.h" #include "base/memory/scoped_refptr.h" #include "base/sequence_checker.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/update_service_internal.h" +#include "chrome/updater/updater_scope.h" @class CRUUpdateServiceInternalProxyImpl; @@ -25,7 +25,7 @@ namespace updater { // All functions and callbacks must be called on the same sequence. class UpdateServiceInternalProxy : public UpdateServiceInternal { public: - explicit UpdateServiceInternalProxy(ServiceScope scope); + explicit UpdateServiceInternalProxy(UpdaterScope scope); // Overrides for UpdateServiceInternal. void Run(base::OnceClosure callback) override; diff --git a/chrome/updater/mac/update_service_internal_proxy.mm b/chrome/updater/mac/update_service_internal_proxy.mm index 4995d9f682721d..547097598e7ed0 100644 --- a/chrome/updater/mac/update_service_internal_proxy.mm +++ b/chrome/updater/mac/update_service_internal_proxy.mm @@ -13,7 +13,7 @@ #include "base/threading/sequenced_task_runner_handle.h" #import "chrome/updater/app/server/mac/service_protocol.h" #import "chrome/updater/mac/xpc_service_names.h" -#include "chrome/updater/service_scope.h" +#include "chrome/updater/updater_scope.h" // Interface to communicate with the XPC Update Service Internal. @interface CRUUpdateServiceInternalProxyImpl @@ -90,13 +90,13 @@ - (void)performInitializeUpdateServiceWithReply:(void (^)(void))reply { namespace updater { -UpdateServiceInternalProxy::UpdateServiceInternalProxy(ServiceScope scope) +UpdateServiceInternalProxy::UpdateServiceInternalProxy(UpdaterScope scope) : callback_runner_(base::SequencedTaskRunnerHandle::Get()) { switch (scope) { - case ServiceScope::kSystem: + case UpdaterScope::kSystem: client_.reset([[CRUUpdateServiceInternalProxyImpl alloc] initPrivileged]); break; - case ServiceScope::kUser: + case UpdaterScope::kUser: client_.reset([[CRUUpdateServiceInternalProxyImpl alloc] init]); break; } diff --git a/chrome/updater/mac/update_service_proxy.h b/chrome/updater/mac/update_service_proxy.h index 6588737cbff624..c5f530b21c8496 100644 --- a/chrome/updater/mac/update_service_proxy.h +++ b/chrome/updater/mac/update_service_proxy.h @@ -14,8 +14,8 @@ #include "base/memory/scoped_refptr.h" #include "base/sequence_checker.h" #include "base/sequenced_task_runner.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/update_service.h" +#include "chrome/updater/updater_scope.h" @class CRUUpdateServiceProxyImpl; @@ -33,7 +33,7 @@ namespace updater { // All functions and callbacks must be called on the same sequence. class UpdateServiceProxy : public UpdateService { public: - explicit UpdateServiceProxy(ServiceScope scope); + explicit UpdateServiceProxy(UpdaterScope scope); // Overrides for UpdateService. void GetVersion( diff --git a/chrome/updater/mac/update_service_proxy.mm b/chrome/updater/mac/update_service_proxy.mm index e87270245b1858..90b15623e5227c 100644 --- a/chrome/updater/mac/update_service_proxy.mm +++ b/chrome/updater/mac/update_service_proxy.mm @@ -21,8 +21,8 @@ #import "chrome/updater/app/server/mac/service_protocol.h" #import "chrome/updater/app/server/mac/update_service_wrappers.h" #import "chrome/updater/mac/xpc_service_names.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/update_service.h" +#include "chrome/updater/updater_scope.h" #include "components/update_client/update_client_errors.h" using base::SysUTF8ToNSString; @@ -143,12 +143,12 @@ - (void)checkForUpdateWithAppID:(NSString* _Nonnull)appID namespace updater { -UpdateServiceProxy::UpdateServiceProxy(ServiceScope scope) { +UpdateServiceProxy::UpdateServiceProxy(UpdaterScope scope) { switch (scope) { - case ServiceScope::kSystem: + case UpdaterScope::kSystem: client_.reset([[CRUUpdateServiceProxyImpl alloc] initPrivileged]); break; - case ServiceScope::kUser: + case UpdaterScope::kUser: client_.reset([[CRUUpdateServiceProxyImpl alloc] init]); break; } diff --git a/chrome/updater/mac/update_service_proxy_test.mm b/chrome/updater/mac/update_service_proxy_test.mm index d5aa21f90635ca..588f93ed981aff 100644 --- a/chrome/updater/mac/update_service_proxy_test.mm +++ b/chrome/updater/mac/update_service_proxy_test.mm @@ -29,8 +29,8 @@ #import "chrome/updater/app/server/mac/update_service_wrappers.h" #include "chrome/updater/mac/scoped_xpc_service_mock.h" #import "chrome/updater/mac/xpc_service_names.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/unittest_util.h" +#include "chrome/updater/updater_scope.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest_mac.h" #import "third_party/ocmock/OCMock/OCMock.h" @@ -294,7 +294,7 @@ void StartSimulating( base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindLambdaForTesting([this]() { service_ = - base::MakeRefCounted(ServiceScope::kUser); + base::MakeRefCounted(UpdaterScope::kUser); })); } diff --git a/chrome/updater/mac/util.h b/chrome/updater/mac/util.h deleted file mode 100644 index 15ac657bc99617..00000000000000 --- a/chrome/updater/mac/util.h +++ /dev/null @@ -1,52 +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. - -#ifndef CHROME_UPDATER_MAC_UTIL_H_ -#define CHROME_UPDATER_MAC_UTIL_H_ - -namespace base { -class FilePath; -class Version; -} // namespace base - -namespace updater { - -bool IsSystemInstall(); - -// For user installations returns: the "~/Library" for the logged in user. -// For system installations returns: "/Library". -base::FilePath GetLibraryFolderPath(); - -// For user installations: -// ~/Library/Google/GoogleUpdater -// For system installations: -// /Library/Google/GoogleUpdater -base::FilePath GetUpdaterFolderPath(); - -// For user installations: -// ~/Library/Google/GoogleUpdater/88.0.4293.0 -// For system installations: -// /Library/Google/GoogleUpdater/88.0.4293.0 -base::FilePath GetVersionedUpdaterFolderPathForVersion( - const base::Version& version); - -// The same as GetVersionedUpdaterFolderPathForVersion, where the version is -// UPDATER_VERSION_STRING. -base::FilePath GetVersionedUpdaterFolderPath(); - -// For user installations: -// ~/Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS/GoogleUpdater -// For system installations: -// /Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS/GoogleUpdater -base::FilePath GetUpdaterExecutablePath(); - -// For user installations: -// ~/Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS -// For system installations: -// /Library/Google/GoogleUpdater/88.0.4293.0/GoogleUpdater.app/Contents/MacOS -base::FilePath GetExecutableFolderPathForVersion(const base::Version& version); - -} // namespace updater - -#endif // CHROME_UPDATER_MAC_UTIL_H_ diff --git a/chrome/updater/mac/util.mm b/chrome/updater/mac/util.mm deleted file mode 100644 index 598c49d8edd17b..00000000000000 --- a/chrome/updater/mac/util.mm +++ /dev/null @@ -1,112 +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. - -#import "chrome/updater/mac/util.h" - -#include -#include - -#include "base/files/file.h" -#include "base/files/file_path.h" -#include "base/logging.h" -#include "base/mac/foundation_util.h" -#include "base/version.h" -#include "chrome/updater/updater_branding.h" -#include "chrome/updater/updater_version.h" -#include "chrome/updater/util.h" - -namespace updater { -namespace { - -base::FilePath GetUpdateFolderName() { - return base::FilePath(COMPANY_SHORTNAME_STRING) - .AppendASCII(PRODUCT_FULLNAME_STRING); -} - -base::FilePath ExecutableFolderPath() { - return base::FilePath(FILE_PATH_LITERAL(PRODUCT_FULLNAME_STRING ".app")) - .Append(FILE_PATH_LITERAL("Contents")) - .Append(FILE_PATH_LITERAL("MacOS")); -} - -} // namespace - -bool IsSystemInstall() { - return geteuid() == 0; -} - -base::FilePath GetLibraryFolderPath() { - if (!IsSystemInstall()) - return base::mac::GetUserLibraryPath(); - - base::FilePath local_library_path; - if (base::mac::GetLocalDirectory(NSLibraryDirectory, &local_library_path)) { - return local_library_path; - } - VLOG(1) << "Could not get local library path"; - return base::FilePath(); -} - -base::FilePath GetUpdaterFolderPath() { - return GetLibraryFolderPath().Append(GetUpdateFolderName()); -} - -base::FilePath GetVersionedUpdaterFolderPathForVersion( - const base::Version& version) { - return GetUpdaterFolderPath().AppendASCII(version.GetString()); -} - -base::FilePath GetVersionedUpdaterFolderPath() { - return GetUpdaterFolderPath().AppendASCII(UPDATER_VERSION_STRING); -} - -base::FilePath GetExecutableFolderPathForVersion(const base::Version& version) { - return GetVersionedUpdaterFolderPathForVersion(version).Append( - ExecutableFolderPath()); -} - -base::FilePath GetUpdaterExecutablePath() { - return GetVersionedUpdaterFolderPath() - .Append(ExecutableFolderPath()) - .Append(FILE_PATH_LITERAL(PRODUCT_FULLNAME_STRING)); -} - -bool PathOwnedByUser(const base::FilePath& path) { - struct passwd* result = nullptr; - struct passwd user_info = {}; - char pwbuf[2048] = {}; - uid_t user_uid = geteuid(); - - int error = getpwuid_r(user_uid, &user_info, pwbuf, sizeof(pwbuf), &result); - - if (error) { - VLOG(1) << "Failed to get user info."; - return true; - } - - if (result == nullptr) { - VLOG(1) << "No entry for user."; - return true; - } - - base::stat_wrapper_t stat_info = {}; - if (base::File::Lstat(path.value().c_str(), &stat_info) != 0) { - DPLOG(ERROR) << "Failed to get information on path " << path.value(); - return false; - } - - if (S_ISLNK(stat_info.st_mode)) { - DLOG(ERROR) << "Path " << path.value() << " is a symbolic link."; - return false; - } - - if (stat_info.st_uid != user_uid) { - DLOG(ERROR) << "Path " << path.value() << " is owned by the wrong user."; - return false; - } - - return true; -} - -} // namespace updater diff --git a/chrome/updater/prefs.cc b/chrome/updater/prefs.cc index 8514fcbd4c34f3..dfb8af4449923d 100644 --- a/chrome/updater/prefs.cc +++ b/chrome/updater/prefs.cc @@ -8,7 +8,9 @@ #include "base/bind.h" #include "base/files/file_path.h" +#include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/run_loop.h" #include "chrome/updater/prefs_impl.h" #include "chrome/updater/util.h" @@ -70,13 +72,14 @@ std::unique_ptr CreateGlobalPrefs() { if (!lock) return nullptr; - base::FilePath global_prefs_dir; - if (!GetBaseDirectory(&global_prefs_dir)) + base::Optional global_prefs_dir = GetBaseDirectory(); + if (!global_prefs_dir) return nullptr; + VLOG(1) << "global_prefs_dir: " << global_prefs_dir; PrefServiceFactory pref_service_factory; pref_service_factory.set_user_prefs(base::MakeRefCounted( - global_prefs_dir.Append(FILE_PATH_LITERAL("prefs.json")))); + global_prefs_dir->Append(FILE_PATH_LITERAL("prefs.json")))); auto pref_registry = base::MakeRefCounted(); update_client::RegisterPrefs(pref_registry.get()); @@ -89,13 +92,13 @@ std::unique_ptr CreateGlobalPrefs() { } std::unique_ptr CreateLocalPrefs() { - base::FilePath local_prefs_dir; - if (!GetVersionedDirectory(&local_prefs_dir)) + base::Optional local_prefs_dir = GetVersionedDirectory(); + if (!local_prefs_dir) return nullptr; PrefServiceFactory pref_service_factory; pref_service_factory.set_user_prefs(base::MakeRefCounted( - local_prefs_dir.Append(FILE_PATH_LITERAL("prefs.json")))); + local_prefs_dir->Append(FILE_PATH_LITERAL("prefs.json")))); auto pref_registry = base::MakeRefCounted(); update_client::RegisterPrefs(pref_registry.get()); diff --git a/chrome/updater/prefs_win.cc b/chrome/updater/prefs_win.cc index c664c8d53cf52d..dd5548001b475d 100644 --- a/chrome/updater/prefs_win.cc +++ b/chrome/updater/prefs_win.cc @@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/time/time.h" #include "base/win/scoped_handle.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/constants.h" #include "chrome/updater/win/util.h" @@ -23,7 +24,7 @@ class ScopedPrefsLockImpl { ScopedPrefsLockImpl& operator=(const ScopedPrefsLockImpl&) = delete; ~ScopedPrefsLockImpl(); - bool Initialize(bool is_machine, base::TimeDelta timeout); + bool Initialize(UpdaterScope scope, base::TimeDelta timeout); private: base::win::ScopedHandle mutex_; @@ -38,18 +39,20 @@ std::unique_ptr AcquireGlobalPrefsLock( base::TimeDelta timeout) { auto lock = std::make_unique(); - // TODO(crbug.com/1096654): need to pass is_machine instead of 'false' here. + // TODO(crbug.com/1096654): need to pass 'scope' instead of + // 'UpdaterScope::kUser' here. DVLOG(2) << "Trying to acquire the lock."; - if (!lock->Initialize(false, timeout)) + if (!lock->Initialize(UpdaterScope::kUser, timeout)) return nullptr; DVLOG(2) << "Lock acquired."; return std::make_unique(std::move(lock)); } -bool ScopedPrefsLockImpl::Initialize(bool is_machine, base::TimeDelta timeout) { +bool ScopedPrefsLockImpl::Initialize(UpdaterScope scope, + base::TimeDelta timeout) { NamedObjectAttributes lock_attr; - GetNamedObjectAttributes(kPrefsAccessMutex, is_machine, &lock_attr); + GetNamedObjectAttributes(kPrefsAccessMutex, scope, &lock_attr); mutex_.Set(::CreateMutex(&lock_attr.sa, false, lock_attr.name.c_str())); if (!mutex_.IsValid()) return false; diff --git a/chrome/updater/service_factory_mac.mm b/chrome/updater/service_factory_mac.mm index 9d93207a5345a7..36ce5adf5e663c 100644 --- a/chrome/updater/service_factory_mac.mm +++ b/chrome/updater/service_factory_mac.mm @@ -5,7 +5,7 @@ #include "base/memory/ref_counted.h" #include "chrome/updater/mac/update_service_internal_proxy.h" #include "chrome/updater/mac/update_service_proxy.h" -#include "chrome/updater/service_scope.h" +#include "chrome/updater/updater_scope.h" namespace updater { diff --git a/chrome/updater/service_factory_win.cc b/chrome/updater/service_factory_win.cc index 5e214f1f091b27..f77d7a3d534dab 100644 --- a/chrome/updater/service_factory_win.cc +++ b/chrome/updater/service_factory_win.cc @@ -5,7 +5,7 @@ #include "base/logging.h" #include "base/memory/ref_counted.h" #include "base/no_destructor.h" -#include "chrome/updater/service_scope.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/update_service_internal_proxy.h" #include "chrome/updater/win/update_service_proxy.h" #include "chrome/updater/win/wrl_module.h" diff --git a/chrome/updater/service_scope.h b/chrome/updater/service_scope.h deleted file mode 100644 index d3cda98932546f..00000000000000 --- a/chrome/updater/service_scope.h +++ /dev/null @@ -1,30 +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. - -#ifndef CHROME_UPDATER_SERVICE_SCOPE_H_ -#define CHROME_UPDATER_SERVICE_SCOPE_H_ - -#include "base/command_line.h" -#include "chrome/updater/constants.h" - -namespace updater { - -// Scope of the service invocation. -enum class ServiceScope { - // The updater is running in the logged in user's scope. - kUser = 1, - - // The updater is running in the system's scope. - kSystem = 2, -}; - -inline ServiceScope GetProcessScope() { - return base::CommandLine::ForCurrentProcess()->HasSwitch(kSystemSwitch) - ? ServiceScope::kSystem - : ServiceScope::kUser; -} - -} // namespace updater - -#endif // CHROME_UPDATER_SERVICE_SCOPE_H_ diff --git a/chrome/updater/setup.h b/chrome/updater/setup.h index 365680702eaa81..d005a943da245f 100644 --- a/chrome/updater/setup.h +++ b/chrome/updater/setup.h @@ -7,12 +7,13 @@ #include "base/callback_forward.h" #include "base/memory/ref_counted.h" +#include "chrome/updater/updater_scope.h" namespace updater { // Installs the candidate, then posts |callback| to the main sequence. Must // be called on the main sequence. -void InstallCandidate(bool is_machine, +void InstallCandidate(UpdaterScope scope, base::OnceCallback callback); } // namespace updater diff --git a/chrome/updater/setup_mac.mm b/chrome/updater/setup_mac.mm index 64f15852f3dfe0..4b6733d03759d8 100644 --- a/chrome/updater/setup_mac.mm +++ b/chrome/updater/setup_mac.mm @@ -13,18 +13,21 @@ #include "chrome/updater/constants.h" #include "chrome/updater/launchd_util.h" #include "chrome/updater/mac/xpc_service_names.h" +#include "chrome/updater/updater_scope.h" namespace updater { namespace { -void SetupDone(base::OnceCallback callback, int result) { +void SetupDone(base::OnceCallback callback, + UpdaterScope scope, + int result) { if (result != setup_exit_codes::kSuccess) { std::move(callback).Run(result); return; } PollLaunchctlList( - kUpdateServiceInternalLaunchdName, LaunchctlPresence::kPresent, + scope, kUpdateServiceInternalLaunchdName, LaunchctlPresence::kPresent, base::TimeDelta::FromSeconds(kWaitForLaunchctlUpdateSec), base::BindOnce( [](base::OnceCallback callback, bool service_exists) { @@ -39,10 +42,11 @@ void SetupDone(base::OnceCallback callback, int result) { } // namespace -void InstallCandidate(bool is_machine, base::OnceCallback callback) { +void InstallCandidate(UpdaterScope scope, + base::OnceCallback callback) { base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, base::BindOnce(&Setup), - base::BindOnce(&SetupDone, std::move(callback))); + FROM_HERE, {base::MayBlock()}, base::BindOnce(&Setup, scope), + base::BindOnce(&SetupDone, std::move(callback), scope)); } } // namespace updater diff --git a/chrome/updater/setup_win.cc b/chrome/updater/setup_win.cc index 7757fe08a7d7d8..2a88745883bd45 100644 --- a/chrome/updater/setup_win.cc +++ b/chrome/updater/setup_win.cc @@ -9,14 +9,15 @@ #include "base/callback.h" #include "base/task/task_traits.h" #include "base/task/thread_pool.h" +#include "chrome/updater/updater_scope.h" namespace updater { -void InstallCandidate(bool is_machine, +void InstallCandidate(UpdaterScope scope, base::OnceCallback callback) { - base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, base::BindOnce(&Setup, is_machine), - std::move(callback)); + base::ThreadPool::PostTaskAndReplyWithResult(FROM_HERE, {base::MayBlock()}, + base::BindOnce(&Setup, scope), + std::move(callback)); } } // namespace updater diff --git a/chrome/updater/test/integration_tests.cc b/chrome/updater/test/integration_tests.cc index 207c31f3dd23a8..27a7a16291e485 100644 --- a/chrome/updater/test/integration_tests.cc +++ b/chrome/updater/test/integration_tests.cc @@ -31,26 +31,269 @@ #include "chrome/updater/test/test_app/constants.h" #include "chrome/updater/test/test_app/test_app_version.h" #include "chrome/updater/update_service.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/updater_version.h" #include "chrome/updater/util.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" +#if defined(OS_WIN) +#include "base/strings/utf_string_conversions.h" +#endif // OS_WIN + namespace updater { namespace test { -// The project's position is that component builds are not portable outside of -// the build directory. Therefore, installation of component builds is not -// expected to work and these tests do not run on component builders. -// See crbug.com/1112527. -#if defined(OS_WIN) || !defined(COMPONENT_BUILD) +// TODO(crbug.com/1096654): Move the IntegrationTestCommands* classes, along +// with all the updater::test::Foo(UpdaterScope) implementations to their +// own file. +class IntegrationTestCommandsUser : public IntegrationTestCommands { + public: + IntegrationTestCommandsUser() = default; + + static scoped_refptr + CreateIntegrationTestCommands() { + return base::MakeRefCounted(); + } -namespace { + UpdaterScope scope() const override { return UpdaterScope::kUser; } -void ExpectActiveVersion(std::string expected) { - EXPECT_EQ(CreateGlobalPrefs()->GetActiveVersion(), expected); -} + void PrintLog() const override { updater::test::PrintLog(scope()); } + + void CopyLog() const override { + base::Optional path = GetDataDirPath(scope()); + EXPECT_TRUE(path); + if (path) + updater::test::CopyLog(*path); + } + + void Clean() const override { updater::test::Clean(scope()); } + + void ExpectClean() const override { updater::test::ExpectClean(scope()); } + + void Install() const override { updater::test::Install(scope()); } + + void ExpectInstalled() const override { + updater::test::ExpectInstalled(scope()); + } + + void Uninstall() const override { updater::test::Uninstall(scope()); } + + void ExpectCandidateUninstalled() const override { + updater::test::ExpectCandidateUninstalled(scope()); + } + + void EnterTestMode(const GURL& url) const override { + updater::test::EnterTestMode(url); + } + + void ExpectVersionActive(const std::string& version) const override { + updater::test::ExpectVersionActive(version); + } + + void ExpectVersionNotActive(const std::string& version) const override { + updater::test::ExpectVersionNotActive(version); + } + + void ExpectActiveUpdater() const override { + updater::test::ExpectActiveUpdater(scope()); + } + + void SetupFakeUpdaterHigherVersion() const override { + updater::test::SetupFakeUpdaterHigherVersion(scope()); + } + + void SetupFakeUpdaterLowerVersion() const override { + updater::test::SetupFakeUpdaterLowerVersion(scope()); + } + + void SetFakeExistenceCheckerPath(const std::string& app_id) const override { + updater::test::SetFakeExistenceCheckerPath(app_id); + } + + void ExpectAppUnregisteredExistenceCheckerPath( + const std::string& app_id) const override { + updater::test::ExpectAppUnregisteredExistenceCheckerPath(app_id); + } + + void SetActive(const std::string& app_id) const override { + updater::test::SetActive(scope(), app_id); + } + + void ExpectActive(const std::string& app_id) const override { + updater::test::ExpectActive(scope(), app_id); + } + + void ExpectNotActive(const std::string& app_id) const override { + updater::test::ExpectNotActive(scope(), app_id); + } + + void RunWake(int exit_code) const override { + updater::test::RunWake(scope(), exit_code); + } + +#if defined(OS_MAC) + void RegisterApp(const std::string& app_id) const override { + updater::test::RegisterApp(app_id); + } + + void RegisterTestApp() const override { + updater::test::RegisterTestApp(scope()); + } +#endif + + private: + ~IntegrationTestCommandsUser() override = default; +}; + +class IntegrationTestCommandsSystem : public IntegrationTestCommands { + public: + IntegrationTestCommandsSystem() = default; + + static scoped_refptr + CreateIntegrationTestCommands() { + return base::MakeRefCounted(); + } + + UpdaterScope scope() const override { return UpdaterScope::kSystem; } + + void PrintLog() const override { RunHelperCommand("print_log"); } + + void CopyLog() const override { + base::Optional path = GetDataDirPath(scope()); + ASSERT_TRUE(path); + +#if defined(OS_WIN) + RunHelperCommand("copy_log", + {HelperParam("path", base::WideToUTF8(path->value()))}); +#else + RunHelperCommand("copy_log", {HelperParam("path", path->value())}); +#endif // OS_WIN + } + + void Clean() const override { RunHelperCommand("clean"); } + + void ExpectClean() const override { RunHelperCommand("expect_clean"); } + + void Install() const override { RunHelperCommand("install"); } + + void ExpectInstalled() const override { + RunHelperCommand("expect_installed"); + } + + void Uninstall() const override { RunHelperCommand("uninstall"); } + + void ExpectCandidateUninstalled() const override { + RunHelperCommand("expect_candidate_uninstalled"); + } + + void EnterTestMode(const GURL& url) const override { + RunHelperCommand("enter_test_mode", {HelperParam("url", url.spec())}); + } + + void ExpectVersionActive(const std::string& version) const override { + RunHelperCommand("expect_version_active", + {HelperParam("version", version)}); + } + + void ExpectVersionNotActive(const std::string& version) const override { + RunHelperCommand("expect_version_not_active", + {HelperParam("version", version)}); + } + + void ExpectActiveUpdater() const override { + RunHelperCommand("expect_active_updater"); + } + + void ExpectActive(const std::string& app_id) const override { + RunHelperCommand("expect_active", {HelperParam("app_id", app_id)}); + } + + void ExpectNotActive(const std::string& app_id) const override { + RunHelperCommand("expect_not_active", {HelperParam("app_id", app_id)}); + } + + void SetupFakeUpdaterHigherVersion() const override { + RunHelperCommand("setup_fake_updater_higher_version"); + } + + void SetupFakeUpdaterLowerVersion() const override { + RunHelperCommand("setup_fake_updater_lower_version"); + } + + void SetFakeExistenceCheckerPath(const std::string& app_id) const override { + RunHelperCommand("set_fake_existence_checker_path", + {HelperParam("app_id", app_id)}); + } + + void ExpectAppUnregisteredExistenceCheckerPath( + const std::string& app_id) const override { + RunHelperCommand("expect_app_unregistered_existence_checker_path", + {HelperParam("app_id", app_id)}); + } + + void SetActive(const std::string& app_id) const override { + RunHelperCommand("set_active", {HelperParam("app_id", app_id)}); + } + + void RunWake(int expected_exit_code) const override { + RunHelperCommand( + "run_wake", + {HelperParam("exit_code", base::NumberToString(expected_exit_code))}); + } + +#if defined(OS_MAC) + void RegisterApp(const std::string& app_id) const override { + RunHelperCommand("register_app", {HelperParam("app_id", app_id)}); + } + + void RegisterTestApp() const override { + RunHelperCommand("register_test_app"); + } +#endif + + private: + ~IntegrationTestCommandsSystem() override = default; + + struct HelperParam { + HelperParam(const std::string& param_name, const std::string& param_value) + : param_name(param_name), param_value(param_value) {} + std::string param_name; + std::string param_value; + }; + + void RunHelperCommand(const std::string& command_switch, + const std::vector& params) const { + const base::CommandLine command_line = + *base::CommandLine::ForCurrentProcess(); + base::FilePath path(command_line.GetProgram()); + EXPECT_TRUE(base::PathExists(path)); + path = path.DirName(); + EXPECT_TRUE(base::PathExists(path)); + path = MakeAbsoluteFilePath(path); + path = path.Append(FILE_PATH_LITERAL("updater_integration_tests_helper")); + EXPECT_TRUE(base::PathExists(path)); + + base::CommandLine helper_command(path); + helper_command.AppendSwitch(command_switch); + + for (const HelperParam& param : params) { + helper_command.AppendSwitchASCII(param.param_name, param.param_value); + } + + helper_command.AppendSwitch(kEnableLoggingSwitch); + helper_command.AppendSwitchASCII(kLoggingModuleSwitch, "*/updater/*=2"); + + int exit_code = -1; + ASSERT_TRUE(Run(scope(), helper_command, &exit_code)); + EXPECT_EQ(exit_code, 0); + } + + void RunHelperCommand(const std::string& command_switch) const { + RunHelperCommand(command_switch, {}); + } +}; #if defined(OS_MAC) void RegisterApp(const std::string& app_id) { @@ -69,13 +312,20 @@ void RegisterApp(const std::string& app_id) { } #endif // defined(OS_MAC) -} // namespace +void ExpectVersionActive(const std::string& version) { + EXPECT_EQ(CreateGlobalPrefs()->GetActiveVersion(), version); +} + +void ExpectVersionNotActive(const std::string& version) { + EXPECT_NE(CreateGlobalPrefs()->GetActiveVersion(), version); +} -void PrintLog() { +void PrintLog(UpdaterScope scope) { std::string contents; - VLOG(0) << GetDataDirPath().AppendASCII("updater.log"); - if (base::ReadFileToString(GetDataDirPath().AppendASCII("updater.log"), - &contents)) { + base::Optional path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path && + base::ReadFileToString(path->AppendASCII("updater.log"), &contents)) { VLOG(0) << "Contents of updater.log:"; VLOG(0) << contents; } else { @@ -97,23 +347,29 @@ void CopyLog(const base::FilePath& src_dir) { // TODO(crbug.com/1159189): copy other test artifacts. base::FilePath dest_dir = GetLogDestinationDir(); if (base::PathExists(dest_dir) && base::PathExists(src_dir)) { - base::FilePath dest_file_path = dest_dir.AppendASCII( - base::StrCat({GetTestInfo()->test_suite_name(), ".", - GetTestInfo()->name(), "_updater.log"})); - EXPECT_TRUE( - base::CopyFile(src_dir.AppendASCII("updater.log"), dest_file_path)); + base::FilePath test_name_path = dest_dir.AppendASCII(base::StrCat( + {GetTestInfo()->test_suite_name(), ".", GetTestInfo()->name()})); + EXPECT_TRUE(base::CreateDirectory(test_name_path)); + + base::FilePath dest_file_path = test_name_path.AppendASCII("updater.log"); + base::FilePath log_path = src_dir.AppendASCII("updater.log"); + VLOG(0) << "Copying updater.log file. From: " << log_path + << ". To: " << dest_file_path; + EXPECT_TRUE(base::CopyFile(log_path, dest_file_path)); } } -void RunWake(int expected_exit_code) { - const base::FilePath installed_executable_path = GetInstalledExecutablePath(); - EXPECT_TRUE(base::PathExists(installed_executable_path)); - base::CommandLine command_line(installed_executable_path); +void RunWake(UpdaterScope scope, int expected_exit_code) { + const base::Optional installed_executable_path = + GetInstalledExecutablePath(scope); + ASSERT_TRUE(installed_executable_path); + EXPECT_TRUE(base::PathExists(*installed_executable_path)); + base::CommandLine command_line(*installed_executable_path); command_line.AppendSwitch(kWakeSwitch); command_line.AppendSwitch(kEnableLoggingSwitch); command_line.AppendSwitchASCII(kLoggingModuleSwitch, "*/updater/*=2"); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); EXPECT_EQ(exit_code, expected_exit_code); } @@ -126,41 +382,75 @@ void SetupFakeUpdaterPrefs(const base::Version& version) { ASSERT_EQ(version.GetString(), global_prefs->GetActiveVersion()); } -void SetupFakeUpdaterInstallFolder(const base::Version& version) { - const base::FilePath folder_path = GetFakeUpdaterInstallFolderPath(version); - ASSERT_TRUE(base::CreateDirectory(folder_path)); +void SetupFakeUpdaterInstallFolder(UpdaterScope scope, + const base::Version& version) { + const base::Optional folder_path = + GetFakeUpdaterInstallFolderPath(scope, version); + ASSERT_TRUE(folder_path); + ASSERT_TRUE(base::CreateDirectory(*folder_path)); } -void SetupFakeUpdater(const base::Version& version) { +void SetupFakeUpdater(UpdaterScope scope, const base::Version& version) { SetupFakeUpdaterPrefs(version); - SetupFakeUpdaterInstallFolder(version); + SetupFakeUpdaterInstallFolder(scope, version); } -void SetupFakeUpdaterVersion(int offset) { +void SetupFakeUpdaterVersion(UpdaterScope scope, int offset) { ASSERT_NE(offset, 0); std::vector components = base::Version(UPDATER_VERSION_STRING).components(); base::CheckedNumeric new_version = components[0]; new_version += offset; ASSERT_TRUE(new_version.AssignIfValid(&components[0])); - SetupFakeUpdater(base::Version(std::move(components))); + SetupFakeUpdater(scope, base::Version(std::move(components))); } -void SetupFakeUpdaterLowerVersion() { - SetupFakeUpdaterVersion(-1); +void SetupFakeUpdaterLowerVersion(UpdaterScope scope) { + SetupFakeUpdaterVersion(scope, -1); } -void SetupFakeUpdaterHigherVersion() { - SetupFakeUpdaterVersion(1); +void SetupFakeUpdaterHigherVersion(UpdaterScope scope) { + SetupFakeUpdaterVersion(scope, 1); } -bool Run(base::CommandLine command_line, int* exit_code) { +void SetFakeExistenceCheckerPath(const std::string& app_id) { + std::unique_ptr global_prefs = CreateGlobalPrefs(); + auto persisted_data = + base::MakeRefCounted(global_prefs->GetPrefService()); + base::FilePath fake_ecp = + persisted_data->GetExistenceCheckerPath(app_id).Append( + FILE_PATH_LITERAL("NOT_THERE")); + persisted_data->SetExistenceCheckerPath(app_id, fake_ecp); + + PrefsCommitPendingWrites(global_prefs->GetPrefService()); + + EXPECT_EQ(fake_ecp.value(), + persisted_data->GetExistenceCheckerPath(app_id).value()); +} + +void ExpectAppUnregisteredExistenceCheckerPath(const std::string& app_id) { + std::unique_ptr global_prefs = CreateGlobalPrefs(); + auto persisted_data = + base::MakeRefCounted(global_prefs->GetPrefService()); + EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("")).value(), + persisted_data->GetExistenceCheckerPath(app_id).value()); +} + +bool Run(UpdaterScope scope, base::CommandLine command_line, int* exit_code) { + base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait_process; command_line.AppendSwitch("enable-logging"); command_line.AppendSwitchASCII("vmodule", "*/updater/*=2"); + if (scope == UpdaterScope::kSystem) { + command_line.AppendSwitch(kSystemSwitch); + command_line = MakeElevated(command_line); + } + VLOG(0) << " Run command: " << command_line.GetCommandLineString(); base::Process process = base::LaunchProcess(command_line, {}); if (!process.IsValid()) return false; - return process.WaitForExitWithTimeout(TestTimeouts::action_max_timeout(), + + // TODO(crbug.com/1096654): Get the timeout from TestTimeouts. + return process.WaitForExitWithTimeout(base::TimeDelta::FromSeconds(45), exit_code); } @@ -176,9 +466,11 @@ void SleepFor(int seconds) { VLOG(2) << "Sleep complete."; } -class IntegrationTest : public ::testing::Test { +class IntegrationTest + : public ::testing::TestWithParam> { protected: void SetUp() override { + test_commands_ = GetParam(); Clean(); ExpectClean(); EnterTestMode(GURL("http://localhost:1234")); @@ -189,40 +481,99 @@ class IntegrationTest : public ::testing::Test { PrintLog(); // TODO(crbug.com/1159189): Use a specific test output directory // because Uninstall() deletes the files under GetDataDirPath(). - CopyLog(GetDataDirPath()); + CopyLog(); ExpectClean(); Clean(); } + scoped_refptr test_commands_; + + void CopyLog() { test_commands_->CopyLog(); } + void PrintLog() { test_commands_->PrintLog(); } + void Install() { test_commands_->Install(); } + void ExpectInstalled() { test_commands_->ExpectInstalled(); } + void Uninstall() { test_commands_->Uninstall(); } + void ExpectCandidateUninstalled() { + test_commands_->ExpectCandidateUninstalled(); + } + void Clean() { test_commands_->Clean(); } + void ExpectClean() { test_commands_->ExpectClean(); } + void EnterTestMode(const GURL& url) { test_commands_->EnterTestMode(url); } + void ExpectVersionActive(const std::string& version) { + test_commands_->ExpectVersionActive(version); + } + void ExpectVersionNotActive(const std::string& version) { + test_commands_->ExpectVersionNotActive(version); + } + void ExpectActiveUpdater() { test_commands_->ExpectActiveUpdater(); } + void SetupFakeUpdaterHigherVersion() { + test_commands_->SetupFakeUpdaterHigherVersion(); + } + void SetActive(const std::string& app_id) { + test_commands_->SetActive(app_id); + } + void ExpectActive(const std::string& app_id) { + test_commands_->ExpectActive(app_id); + } + void ExpectNotActive(const std::string& app_id) { + test_commands_->ExpectNotActive(app_id); + } + void SetFakeExistenceCheckerPath(const std::string& app_id) { + test_commands_->SetFakeExistenceCheckerPath(app_id); + } + void ExpectAppUnregisteredExistenceCheckerPath(const std::string& app_id) { + test_commands_->ExpectAppUnregisteredExistenceCheckerPath(app_id); + } + +#if defined(OS_MAC) + void RegisterApp(const std::string& app_id) { + test_commands_->RegisterApp(app_id); + } + void RegisterTestApp() { test_commands_->RegisterTestApp(); } +#endif + void RunWake(int exit_code) { test_commands_->RunWake(exit_code); } + UpdaterScope GetUpdaterScope() { return test_commands_->scope(); } private: base::test::TaskEnvironment environment_; }; -TEST_F(IntegrationTest, InstallUninstall) { +// The project's position is that component builds are not portable outside of +// the build directory. Therefore, installation of component builds is not +// expected to work and these tests do not run on component builders. +// See crbug.com/1112527. +#if defined(OS_WIN) || !defined(COMPONENT_BUILD) + +TEST_P(IntegrationTest, InstallUninstall) { Install(); ExpectInstalled(); - ExpectActiveVersion(UPDATER_VERSION_STRING); - ExpectActive(); + ExpectVersionActive(UPDATER_VERSION_STRING); + ExpectActiveUpdater(); Uninstall(); } -TEST_F(IntegrationTest, SelfUninstallOutdatedUpdater) { +TEST_P(IntegrationTest, SelfUninstallOutdatedUpdater) { + // TODO(crbug.com/1096654): Enable for system. + if (GetUpdaterScope() == UpdaterScope::kSystem) { + Uninstall(); + return; + } + Install(); ExpectInstalled(); SetupFakeUpdaterHigherVersion(); - EXPECT_NE(CreateGlobalPrefs()->GetActiveVersion(), UPDATER_VERSION_STRING); + ExpectVersionNotActive(UPDATER_VERSION_STRING); RunWake(0); // The mac server will remain active for 10 seconds after it replies to the // wake client, then shut down and uninstall itself. Sleep to wait for this // to happen. - SleepFor(11); + SleepFor(13); ExpectCandidateUninstalled(); // The candidate uninstall should not have altered global prefs. - EXPECT_NE(CreateGlobalPrefs()->GetActiveVersion(), UPDATER_VERSION_STRING); - EXPECT_NE(CreateGlobalPrefs()->GetActiveVersion(), "0.0.0.0"); + ExpectVersionNotActive(UPDATER_VERSION_STRING); + ExpectVersionNotActive("0.0.0.0"); Uninstall(); Clean(); @@ -230,17 +581,23 @@ TEST_F(IntegrationTest, SelfUninstallOutdatedUpdater) { #if defined(OS_MAC) // TODO(crbug.com/1163524): Enable on Windows. -TEST_F(IntegrationTest, RegisterTestApp) { +TEST_P(IntegrationTest, RegisterTestApp) { RegisterTestApp(); ExpectInstalled(); - ExpectActiveVersion(UPDATER_VERSION_STRING); - ExpectActive(); + ExpectVersionActive(UPDATER_VERSION_STRING); + ExpectActiveUpdater(); Uninstall(); } // TODO(crbug.com/1163524): Enable on Windows. // TODO(crbug.com/1163625): Failing on Mac 10.11. -TEST_F(IntegrationTest, ReportsActive) { +TEST_P(IntegrationTest, ReportsActive) { + // TODO(crbug.com/1096654): Enable for system. + if (GetUpdaterScope() == UpdaterScope::kSystem) { + Uninstall(); + return; + } + // A longer than usual timeout is needed for this test because the macOS // UpdateServiceInternal server takes at least 10 seconds to shut down after // Install, and RegisterApp cannot make progress until it shut downs and @@ -278,74 +635,38 @@ TEST_F(IntegrationTest, ReportsActive) { Uninstall(); } -TEST_F(IntegrationTest, UnregisterUninstalledApp) { +TEST_P(IntegrationTest, UnregisterUninstalledApp) { RegisterTestApp(); ExpectInstalled(); - ExpectActiveVersion(UPDATER_VERSION_STRING); - ExpectActive(); + ExpectVersionActive(UPDATER_VERSION_STRING); + ExpectActiveUpdater(); RegisterApp("test1"); RegisterApp("test2"); - { - std::unique_ptr global_prefs = CreateGlobalPrefs(); - auto persisted_data = - base::MakeRefCounted(global_prefs->GetPrefService()); - base::FilePath fake_ecp = - persisted_data->GetExistenceCheckerPath(kTestAppId) - .Append(FILE_PATH_LITERAL("NOT_THERE")); - persisted_data->SetExistenceCheckerPath(kTestAppId, fake_ecp); - - PrefsCommitPendingWrites(global_prefs->GetPrefService()); - - EXPECT_EQ(fake_ecp.value(), - persisted_data->GetExistenceCheckerPath(kTestAppId).value()); - } + SetFakeExistenceCheckerPath(kTestAppId); RunWake(0); SleepFor(13); ExpectInstalled(); - { - std::unique_ptr global_prefs = CreateGlobalPrefs(); - auto persisted_data = - base::MakeRefCounted(global_prefs->GetPrefService()); - EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("")).value(), - persisted_data->GetExistenceCheckerPath(kTestAppId).value()); - } + ExpectAppUnregisteredExistenceCheckerPath(kTestAppId); Uninstall(); - Clean(); } -TEST_F(IntegrationTest, UninstallUpdaterWhenAllAppsUninstalled) { +TEST_P(IntegrationTest, UninstallUpdaterWhenAllAppsUninstalled) { RegisterTestApp(); ExpectInstalled(); - ExpectActiveVersion(UPDATER_VERSION_STRING); - ExpectActive(); - - { - std::unique_ptr global_prefs = CreateGlobalPrefs(); - auto persisted_data = - base::MakeRefCounted(global_prefs->GetPrefService()); - const base::FilePath fake_ecp = - persisted_data->GetExistenceCheckerPath(kTestAppId) - .Append(FILE_PATH_LITERAL("NOT_THERE")); - persisted_data->SetExistenceCheckerPath(kTestAppId, fake_ecp); + ExpectVersionActive(UPDATER_VERSION_STRING); + ExpectActiveUpdater(); - PrefsCommitPendingWrites(global_prefs->GetPrefService()); - - EXPECT_EQ(fake_ecp.value(), - persisted_data->GetExistenceCheckerPath(kTestAppId).value()); - } + SetFakeExistenceCheckerPath(kTestAppId); RunWake(0); SleepFor(13); - - ExpectClean(); - Clean(); } // TODO(https://crbug.com/1166196): Fix flaky timeouts. The timeout is in @@ -355,11 +676,11 @@ TEST_F(IntegrationTest, UninstallUpdaterWhenAllAppsUninstalled) { #else #define MAYBE_UnregisterUnownedApp UnregisterUnownedApp #endif -TEST_F(IntegrationTest, MAYBE_UnregisterUnownedApp) { +TEST_P(IntegrationTest, MAYBE_UnregisterUnownedApp) { RegisterTestApp(); ExpectInstalled(); - ExpectActiveVersion(UPDATER_VERSION_STRING); - ExpectActive(); + ExpectVersionActive(UPDATER_VERSION_STRING); + ExpectActiveUpdater(); { std::unique_ptr global_prefs = CreateGlobalPrefs(); @@ -385,7 +706,6 @@ TEST_F(IntegrationTest, MAYBE_UnregisterUnownedApp) { } Uninstall(); - Clean(); } #endif // OS_MAC @@ -394,13 +714,32 @@ TEST_F(IntegrationTest, MAYBE_UnregisterUnownedApp) { // Tests the COM registration after the install. For now, tests that the // COM interfaces are registered, which is indirectly testing the type // library separation for the public, private, and legacy interfaces. -TEST_F(IntegrationTest, COMRegistration) { +TEST_P(IntegrationTest, COMRegistration) { Install(); ExpectInterfacesRegistered(); Uninstall(); } #endif // OS_WIN +#if defined(OS_MAC) +// TODO(crbug.com/1096654): Enable tests for IntegrationTestCommandsSystem on +// bots that support passwordless sudo. +INSTANTIATE_TEST_SUITE_P( + IntegrationTestVariations, + IntegrationTest, + testing::Values( + IntegrationTestCommandsUser::CreateIntegrationTestCommands())); +#endif + +#if defined(OS_WIN) +// TODO(crbug.com/1096654): Enable for system. +INSTANTIATE_TEST_SUITE_P( + IntegrationTestVariations, + IntegrationTest, + testing::Values( + IntegrationTestCommandsUser::CreateIntegrationTestCommands())); +#endif + #endif // defined(OS_WIN) || !defined(COMPONENT_BUILD) } // namespace test diff --git a/chrome/updater/test/integration_tests.h b/chrome/updater/test/integration_tests.h index a6509868c6960a..5fc3bee96b579a 100644 --- a/chrome/updater/test/integration_tests.h +++ b/chrome/updater/test/integration_tests.h @@ -7,31 +7,35 @@ #include +#include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/optional.h" #include "build/build_config.h" +#include "chrome/updater/updater_scope.h" namespace base { class CommandLine; -class FilePath; class Version; } // namespace base class GURL; namespace updater { + namespace test { // Prints the updater.log file to stdout. -void PrintLog(); +void PrintLog(UpdaterScope scope); // Removes traces of the updater from the system. It is best to run this at the // start of each test in case a previous crash or timeout on the machine running // the test left the updater in an installed or partially installed state. -void Clean(); +void Clean(UpdaterScope scope); // Expects that the system is in a clean state, i.e. no updater is installed and // no traces of an updater exist. Should be run at the start and end of each // test. -void ExpectClean(); +void ExpectClean(UpdaterScope scope); // Places the updater into test mode (use `url` as the update server and disable // CUP). @@ -46,69 +50,121 @@ void CopyLog(const base::FilePath& src_dir); void SleepFor(int seconds); // Returns the path to the updater data dir. -base::FilePath GetDataDirPath(); +base::Optional GetDataDirPath(UpdaterScope scope); // Expects that the updater is installed on the system. -void ExpectInstalled(); +void ExpectInstalled(UpdaterScope scope); // Installs the updater. -void Install(); +void Install(UpdaterScope scope); // Expects that the updater is installed on the system and the launchd tasks // are updated correctly. -void ExpectActive(); +void ExpectActiveUpdater(UpdaterScope scope); + +void ExpectVersionActive(const std::string& version); +void ExpectVersionNotActive(const std::string& version); // Uninstalls the updater. If the updater was installed during the test, it // should be uninstalled before the end of the test to avoid having an actual // live updater on the machine that ran the test. -void Uninstall(); +void Uninstall(UpdaterScope scope); // Runs the wake client and wait for it to exit. Assert that it exits with // `exit_code`. The server should exit a few seconds after. -void RunWake(int exit_code); +void RunWake(UpdaterScope scope, int exit_code); // Registers the test app. As a result, the bundled updater is installed, // promoted and registered. -void RegisterTestApp(); +void RegisterTestApp(UpdaterScope scope); // Runs the command and waits for it to exit or time out. -bool Run(base::CommandLine command_line, int* exit_code); +bool Run(UpdaterScope scope, base::CommandLine command_line, int* exit_code); // Returns the path of the Updater executable. -base::FilePath GetInstalledExecutablePath(); +base::Optional GetInstalledExecutablePath(UpdaterScope scope); // Returns the folder path under which the executable for the fake updater // should reside. -base::FilePath GetFakeUpdaterInstallFolderPath(const base::Version& version); +base::Optional GetFakeUpdaterInstallFolderPath( + UpdaterScope scope, + const base::Version& version); // Creates Prefs with the fake updater version set as active. void SetupFakeUpdaterPrefs(const base::Version& version); // Creates an install folder on the system with the fake updater version. -void SetupFakeUpdaterInstallFolder(const base::Version& version); +void SetupFakeUpdaterInstallFolder(UpdaterScope scope, + const base::Version& version); // Sets up a fake updater on the system at a version lower than the test. -void SetupFakeUpdaterLowerVersion(); +void SetupFakeUpdaterLowerVersion(UpdaterScope scope); // Sets up a fake updater on the system at a version higher than the test. -void SetupFakeUpdaterHigherVersion(); +void SetupFakeUpdaterHigherVersion(UpdaterScope scope); // Expects that this version of updater is uninstalled from the system. -void ExpectCandidateUninstalled(); +void ExpectCandidateUninstalled(UpdaterScope scope); // Sets the active bit for `app_id`. -void SetActive(const std::string& app_id); +void SetActive(UpdaterScope scope, const std::string& app_id); // Expects that the active bit for `app_id` is set. -void ExpectActive(const std::string& app_id); +void ExpectActive(UpdaterScope scope, const std::string& app_id); // Expects that the active bit for `app_id` is unset. -void ExpectNotActive(const std::string& app_id); +void ExpectNotActive(UpdaterScope scope, const std::string& app_id); + +void SetFakeExistenceCheckerPath(const std::string& app_id); +void ExpectAppUnregisteredExistenceCheckerPath(const std::string& app_id); + +#if defined(OS_MAC) +void RegisterApp(const std::string& app_id); +#endif #if defined(OS_WIN) void ExpectInterfacesRegistered(); #endif +class IntegrationTestCommands + : public base::RefCountedThreadSafe { + public: + static scoped_refptr CreateIntegrationTestCommands(); + + virtual UpdaterScope scope() const = 0; + + virtual void EnterTestMode(const GURL& url) const = 0; + virtual void Clean() const = 0; + virtual void ExpectClean() const = 0; + virtual void ExpectInstalled() const = 0; + virtual void ExpectCandidateUninstalled() const = 0; + virtual void Install() const = 0; + virtual void SetActive(const std::string& app_id) const = 0; + virtual void ExpectActiveUpdater() const = 0; + virtual void ExpectActive(const std::string& app_id) const = 0; + virtual void ExpectNotActive(const std::string& app_id) const = 0; + virtual void ExpectVersionActive(const std::string& version) const = 0; + virtual void ExpectVersionNotActive(const std::string& version) const = 0; + virtual void Uninstall() const = 0; +#if defined(OS_MAC) + virtual void RegisterApp(const std::string& app_id) const = 0; + virtual void RegisterTestApp() const = 0; +#endif + virtual void CopyLog() const = 0; + virtual void SetupFakeUpdaterHigherVersion() const = 0; + virtual void SetupFakeUpdaterLowerVersion() const = 0; + virtual void SetFakeExistenceCheckerPath(const std::string& app_id) const = 0; + virtual void ExpectAppUnregisteredExistenceCheckerPath( + const std::string& app_id) const = 0; + virtual void RunWake(int exit_code) const = 0; + virtual void PrintLog() const = 0; + + protected: + friend class base::RefCountedThreadSafe; + + virtual ~IntegrationTestCommands() = default; +}; + } // namespace test } // namespace updater diff --git a/chrome/updater/test/integration_tests_helper.cc b/chrome/updater/test/integration_tests_helper.cc new file mode 100644 index 00000000000000..ced6444e879282 --- /dev/null +++ b/chrome/updater/test/integration_tests_helper.cc @@ -0,0 +1,235 @@ +// Copyright 2021 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 + +#include "base/at_exit.h" +#include "base/command_line.h" +#include "base/logging.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/utf_string_conversions.h" +#include "base/task/single_thread_task_executor.h" +#include "base/task/thread_pool/thread_pool_instance.h" +#include "base/threading/thread_restrictions.h" +#include "base/threading/thread_task_runner_handle.h" +#include "build/build_config.h" +#include "chrome/updater/app/app.h" +#include "chrome/updater/constants.h" +#include "chrome/updater/test/integration_tests.h" +#include "chrome/updater/updater_scope.h" +#include "url/gurl.h" + +namespace updater { +namespace test { +namespace { + +constexpr char kAppId[] = "app_id"; +constexpr int kSuccess = 0; +constexpr int kUnknownSwitch = 101; +constexpr int kMissingAppIdSwitch = 102; +constexpr int kMissingUrlSwitch = 103; +constexpr int kMissingExitCodeSwitch = 104; +constexpr int kBadExitCodeSwitch = 105; +constexpr int kMissingPathSwitch = 106; +constexpr int kMissingVersionParameter = 107; + +class AppTestHelper : public App { + private: + ~AppTestHelper() override = default; + void FirstTaskRun() override; + void InitializeThreadPool() override; +}; + +void AppTestHelper::FirstTaskRun() { + base::ScopedAllowBlockingForTesting allow_blocking; + const base::CommandLine* command_line = + base::CommandLine::ForCurrentProcess(); + scoped_refptr task_runner = + base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()}); + + if (command_line->HasSwitch("clean")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&Clean, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("expect_clean")) { + ExpectClean(UpdaterScope::kSystem); + Shutdown(kSuccess); + } else if (command_line->HasSwitch("install")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&Install, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("expect_installed")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&ExpectInstalled, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("uninstall")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&Uninstall, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("expect_candidate_uninstalled")) { + task_runner->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&ExpectCandidateUninstalled, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("enter_test_mode")) { + if (command_line->HasSwitch("url")) { + GURL url(command_line->GetSwitchValueASCII("url")); + EnterTestMode(url); + Shutdown(kSuccess); + } else { + Shutdown(kMissingUrlSwitch); + } + } else if (command_line->HasSwitch("expect_active_updater")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&ExpectActiveUpdater, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("expect_version_active")) { + if (command_line->HasSwitch("version")) { + task_runner->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&ExpectVersionActive, + command_line->GetSwitchValueASCII("version")), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else { + Shutdown(kMissingVersionParameter); + } + } else if (command_line->HasSwitch("expect_version_not_active")) { + if (command_line->HasSwitch("version")) { + task_runner->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&ExpectVersionNotActive, + command_line->GetSwitchValueASCII("version")), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else { + Shutdown(kMissingVersionParameter); + } + } else if (command_line->HasSwitch("run_wake")) { + if (command_line->HasSwitch("exit_code")) { + int exit_code = -1; + if (base::StringToInt(command_line->GetSwitchValueASCII("exit_code"), + &exit_code)) { + RunWake(UpdaterScope::kSystem, exit_code); + Shutdown(kSuccess); + } else { + Shutdown(kBadExitCodeSwitch); + } + } else { + Shutdown(kMissingExitCodeSwitch); + } + } else if (command_line->HasSwitch("setup_fake_updater_higher_version")) { + SetupFakeUpdaterHigherVersion(UpdaterScope::kSystem); + Shutdown(kSuccess); + } else if (command_line->HasSwitch("setup_fake_updater_lower_version")) { + SetupFakeUpdaterLowerVersion(UpdaterScope::kSystem); + Shutdown(kSuccess); + } else if (command_line->HasSwitch("set_fake_existence_checker_path")) { + if (command_line->HasSwitch(kAppId)) { + SetFakeExistenceCheckerPath(command_line->GetSwitchValueASCII(kAppId)); + Shutdown(kSuccess); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch( + "expect_app_unregistered_existence_checker_path")) { + if (command_line->HasSwitch(kAppId)) { + ExpectAppUnregisteredExistenceCheckerPath( + command_line->GetSwitchValueASCII(kAppId)); + Shutdown(kSuccess); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch("set_active")) { + if (command_line->HasSwitch(kAppId)) { + task_runner->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&SetActive, UpdaterScope::kSystem, + command_line->GetSwitchValueASCII(kAppId)), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch("expect_active")) { + if (command_line->HasSwitch(kAppId)) { + task_runner->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&ExpectActive, UpdaterScope::kSystem, + command_line->GetSwitchValueASCII(kAppId)), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch("expect_not_active")) { + if (command_line->HasSwitch(kAppId)) { + ExpectNotActive(UpdaterScope::kSystem, + command_line->GetSwitchValueASCII(kAppId)); + Shutdown(kSuccess); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch("print_log")) { + task_runner->PostTaskAndReply( + FROM_HERE, base::BindOnce(&PrintLog, UpdaterScope::kSystem), + base::BindOnce(&AppTestHelper::Shutdown, this, kSuccess)); + } else if (command_line->HasSwitch("copy_log")) { + if (command_line->HasSwitch("path")) { + const std::string path_string = command_line->GetSwitchValueASCII("path"); + base::FilePath path; +#if defined(OS_WIN) + base::FilePath::StringType str; + path = base::UTF8ToWide(path_string.c_str(), path_string.size(), &str) + ? base::FilePath(str) + : base::FilePath(); +#else + path = base::FilePath(path_string); +#endif // OS_WIN + CopyLog(path); + Shutdown(kSuccess); + } else { + Shutdown(kMissingPathSwitch); + } + } +#if defined(OS_MAC) + else if (command_line->HasSwitch("register_app")) { + if (command_line->HasSwitch(kAppId)) { + RegisterApp(command_line->GetSwitchValueASCII(kAppId)); + Shutdown(kSuccess); + } else { + Shutdown(kMissingAppIdSwitch); + } + } else if (command_line->HasSwitch("register_test_app")) { + RegisterTestApp(UpdaterScope::kSystem); + Shutdown(kSuccess); + } +#endif + else { + VLOG(0) << "No supported switch provided. Command: " + << command_line->GetCommandLineString(); + Shutdown(kUnknownSwitch); + } +} + +void AppTestHelper::InitializeThreadPool() { + base::ThreadPoolInstance::CreateAndStartWithDefaultParams("test_helper"); +} + +scoped_refptr MakeAppTestHelper() { + return base::MakeRefCounted(); +} + +int IntegrationTestsHelperMain(int argc, char** argv) { + base::PlatformThread::SetName("IntegrationTestsHelperMain"); + base::AtExitManager exit_manager; + base::SingleThreadTaskExecutor main_task_executor(base::MessagePumpType::UI); + + base::CommandLine::Init(argc, argv); + return MakeAppTestHelper()->Run(); +} + +} // namespace +} // namespace test +} // namespace updater + +int main(int argc, char** argv) { + return updater::test::IntegrationTestsHelperMain(argc, argv); +} diff --git a/chrome/updater/test/integration_tests_mac.mm b/chrome/updater/test/integration_tests_mac.mm index c9e1ddd9c3cee9..d3aeb9e8896b2d 100644 --- a/chrome/updater/test/integration_tests_mac.mm +++ b/chrome/updater/test/integration_tests_mac.mm @@ -9,7 +9,9 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/files/file_util.h" +#include "base/logging.h" #include "base/mac/foundation_util.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/run_loop.h" #include "base/strings/sys_string_conversions.h" @@ -19,13 +21,14 @@ #include "chrome/updater/constants.h" #include "chrome/updater/external_constants_builder.h" #include "chrome/updater/launchd_util.h" -#import "chrome/updater/mac/util.h" +#import "chrome/updater/mac/mac_util.h" #include "chrome/updater/mac/xpc_service_names.h" #include "chrome/updater/prefs.h" #include "chrome/updater/test/integration_tests.h" #include "chrome/updater/test/test_app/constants.h" #include "chrome/updater/test/test_app/test_app_version.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/util.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -33,20 +36,24 @@ namespace updater { namespace test { -// crbug.com/1112527: These tests are not compatible with component build. -#if !defined(COMPONENT_BUILD) - namespace { -void RemoveJobFromLaunchd(Launchd::Domain domain, - Launchd::Type type, - base::ScopedCFTypeRef name) { - EXPECT_TRUE(Launchd::GetInstance()->DeletePlist(domain, type, name)) - << "Failed to delete plist for " << name; +Launchd::Domain LaunchdDomain(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return Launchd::Domain::Local; + case UpdaterScope::kUser: + return Launchd::Domain::User; + } +} - // Return value is ignored, since RemoveJob returns false if the job already - // doesn't exist. - Launchd::GetInstance()->RemoveJob(base::SysCFStringRefToUTF8(name)); +Launchd::Type LaunchdType(UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kSystem: + return Launchd::Type::Daemon; + case UpdaterScope::kUser: + return Launchd::Type::Agent; + } } base::FilePath GetExecutablePath() { @@ -71,25 +78,32 @@ void RemoveJobFromLaunchd(Launchd::Domain domain, .Append(FILE_PATH_LITERAL(TEST_APP_FULLNAME_STRING)); } -base::FilePath GetProductPath() { - return base::mac::GetUserLibraryPath() - .AppendASCII(COMPANY_SHORTNAME_STRING) +base::Optional GetProductPath(UpdaterScope scope) { + base::Optional path = GetLibraryFolderPath(scope); + if (!path) + return base::nullopt; + + return path->AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(PRODUCT_FULLNAME_STRING); } -base::FilePath GetActiveFile(const std::string& id) { - return base::GetHomeDir() - .AppendASCII("Library") - .AppendASCII(COMPANY_SHORTNAME_STRING) +base::Optional GetActiveFile(UpdaterScope scope, + const std::string& id) { + base::Optional path = GetLibraryFolderPath(scope); + if (!path) + return base::nullopt; + + return path->AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(COMPANY_SHORTNAME_STRING "SoftwareUpdate") .AppendASCII("Actives") .AppendASCII(id); } -void ExpectServiceAbsent(const std::string& service) { +void ExpectServiceAbsent(UpdaterScope scope, const std::string& service) { + VLOG(0) << __func__ << " - scope: " << scope << ". service: " << service; bool success = false; base::RunLoop loop; - PollLaunchctlList(service, LaunchctlPresence::kAbsent, + PollLaunchctlList(scope, service, LaunchctlPresence::kAbsent, base::TimeDelta::FromSeconds(7), base::BindLambdaForTesting([&](bool result) { success = result; @@ -101,8 +115,6 @@ void ExpectServiceAbsent(const std::string& service) { } // namespace -#endif // defined(COMPONENT_BUILD - void EnterTestMode(const GURL& url) { ASSERT_TRUE(ExternalConstantsBuilder() .SetUpdateURL(std::vector{url.spec()}) @@ -112,143 +124,195 @@ void EnterTestMode(const GURL& url) { .Overwrite()); } -// crbug.com/1112527: These tests are not compatible with component build. -#if !defined(COMPONENT_BUILD) +base::Optional GetDataDirPath(UpdaterScope scope) { + base::Optional app_path = + GetApplicationSupportDirectory(scope); + if (!app_path) { + VLOG(1) << "Failed to get Application support path."; + return base::nullopt; + } -base::FilePath GetDataDirPath() { - return base::mac::GetUserLibraryPath() - .AppendASCII("Application Support") - .AppendASCII(COMPANY_SHORTNAME_STRING) + return app_path->AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(PRODUCT_FULLNAME_STRING); } -void Clean() { - EXPECT_TRUE(base::DeletePathRecursively(GetProductPath())); +void Clean(UpdaterScope scope) { + Launchd::Domain launchd_domain = LaunchdDomain(scope); + Launchd::Type launchd_type = LaunchdType(scope); + + base::Optional path = GetProductPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::DeletePathRecursively(*path)); EXPECT_TRUE(Launchd::GetInstance()->DeletePlist( - Launchd::User, Launchd::Agent, updater::CopyWakeLaunchdName())); + launchd_domain, launchd_type, updater::CopyWakeLaunchdName())); EXPECT_TRUE(Launchd::GetInstance()->DeletePlist( - Launchd::User, Launchd::Agent, + launchd_domain, launchd_type, updater::CopyUpdateServiceInternalLaunchdName())); EXPECT_TRUE(Launchd::GetInstance()->DeletePlist( - Launchd::User, Launchd::Agent, updater::CopyUpdateServiceLaunchdName())); - EXPECT_TRUE(base::DeletePathRecursively(GetDataDirPath())); + launchd_domain, launchd_type, updater::CopyUpdateServiceLaunchdName())); + + path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::DeletePathRecursively(*path)); @autoreleasepool { - // TODO(crbug.com/1096654): support machine case (Launchd::Domain::Local and - // Launchd::Type::Daemon). - RemoveJobFromLaunchd(Launchd::Domain::User, Launchd::Type::Agent, + RemoveJobFromLaunchd(scope, launchd_domain, launchd_type, CopyUpdateServiceLaunchdName()); - RemoveJobFromLaunchd(Launchd::Domain::User, Launchd::Type::Agent, + RemoveJobFromLaunchd(scope, launchd_domain, launchd_type, CopyUpdateServiceInternalLaunchdName()); } } -void ExpectClean() { +void ExpectClean(UpdaterScope scope) { + Launchd::Domain launchd_domain = LaunchdDomain(scope); + Launchd::Type launchd_type = LaunchdType(scope); + // Files must not exist on the file system. - EXPECT_FALSE(base::PathExists(GetProductPath())); + base::Optional path = GetProductPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_FALSE(base::PathExists(*path)); EXPECT_FALSE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, updater::CopyWakeLaunchdName())); + launchd_domain, launchd_type, updater::CopyWakeLaunchdName())); EXPECT_FALSE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, + launchd_domain, launchd_type, updater::CopyUpdateServiceInternalLaunchdName())); EXPECT_FALSE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, updater::CopyUpdateServiceLaunchdName())); - EXPECT_FALSE(base::PathExists(GetDataDirPath())); - ExpectServiceAbsent(kUpdateServiceLaunchdName); - ExpectServiceAbsent(kUpdateServiceInternalLaunchdName); + launchd_domain, launchd_type, updater::CopyUpdateServiceLaunchdName())); + + path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_FALSE(base::PathExists(*path)); + + ExpectServiceAbsent(scope, kUpdateServiceLaunchdName); + ExpectServiceAbsent(scope, kUpdateServiceInternalLaunchdName); } -void ExpectInstalled() { +void ExpectInstalled(UpdaterScope scope) { + Launchd::Domain launchd_domain = LaunchdDomain(scope); + Launchd::Type launchd_type = LaunchdType(scope); + + VLOG(0) << "Scope :" << scope + << "; GetProductPath(scope): " << GetProductPath(scope); // Files must exist on the file system. - EXPECT_TRUE(base::PathExists(GetProductPath())); - EXPECT_TRUE(Launchd::GetInstance()->PlistExists(Launchd::User, Launchd::Agent, + base::Optional path = GetProductPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::PathExists(*path)); + + EXPECT_TRUE(Launchd::GetInstance()->PlistExists(launchd_domain, launchd_type, CopyWakeLaunchdName())); EXPECT_TRUE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, CopyUpdateServiceInternalLaunchdName())); + launchd_domain, launchd_type, CopyUpdateServiceInternalLaunchdName())); } -void Install() { +void Install(UpdaterScope scope) { const base::FilePath path = GetExecutablePath(); ASSERT_FALSE(path.empty()); base::CommandLine command_line(path); command_line.AppendSwitch(kInstallSwitch); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); - EXPECT_EQ(0, exit_code); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); + EXPECT_EQ(exit_code, 0); } -void ExpectActive() { +void ExpectActiveUpdater(UpdaterScope scope) { + Launchd::Domain launchd_domain = LaunchdDomain(scope); + Launchd::Type launchd_type = LaunchdType(scope); + // Files must exist on the file system. - EXPECT_TRUE(base::PathExists(GetProductPath())); + base::Optional path = GetProductPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::PathExists(*path)); + EXPECT_TRUE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, CopyUpdateServiceLaunchdName())); + launchd_domain, launchd_type, CopyUpdateServiceLaunchdName())); } -void RegisterTestApp() { +void RegisterTestApp(UpdaterScope scope) { const base::FilePath path = GetTestAppExecutablePath(); ASSERT_FALSE(path.empty()); base::CommandLine command_line(path); command_line.AppendSwitch(kRegisterUpdaterSwitch); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); - EXPECT_EQ(0, exit_code); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); + EXPECT_EQ(exit_code, 0); } -base::FilePath GetInstalledExecutablePath() { - return GetUpdaterExecutablePath(); +base::Optional GetInstalledExecutablePath(UpdaterScope scope) { + return GetUpdaterExecutablePath(scope); } -void ExpectCandidateUninstalled() { - base::FilePath versioned_folder_path = GetVersionedUpdaterFolderPath(); - EXPECT_FALSE(base::PathExists(versioned_folder_path)); - EXPECT_FALSE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, CopyWakeLaunchdName())); +void ExpectCandidateUninstalled(UpdaterScope scope) { + Launchd::Domain launchd_domain = LaunchdDomain(scope); + Launchd::Type launchd_type = LaunchdType(scope); + + base::Optional versioned_folder_path = + GetVersionedUpdaterFolderPath(scope); + EXPECT_TRUE(versioned_folder_path); + if (versioned_folder_path) + EXPECT_FALSE(base::PathExists(*versioned_folder_path)); + + EXPECT_FALSE(Launchd::GetInstance()->PlistExists(launchd_domain, launchd_type, + CopyWakeLaunchdName())); EXPECT_FALSE(Launchd::GetInstance()->PlistExists( - Launchd::User, Launchd::Agent, CopyUpdateServiceInternalLaunchdName())); + launchd_domain, launchd_type, CopyUpdateServiceInternalLaunchdName())); } -void Uninstall() { +void Uninstall(UpdaterScope scope) { if (::testing::Test::HasFailure()) - PrintLog(); + PrintLog(scope); // Copy logs from GetDataDirPath() before updater uninstalls itself // and deletes the path. - CopyLog(GetDataDirPath()); + base::Optional path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path) + CopyLog(*path); // Uninstall the updater. - const base::FilePath path = GetExecutablePath(); - ASSERT_FALSE(path.empty()); - base::CommandLine command_line(path); + path = GetExecutablePath(); + ASSERT_TRUE(path); + base::CommandLine command_line(*path); command_line.AppendSwitch(kUninstallSwitch); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); - EXPECT_EQ(0, exit_code); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); + EXPECT_EQ(exit_code, 0); } -base::FilePath GetFakeUpdaterInstallFolderPath(const base::Version& version) { - return GetExecutableFolderPathForVersion(version); +base::Optional GetFakeUpdaterInstallFolderPath( + UpdaterScope scope, + const base::Version& version) { + return GetExecutableFolderPathForVersion(scope, version); } -void SetActive(const std::string& app_id) { +void SetActive(UpdaterScope scope, const std::string& app_id) { base::File::Error err = base::File::FILE_OK; - base::FilePath actives_file = GetActiveFile(app_id); - EXPECT_TRUE(base::CreateDirectoryAndGetError(actives_file.DirName(), &err)) + base::Optional actives_file = GetActiveFile(scope, app_id); + ASSERT_TRUE(actives_file); + VLOG(0) << "Actives file: " << *actives_file; + VLOG(0) << "Actives file dir name: " << actives_file->DirName(); + EXPECT_TRUE(base::CreateDirectoryAndGetError(actives_file->DirName(), &err)) << "Error: " << err; - EXPECT_TRUE(base::WriteFile(actives_file, "")); + EXPECT_TRUE(base::WriteFile(*actives_file, "")); } -void ExpectActive(const std::string& app_id) { - base::FilePath path = GetActiveFile(app_id); - EXPECT_TRUE(base::PathExists(path) && base::PathIsWritable(path)) - << app_id << " is not active"; +void ExpectActive(UpdaterScope scope, const std::string& app_id) { + base::Optional path = GetActiveFile(scope, app_id); + ASSERT_TRUE(path); + EXPECT_TRUE(base::PathExists(*path)); + EXPECT_TRUE(base::PathIsWritable(*path)); } -void ExpectNotActive(const std::string& app_id) { - base::FilePath path = GetActiveFile(app_id); - EXPECT_FALSE(base::PathExists(path) && base::PathIsWritable(path)) - << app_id << " is active."; +void ExpectNotActive(UpdaterScope scope, const std::string& app_id) { + base::Optional path = GetActiveFile(scope, app_id); + ASSERT_TRUE(path); + EXPECT_FALSE(base::PathExists(*path)); + EXPECT_FALSE(base::PathIsWritable(*path)); } -#endif // !defined(COMPONENT_BUILD) - } // namespace test } // namespace updater diff --git a/chrome/updater/test/integration_tests_win.cc b/chrome/updater/test/integration_tests_win.cc index e4411de34026c8..53682c5c96d7e0 100644 --- a/chrome/updater/test/integration_tests_win.cc +++ b/chrome/updater/test/integration_tests_win.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/files/file_util.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/strings/strcat.h" #include "base/strings/string16.h" @@ -25,6 +26,7 @@ #include "chrome/updater/external_constants_builder.h" #include "chrome/updater/test/integration_tests.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/updater_version.h" #include "chrome/updater/util.h" #include "chrome/updater/win/constants.h" @@ -44,10 +46,10 @@ base::FilePath GetInstallerPath() { return test_executable.DirName().AppendASCII("UpdaterSetup.exe"); } -base::FilePath GetProductPath() { +base::Optional GetProductPath() { base::FilePath app_data_dir; if (!base::PathService::Get(base::DIR_LOCAL_APP_DATA, &app_data_dir)) - return base::FilePath(); + return base::nullopt; return app_data_dir.AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(PRODUCT_FULLNAME_STRING) .AppendASCII(UPDATER_VERSION_STRING); @@ -59,33 +61,47 @@ std::wstring GetAppClientStateKey(const std::string& id) { } // namespace -base::FilePath GetInstalledExecutablePath() { - return GetProductPath().AppendASCII("updater.exe"); +base::Optional GetInstalledExecutablePath(UpdaterScope scope) { + base::Optional path = GetProductPath(); + if (!path) + return base::nullopt; + return path->AppendASCII("updater.exe"); } -base::FilePath GetFakeUpdaterInstallFolderPath(const base::Version& version) { - return GetProductPath().AppendASCII(version.GetString()); +base::Optional GetFakeUpdaterInstallFolderPath( + UpdaterScope scope, + const base::Version& version) { + base::Optional path = GetProductPath(); + if (!path) + return base::nullopt; + return path->AppendASCII(version.GetString()); } -base::FilePath GetDataDirPath() { +base::Optional GetDataDirPath(UpdaterScope scope) { base::FilePath app_data_dir; if (!base::PathService::Get(base::DIR_LOCAL_APP_DATA, &app_data_dir)) - return base::FilePath(); + return base::nullopt; return app_data_dir.AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(PRODUCT_FULLNAME_STRING); } -void Clean() { +void Clean(UpdaterScope scope) { // TODO(crbug.com/1062288): Delete the Client / ClientState registry keys. // TODO(crbug.com/1062288): Delete the COM server items. // TODO(crbug.com/1062288): Delete the COM service items. // TODO(crbug.com/1062288): Delete the COM interfaces. // TODO(crbug.com/1062288): Delete the Wake task. - EXPECT_TRUE(base::DeletePathRecursively(GetProductPath())); - EXPECT_TRUE(base::DeletePathRecursively(GetDataDirPath())); + base::Optional path = GetProductPath(); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::DeletePathRecursively(*path)); + path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::DeletePathRecursively(*path)); } -void ExpectClean() { +void ExpectClean(UpdaterScope scope) { // TODO(crbug.com/1062288): Assert there are no Client / ClientState registry // keys. // TODO(crbug.com/1062288): Assert there is no UpdateDev registry key. @@ -95,8 +111,14 @@ void ExpectClean() { // TODO(crbug.com/1062288): Assert there are no Wake tasks. // Files must not exist on the file system. - EXPECT_FALSE(base::PathExists(GetProductPath())); - EXPECT_FALSE(base::PathExists(GetDataDirPath())); + base::Optional path = GetProductPath(); + EXPECT_TRUE(path); + if (path) + EXPECT_FALSE(base::PathExists(*path)); + path = GetDataDirPath(scope); + EXPECT_TRUE(path); + if (path) + EXPECT_FALSE(base::PathExists(*path)); } void EnterTestMode(const GURL& url) { @@ -107,7 +129,7 @@ void EnterTestMode(const GURL& url) { .Overwrite()); } -void ExpectInstalled() { +void ExpectInstalled(UpdaterScope scope) { // TODO(crbug.com/1062288): Assert there are Client / ClientState registry // keys. // TODO(crbug.com/1062288): Assert there are COM server items. @@ -116,40 +138,52 @@ void ExpectInstalled() { // TODO(crbug.com/1062288): Assert there are Wake tasks. // Files must exist on the file system. - EXPECT_TRUE(base::PathExists(GetProductPath())); + base::Optional path = GetProductPath(); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::PathExists(*path)); } -void ExpectCandidateUninstalled() { +void ExpectCandidateUninstalled(UpdaterScope scope) { // TODO(crbug.com/1062288): Assert there are no side-by-side COM interfaces. // TODO(crbug.com/1062288): Assert there are no Wake tasks. // Files must not exist on the file system. - EXPECT_FALSE(base::PathExists(GetProductPath())); + base::Optional path = GetProductPath(); + EXPECT_TRUE(path); + if (path) + EXPECT_FALSE(base::PathExists(*path)); } -void ExpectActive() { +void ExpectActiveUpdater(UpdaterScope scope) { // TODO(crbug.com/1062288): Assert that COM interfaces point to this version. // Files must exist on the file system. - EXPECT_TRUE(base::PathExists(GetProductPath())); + base::Optional path = GetProductPath(); + EXPECT_TRUE(path); + if (path) + EXPECT_TRUE(base::PathExists(*path)); } -void Install() { +void Install(UpdaterScope scope) { const base::FilePath path = GetInstallerPath(); ASSERT_FALSE(path.empty()); base::CommandLine command_line(path); command_line.AppendSwitch(kInstallSwitch); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); EXPECT_EQ(0, exit_code); } -void Uninstall() { +void Uninstall(UpdaterScope scope) { if (::testing::Test::HasFailure()) - PrintLog(); + PrintLog(scope); // Copy logs from GetDataDirPath() before updater uninstalls itself // and deletes the path. - CopyLog(GetDataDirPath()); + base::Optional data_dir_path = GetDataDirPath(scope); + EXPECT_TRUE(data_dir_path); + if (data_dir_path) + CopyLog(*data_dir_path); // Note: updater.exe --uninstall is run from the build dir, not the install // dir, because it is useful for tests to be able to run it to clean the @@ -160,7 +194,7 @@ void Uninstall() { base::CommandLine command_line(path); command_line.AppendSwitch("uninstall"); int exit_code = -1; - ASSERT_TRUE(Run(command_line, &exit_code)); + ASSERT_TRUE(Run(scope, command_line, &exit_code)); EXPECT_EQ(0, exit_code); // Uninstallation involves a race with the uninstall.cmd script and the @@ -168,7 +202,7 @@ void Uninstall() { SleepFor(5); } -void SetActive(const std::string& id) { +void SetActive(UpdaterScope scope, const std::string& id) { // TODO(crbug/1159498): Standardize registry access. base::win::RegKey key; ASSERT_EQ(key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), @@ -177,7 +211,7 @@ void SetActive(const std::string& id) { EXPECT_EQ(key.WriteValue(kDidRun, L"1"), ERROR_SUCCESS); } -void ExpectActive(const std::string& id) { +void ExpectActive(UpdaterScope scope, const std::string& id) { // TODO(crbug/1159498): Standardize registry access. base::win::RegKey key; ASSERT_EQ(key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), @@ -188,7 +222,7 @@ void ExpectActive(const std::string& id) { EXPECT_EQ(value, L"1"); } -void ExpectNotActive(const std::string& id) { +void ExpectNotActive(UpdaterScope scope, const std::string& id) { // TODO(crbug/1159498): Standardize registry access. base::win::RegKey key; if (key.Open(HKEY_CURRENT_USER, GetAppClientStateKey(id).c_str(), diff --git a/chrome/updater/test/test_app/test_app_mac.mm b/chrome/updater/test/test_app/test_app_mac.mm index 9e06ce88e88ab8..6f5dc1f94b08df 100644 --- a/chrome/updater/test/test_app/test_app_mac.mm +++ b/chrome/updater/test/test_app/test_app_mac.mm @@ -19,6 +19,8 @@ #include "chrome/updater/mac/xpc_service_names.h" #include "chrome/updater/test/test_app/constants.h" #include "chrome/updater/test/test_app/test_app_version.h" +#include "chrome/updater/updater_scope.h" +#include "chrome/updater/util.h" namespace updater { @@ -65,6 +67,10 @@ int InstallUpdater() { base::CommandLine command(updater_executable_path); command.AppendSwitch(kInstallSwitch); + if (GetProcessScope() == UpdaterScope::kSystem) { + command.AppendSwitch(kSystemSwitch); + command = MakeElevated(command); + } command.AppendSwitchASCII("--vmodule", "*/updater/*=2"); std::string output; diff --git a/chrome/updater/update_service.h b/chrome/updater/update_service.h index 1f09840e37f4f1..0125b11e26ba10 100644 --- a/chrome/updater/update_service.h +++ b/chrome/updater/update_service.h @@ -156,15 +156,6 @@ class UpdateService : public base::RefCountedThreadSafe { kForeground = 2, }; - // Scope of the update service invocation. - enum class Scope { - // The updater is running in the logged in user's scope. - kUser = 1, - - // The updater is running in the system's scope. - kSystem = 2, - }; - using StateChangeCallback = base::RepeatingCallback; using Callback = base::OnceCallback; diff --git a/chrome/updater/updater.cc b/chrome/updater/updater.cc index 233209fa1c14a3..c3bfca8b9d9152 100644 --- a/chrome/updater/updater.cc +++ b/chrome/updater/updater.cc @@ -9,6 +9,7 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/message_loop/message_pump_type.h" +#include "base/optional.h" #include "base/task/single_thread_task_executor.h" #include "build/build_config.h" #include "chrome/updater/app/app.h" @@ -53,9 +54,12 @@ namespace { // The log file is created in DIR_LOCAL_APP_DATA or DIR_APP_DATA. void InitLogging(const base::CommandLine& command_line) { logging::LoggingSettings settings; - base::FilePath log_dir; - GetBaseDirectory(&log_dir); - const auto log_file = log_dir.Append(FILE_PATH_LITERAL("updater.log")); + base::Optional log_dir = GetBaseDirectory(); + if (!log_dir) { + LOG(ERROR) << "Error getting base dir."; + return; + } + const auto log_file = log_dir->Append(FILE_PATH_LITERAL("updater.log")); settings.log_file_path = log_file.value().c_str(); settings.logging_dest = logging::LOG_TO_ALL; logging::InitLogging(settings); diff --git a/chrome/updater/updater_scope.h b/chrome/updater/updater_scope.h new file mode 100644 index 00000000000000..ebb33f558e703b --- /dev/null +++ b/chrome/updater/updater_scope.h @@ -0,0 +1,41 @@ +// Copyright 2021 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. + +#ifndef CHROME_UPDATER_UPDATER_SCOPE_H_ +#define CHROME_UPDATER_UPDATER_SCOPE_H_ + +#include + +#include "base/command_line.h" +#include "chrome/updater/constants.h" + +namespace updater { + +// Scope of the service invocation. +enum class UpdaterScope { + // The updater is running in the logged in user's scope. + kUser = 1, + + // The updater is running in the system's scope. + kSystem = 2, +}; + +inline UpdaterScope GetProcessScope() { + return base::CommandLine::ForCurrentProcess()->HasSwitch(kSystemSwitch) + ? UpdaterScope::kSystem + : UpdaterScope::kUser; +} + +inline std::ostream& operator<<(std::ostream& os, UpdaterScope scope) { + switch (scope) { + case UpdaterScope::kUser: + return os << "User"; + case UpdaterScope::kSystem: + return os << "System"; + } +} + +} // namespace updater + +#endif // CHROME_UPDATER_UPDATER_SCOPE_H_ diff --git a/chrome/updater/service_scope_unittest.cc b/chrome/updater/updater_scope_unittest.cc similarity index 76% rename from chrome/updater/service_scope_unittest.cc rename to chrome/updater/updater_scope_unittest.cc index 38300a203d98d8..cbe3c0a954024b 100644 --- a/chrome/updater/service_scope_unittest.cc +++ b/chrome/updater/updater_scope_unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/updater/service_scope.h" +#include "chrome/updater/updater_scope.h" #include "base/command_line.h" #include "base/test/scoped_command_line.h" @@ -11,14 +11,14 @@ namespace updater { -TEST(ServiceScope, GetProcessScope) { +TEST(UpdaterScope, GetProcessScope) { base::test::ScopedCommandLine original_command_line; { base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); command_line->RemoveSwitch(kSystemSwitch); - DCHECK_EQ(GetProcessScope(), ServiceScope::kUser); + DCHECK_EQ(GetProcessScope(), UpdaterScope::kUser); command_line->AppendSwitch(kSystemSwitch); - DCHECK_EQ(GetProcessScope(), ServiceScope::kSystem); + DCHECK_EQ(GetProcessScope(), UpdaterScope::kSystem); } } diff --git a/chrome/updater/util.cc b/chrome/updater/util.cc index 5a7ffe0211d06a..fbacbaf9606cb8 100644 --- a/chrome/updater/util.cc +++ b/chrome/updater/util.cc @@ -5,15 +5,22 @@ #include "chrome/updater/util.h" #include "base/base_paths.h" +#include "base/command_line.h" #include "base/files/file_util.h" #include "base/logging.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/strings/string_util.h" #include "build/build_config.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/updater_version.h" #include "url/gurl.h" +#if defined(OS_MAC) +#import "chrome/updater/mac/mac_util.h" +#endif + namespace updater { namespace { @@ -78,55 +85,68 @@ std::string EscapeQueryParamValue(base::StringPiece text, bool use_plus) { } // namespace -bool GetBaseDirectory(base::FilePath* path) { - constexpr int kPathKey = +base::Optional GetBaseDirectory() { + UpdaterScope scope = GetProcessScope(); + base::Optional app_data_dir; #if defined(OS_WIN) - base::DIR_LOCAL_APP_DATA; + base::FilePath path; + if (!base::PathService::Get(scope == UpdaterScope::kSystem + ? base::DIR_PROGRAM_FILES + : base::DIR_LOCAL_APP_DATA, + &path)) { + LOG(ERROR) << "Can't retrieve app data directory."; + return base::nullopt; + } + app_data_dir = path; #elif defined(OS_MAC) - base::DIR_APP_DATA; -#endif - - base::FilePath app_data_dir; - if (!base::PathService::Get(kPathKey, &app_data_dir)) { - LOG(ERROR) << "Can't retrieve local app data directory."; - return false; + app_data_dir = GetApplicationSupportDirectory(scope); + if (!app_data_dir) { + LOG(ERROR) << "Can't retrieve app data directory."; + return base::nullopt; } - +#endif const auto product_data_dir = - app_data_dir.AppendASCII(COMPANY_SHORTNAME_STRING) + app_data_dir->AppendASCII(COMPANY_SHORTNAME_STRING) .AppendASCII(PRODUCT_FULLNAME_STRING); if (!base::CreateDirectory(product_data_dir)) { - LOG(ERROR) << "Can't create base directory."; - return false; + LOG(ERROR) << "Can't create base directory: " << product_data_dir; + return base::nullopt; } - - *path = product_data_dir; - return true; + return product_data_dir; } -bool GetVersionedDirectory(base::FilePath* path) { - base::FilePath product_dir; - if (!GetBaseDirectory(&product_dir)) { +base::Optional GetVersionedDirectory() { + base::Optional product_dir = GetBaseDirectory(); + if (!product_dir) { LOG(ERROR) << "Failed to get the base directory."; - return false; + return base::nullopt; } - const auto versioned_dir = product_dir.AppendASCII(UPDATER_VERSION_STRING); + const auto versioned_dir = product_dir->AppendASCII(UPDATER_VERSION_STRING); if (!base::CreateDirectory(versioned_dir)) { LOG(ERROR) << "Can't create versioned directory."; - return false; + return base::nullopt; } - *path = versioned_dir; - return true; + return versioned_dir; +} + +base::CommandLine MakeElevated(base::CommandLine command_line) { +#if defined(OS_MAC) + command_line.PrependWrapper("/usr/bin/sudo"); +#endif + return command_line; } // The log file is created in DIR_LOCAL_APP_DATA or DIR_APP_DATA. void InitLogging(const base::FilePath::StringType& filename) { logging::LoggingSettings settings; - base::FilePath log_dir; - GetBaseDirectory(&log_dir); - const base::FilePath log_file = log_dir.Append(filename); + base::Optional log_dir = GetBaseDirectory(); + if (!log_dir) { + LOG(ERROR) << "Error getting base dir."; + return; + } + const base::FilePath log_file = log_dir->Append(filename); settings.log_file_path = log_file.value().c_str(); settings.logging_dest = logging::LOG_TO_ALL; logging::InitLogging(settings); diff --git a/chrome/updater/util.h b/chrome/updater/util.h index 67ce73c3579247..d0d732f7aa3edb 100644 --- a/chrome/updater/util.h +++ b/chrome/updater/util.h @@ -6,6 +6,7 @@ #define CHROME_UPDATER_UTIL_H_ #include "base/files/file_path.h" +#include "base/optional.h" #include "base/strings/string_piece.h" #include "base/strings/string_util.h" @@ -14,6 +15,8 @@ class GURL; // Externally-defined printers for base types. namespace base { +class CommandLine; + template std::ostream& operator<<(std::ostream& os, const base::Optional& opt) { if (opt.has_value()) { @@ -30,12 +33,12 @@ namespace updater { // Returns the base directory common to all versions of the updater. For // instance, this function may return %localappdata%\Chromium\ChromiumUpdater // for a User install. -bool GetBaseDirectory(base::FilePath* path); +base::Optional GetBaseDirectory(); // Returns a versioned directory under which the running version of the updater // stores its files and data. For instance, this function may return // %localappdata%\Chromium\ChromiumUpdater\1.2.3.4 for a User install. -bool GetVersionedDirectory(base::FilePath* path); +base::Optional GetVersionedDirectory(); // Returns true if the user running the updater also owns the |path|. bool PathOwnedByUser(const base::FilePath& path); @@ -43,6 +46,10 @@ bool PathOwnedByUser(const base::FilePath& path); // Initializes logging for an executable. void InitLogging(const base::FilePath::StringType& filename); +// Wraps the 'command_line' to be executed in an elevated context. +// On macOS this is done with 'sudo'. +base::CommandLine MakeElevated(base::CommandLine command_line); + // Functor used by associative containers of strings as a case-insensitive ASCII // compare. |T| could be std::string or base::string16. template diff --git a/chrome/updater/win/setup/setup.cc b/chrome/updater/win/setup/setup.cc index b5b2629f2504bf..c3700a407c0511 100644 --- a/chrome/updater/win/setup/setup.cc +++ b/chrome/updater/win/setup/setup.cc @@ -17,6 +17,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/strings/strcat.h" #include "base/strings/string16.h" @@ -31,6 +32,7 @@ #include "chrome/updater/app/server/win/updater_legacy_idl.h" #include "chrome/updater/constants.h" #include "chrome/updater/updater_branding.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/updater_version.h" #include "chrome/updater/util.h" #include "chrome/updater/win/constants.h" @@ -99,10 +101,18 @@ std::vector GetSetupFiles(const base::FilePath& source_dir) { } // namespace // TODO(crbug.com/1069976): use specific return values for different code paths. -int Setup(bool is_machine) { - VLOG(1) << __func__ << ", is_machine: " << is_machine; - DCHECK(!is_machine || ::IsUserAnAdmin()); - HKEY key = is_machine ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; +int Setup(UpdaterScope scope) { + VLOG(1) << __func__ << ", scope: " << scope; + DCHECK(scope == UpdaterScope::kUser || ::IsUserAnAdmin()); + HKEY key; + switch (scope) { + case UpdaterScope::kSystem: + key = HKEY_LOCAL_MACHINE; + break; + case UpdaterScope::kUser: + key = HKEY_CURRENT_USER; + break; + } auto scoped_com_initializer = std::make_unique( @@ -113,8 +123,8 @@ int Setup(bool is_machine) { LOG(ERROR) << "GetTempDir failed."; return -1; } - base::FilePath versioned_dir; - if (!GetVersionedDirectory(&versioned_dir)) { + base::Optional versioned_dir = GetVersionedDirectory(); + if (!versioned_dir) { LOG(ERROR) << "GetVersionedDirectory failed."; return -1; } @@ -141,7 +151,7 @@ int Setup(bool is_machine) { // versioned directory, hence the BaseName function call below. std::unique_ptr install_list(WorkItem::CreateWorkItemList()); for (const auto& file : setup_files) { - const base::FilePath target_path = versioned_dir.Append(file.BaseName()); + const base::FilePath target_path = versioned_dir->Append(file.BaseName()); const base::FilePath source_path = source_dir.Append(file); install_list->AddCopyTreeWorkItem(source_path, target_path, temp_dir, WorkItem::ALWAYS); @@ -161,13 +171,14 @@ int Setup(bool is_machine) { static constexpr base::FilePath::StringPieceType kUpdaterExe = FILE_PATH_LITERAL("updater.exe"); - AddComServerWorkItems(key, versioned_dir.Append(kUpdaterExe), + AddComServerWorkItems(key, versioned_dir->Append(kUpdaterExe), install_list.get()); - AddComInterfacesWorkItems(key, versioned_dir.Append(kUpdaterExe), + AddComInterfacesWorkItems(key, versioned_dir->Append(kUpdaterExe), install_list.get()); - base::CommandLine run_updater_wake_command(versioned_dir.Append(kUpdaterExe)); + base::CommandLine run_updater_wake_command( + versioned_dir->Append(kUpdaterExe)); run_updater_wake_command.AppendSwitch(kWakeSwitch); #if !defined(NDEBUG) diff --git a/chrome/updater/win/setup/setup.h b/chrome/updater/win/setup/setup.h index 24f20350dc71d5..d3eed9dd669caf 100644 --- a/chrome/updater/win/setup/setup.h +++ b/chrome/updater/win/setup/setup.h @@ -5,9 +5,11 @@ #ifndef CHROME_UPDATER_WIN_SETUP_SETUP_H_ #define CHROME_UPDATER_WIN_SETUP_SETUP_H_ +#include "chrome/updater/updater_scope.h" + namespace updater { -int Setup(bool is_machine); +int Setup(UpdaterScope scope); } // namespace updater diff --git a/chrome/updater/win/setup/uninstall.cc b/chrome/updater/win/setup/uninstall.cc index b19592a29b7213..fb611209fd943c 100644 --- a/chrome/updater/win/setup/uninstall.cc +++ b/chrome/updater/win/setup/uninstall.cc @@ -12,6 +12,7 @@ #include "base/callback_helpers.h" #include "base/files/file_path.h" #include "base/logging.h" +#include "base/optional.h" #include "base/process/launch.h" #include "base/process/process.h" #include "base/stl_util.h" @@ -26,6 +27,7 @@ #include "chrome/updater/app/server/win/updater_internal_idl.h" #include "chrome/updater/app/server/win/updater_legacy_idl.h" #include "chrome/updater/constants.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/util.h" #include "chrome/updater/win/constants.h" #include "chrome/updater/win/setup/setup_util.h" @@ -75,8 +77,8 @@ void DeleteComInterfaces(HKEY root) { } int RunUninstallScript(bool uninstall_all) { - base::FilePath versioned_dir; - if (!GetVersionedDirectory(&versioned_dir)) { + base::Optional versioned_dir = GetVersionedDirectory(); + if (!versioned_dir) { LOG(ERROR) << "GetVersionedDirectory failed."; return -1; } @@ -87,7 +89,7 @@ int RunUninstallScript(bool uninstall_all) { if (!size || size >= MAX_PATH) return -1; - base::FilePath script_path = versioned_dir.AppendASCII(kUninstallScript); + base::FilePath script_path = versioned_dir->AppendASCII(kUninstallScript); std::wstring cmdline = cmd_path; base::StringAppendF(&cmdline, L" /Q /C \"%ls\" %ls", @@ -114,10 +116,11 @@ int RunUninstallScript(bool uninstall_all) { // 3. Runs the uninstall script in the install directory of the updater. // The execution of this function and the script race each other but the script // loops and waits in between iterations trying to delete the install directory. -int Uninstall(bool is_machine) { - VLOG(1) << __func__ << ", is_machine: " << is_machine; - DCHECK(!is_machine || ::IsUserAnAdmin()); - HKEY key = is_machine ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; +int Uninstall(UpdaterScope scope) { + VLOG(1) << __func__ << ", scope: " << scope; + DCHECK(scope == UpdaterScope::kUser || ::IsUserAnAdmin()); + HKEY key = + scope == UpdaterScope::kSystem ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; auto scoped_com_initializer = std::make_unique( @@ -135,7 +138,7 @@ int Uninstall(bool is_machine) { } DeleteComInterfaces(key); - if (is_machine) + if (scope == UpdaterScope::kSystem) DeleteComService(); DeleteComServer(key); @@ -144,7 +147,7 @@ int Uninstall(bool is_machine) { // Uninstalls this version of the updater, without uninstalling any other // versions. This version is assumed to not be the active version. -int UninstallCandidate(bool is_machine) { +int UninstallCandidate(UpdaterScope scope) { { auto scoped_com_initializer = std::make_unique( diff --git a/chrome/updater/win/setup/uninstall.h b/chrome/updater/win/setup/uninstall.h index b723d64b9bd148..f5affa126b44a7 100644 --- a/chrome/updater/win/setup/uninstall.h +++ b/chrome/updater/win/setup/uninstall.h @@ -5,11 +5,13 @@ #ifndef CHROME_UPDATER_WIN_SETUP_UNINSTALL_H_ #define CHROME_UPDATER_WIN_SETUP_UNINSTALL_H_ +#include "chrome/updater/updater_scope.h" + namespace updater { -int Uninstall(bool is_machine); +int Uninstall(UpdaterScope scope); -int UninstallCandidate(bool is_machine); +int UninstallCandidate(UpdaterScope scope); } // namespace updater diff --git a/chrome/updater/win/task_scheduler_unittest.cc b/chrome/updater/win/task_scheduler_unittest.cc index b94555204fb4be..8a48da793bc743 100644 --- a/chrome/updater/win/task_scheduler_unittest.cc +++ b/chrome/updater/win/task_scheduler_unittest.cc @@ -42,7 +42,7 @@ const wchar_t kTaskDescription1[] = const wchar_t kTaskDescription2[] = L"Task 2 used only for Chrome Updater unit testing."; // A command-line switch used in testing. -const char kTestSwitch[] = "a_switch"; +const char kUnitTestSwitch[] = "a_switch"; class TaskSchedulerTests : public testing::Test { public: @@ -255,7 +255,7 @@ TEST_F(TaskSchedulerTests, GetTaskInfoExecActions) { base::CommandLine command_line2( executable_path.Append(kTestProcessExecutableName)); - command_line2.AppendSwitch(kTestSwitch); + command_line2.AppendSwitch(kUnitTestSwitch); EXPECT_TRUE(task_scheduler_->RegisterTask( kTaskName2, kTaskDescription2, command_line2, TaskScheduler::TRIGGER_TYPE_HOURLY, false)); diff --git a/chrome/updater/win/ui/ui.cc b/chrome/updater/win/ui/ui.cc index 2cdeb36a55c1ed..b895d3cefea674 100644 --- a/chrome/updater/win/ui/ui.cc +++ b/chrome/updater/win/ui/ui.cc @@ -8,6 +8,7 @@ #include #include "base/logging.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/ui/constants.h" #include "chrome/updater/win/ui/util.h" #include "chrome/updater/win/util.h" @@ -61,7 +62,7 @@ OmahaWnd::OmahaWnd(int dialog_id, WTL::CMessageLoop* message_loop, HWND parent) is_complete_(false), is_close_enabled_(true), events_sink_(nullptr), - is_machine_(false) { + scope_(UpdaterScope::kUser) { DCHECK(message_loop); } diff --git a/chrome/updater/win/ui/ui.h b/chrome/updater/win/ui/ui.h index 53b016e907ad0d..112e55c37af2e8 100644 --- a/chrome/updater/win/ui/ui.h +++ b/chrome/updater/win/ui/ui.h @@ -11,6 +11,7 @@ #include "base/threading/thread_checker.h" #include "base/win/atl.h" #include "base/win/scoped_gdi_object.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/ui/owner_draw_controls.h" #include "chrome/updater/win/ui/resources/resources.grh" @@ -49,7 +50,7 @@ class OmahaWnd : public CAxDialogImpl, void SetEventSink(OmahaWndEvents* ev) { events_sink_ = ev; } - void set_is_machine(bool is_machine) { is_machine_ = is_machine; } + void set_scope(UpdaterScope scope) { scope_ = scope; } void set_bundle_name(const base::string16& bundle_name) { bundle_name_ = bundle_name; } @@ -112,7 +113,7 @@ class OmahaWnd : public CAxDialogImpl, WTL::CMessageLoop* message_loop() { return message_loop_; } bool is_complete() { return is_complete_; } bool is_close_enabled() { return is_close_enabled_; } - bool is_machine() { return is_machine_; } + UpdaterScope scope() { return scope_; } const base::string16& bundle_name() { return bundle_name_; } static const ControlAttributes kVisibleTextAttributes; @@ -138,7 +139,7 @@ class OmahaWnd : public CAxDialogImpl, OmahaWndEvents* events_sink_; - bool is_machine_; + UpdaterScope scope_; base::string16 bundle_name_; // Handle to large icon to show when ALT-TAB diff --git a/chrome/updater/win/ui/ui_displayed_event.cc b/chrome/updater/win/ui/ui_displayed_event.cc index 7b02b5d972d39a..a94b476cf04b61 100644 --- a/chrome/updater/win/ui/ui_displayed_event.cc +++ b/chrome/updater/win/ui/ui_displayed_event.cc @@ -7,20 +7,21 @@ #include "base/check.h" #include "base/no_destructor.h" #include "base/stl_util.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/ui/constants.h" #include "chrome/updater/win/util.h" namespace updater { namespace ui { -HRESULT UIDisplayedEventManager::CreateEvent(bool is_machine) { +HRESULT UIDisplayedEventManager::CreateEvent(UpdaterScope scope) { DCHECK(!IsEventHandleInitialized()); return CreateUniqueEventInEnvironment( - kLegacyUiDisplayedEventEnvironmentVariableName, is_machine, + kLegacyUiDisplayedEventEnvironmentVariableName, scope, ScopedKernelHANDLE::Receiver(GetUIDisplayedEvent()).get()); } -HRESULT UIDisplayedEventManager::GetEvent(bool is_machine, +HRESULT UIDisplayedEventManager::GetEvent(UpdaterScope scope, HANDLE* ui_displayed_event) { DCHECK(ui_displayed_event); *ui_displayed_event = nullptr; @@ -30,7 +31,7 @@ HRESULT UIDisplayedEventManager::GetEvent(bool is_machine, } HRESULT hr = OpenUniqueEventFromEnvironment( - kLegacyUiDisplayedEventEnvironmentVariableName, is_machine, + kLegacyUiDisplayedEventEnvironmentVariableName, scope, ScopedKernelHANDLE::Receiver(GetUIDisplayedEvent()).get()); if (FAILED(hr)) return hr; @@ -39,12 +40,12 @@ HRESULT UIDisplayedEventManager::GetEvent(bool is_machine, return S_OK; } -void UIDisplayedEventManager::SignalEvent(bool is_machine) { +void UIDisplayedEventManager::SignalEvent(UpdaterScope scope) { if (!IsEventHandleInitialized()) { HRESULT hr = GetEvent( - is_machine, ScopedKernelHANDLE::Receiver(GetUIDisplayedEvent()).get()); + scope, ScopedKernelHANDLE::Receiver(GetUIDisplayedEvent()).get()); if (HRESULT_FROM_WIN32(ERROR_ENVVAR_NOT_FOUND) == hr) - hr = CreateEvent(is_machine); + hr = CreateEvent(scope); if (FAILED(hr)) { // We may display two UIs in this case. GetUIDisplayedEvent().reset(); diff --git a/chrome/updater/win/ui/ui_displayed_event.h b/chrome/updater/win/ui/ui_displayed_event.h index 4d18be0bb13a51..72be95da2e7748 100644 --- a/chrome/updater/win/ui/ui_displayed_event.h +++ b/chrome/updater/win/ui/ui_displayed_event.h @@ -7,6 +7,7 @@ #include +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/scoped_handle.h" namespace updater { @@ -19,16 +20,16 @@ namespace ui { class UIDisplayedEventManager { public: // Signals the event. Creates the event if the event does not exist. - static void SignalEvent(bool is_machine); + static void SignalEvent(UpdaterScope scope); private: // Creates the event and sets its name in an environment variable. - static HRESULT CreateEvent(bool is_machine); + static HRESULT CreateEvent(UpdaterScope scope); // Gets the event from the name in the environment variable. The caller does // not own the event handle and must not close it. The handle is own by // the process and the handle is closed when the process exits. - static HRESULT GetEvent(bool is_machine, HANDLE* ui_displayed_event); + static HRESULT GetEvent(UpdaterScope scope, HANDLE* ui_displayed_event); // Returns true if the event handle has been initialized in this process. static bool IsEventHandleInitialized(); diff --git a/chrome/updater/win/update_service_internal_proxy.cc b/chrome/updater/win/update_service_internal_proxy.cc index 78ef972360e298..133194dd47add6 100644 --- a/chrome/updater/win/update_service_internal_proxy.cc +++ b/chrome/updater/win/update_service_internal_proxy.cc @@ -16,6 +16,7 @@ #include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/updater/app/server/win/updater_internal_idl.h" +#include "chrome/updater/updater_scope.h" namespace updater { namespace { @@ -109,7 +110,7 @@ void UpdaterInternalCallback::RunOnSTA() { } // namespace -UpdateServiceInternalProxy::UpdateServiceInternalProxy(ServiceScope /*scope*/) +UpdateServiceInternalProxy::UpdateServiceInternalProxy(UpdaterScope /*scope*/) : STA_task_runner_( base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {} diff --git a/chrome/updater/win/update_service_internal_proxy.h b/chrome/updater/win/update_service_internal_proxy.h index e98bfb56c4fc46..ad46eb3c3df177 100644 --- a/chrome/updater/win/update_service_internal_proxy.h +++ b/chrome/updater/win/update_service_internal_proxy.h @@ -8,8 +8,8 @@ #include "base/callback_forward.h" #include "base/memory/scoped_refptr.h" #include "base/sequence_checker.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/update_service_internal.h" +#include "chrome/updater/updater_scope.h" namespace base { class SingleThreadTaskRunner; @@ -20,7 +20,7 @@ namespace updater { // All functions and callbacks must be called on the same sequence. class UpdateServiceInternalProxy : public UpdateServiceInternal { public: - explicit UpdateServiceInternalProxy(ServiceScope scope); + explicit UpdateServiceInternalProxy(UpdaterScope scope); // Overrides for UpdateServiceInternal. void Run(base::OnceClosure callback) override; diff --git a/chrome/updater/win/update_service_proxy.cc b/chrome/updater/win/update_service_proxy.cc index 4cacdd09cfb528..85e6e485915c7d 100644 --- a/chrome/updater/win/update_service_proxy.cc +++ b/chrome/updater/win/update_service_proxy.cc @@ -26,6 +26,7 @@ #include "base/win/scoped_bstr.h" #include "chrome/updater/app/server/win/updater_idl.h" #include "chrome/updater/registration_data.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/util.h" namespace updater { @@ -272,11 +273,11 @@ void UpdaterObserver::OnCompleteOnSTA(const UpdateService::Result& result) { base::BindOnce(std::move(callback_), result)); } -UpdateServiceProxy::UpdateServiceProxy(ServiceScope service_scope) +UpdateServiceProxy::UpdateServiceProxy(UpdaterScope updater_scope) : main_task_runner_(base::SequencedTaskRunnerHandle::Get()), com_task_runner_( base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) { - DCHECK_EQ(service_scope, ServiceScope::kUser); + DCHECK_EQ(updater_scope, UpdaterScope::kUser); } UpdateServiceProxy::~UpdateServiceProxy() = default; diff --git a/chrome/updater/win/update_service_proxy.h b/chrome/updater/win/update_service_proxy.h index 67cf76c56878ff..5b5d92ad94a0de 100644 --- a/chrome/updater/win/update_service_proxy.h +++ b/chrome/updater/win/update_service_proxy.h @@ -11,8 +11,8 @@ #include "base/memory/scoped_refptr.h" #include "base/sequence_checker.h" #include "chrome/updater/app/server/win/updater_idl.h" -#include "chrome/updater/service_scope.h" #include "chrome/updater/update_service.h" +#include "chrome/updater/updater_scope.h" namespace base { class SequencedTaskRunner; @@ -35,7 +35,7 @@ namespace updater { // All public functions and callbacks must be called on the same sequence. class UpdateServiceProxy : public UpdateService { public: - explicit UpdateServiceProxy(ServiceScope service_scope); + explicit UpdateServiceProxy(UpdaterScope updater_scope); // Overrides for updater::UpdateService. void GetVersion( diff --git a/chrome/updater/win/util.cc b/chrome/updater/win/util.cc index 58c36fb527a8c2..29917c07e07846 100644 --- a/chrome/updater/win/util.cc +++ b/chrome/updater/win/util.cc @@ -21,6 +21,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/win/registry.h" #include "chrome/updater/constants.h" +#include "chrome/updater/updater_scope.h" #include "chrome/updater/win/constants.h" #include "chrome/updater/win/user_info.h" @@ -169,13 +170,13 @@ HMODULE GetCurrentModuleHandle() { // The event name saved to the environment variable does not contain the // decoration added by GetNamedObjectAttributes. HRESULT CreateUniqueEventInEnvironment(const std::wstring& var_name, - bool is_machine, + UpdaterScope scope, HANDLE* unique_event) { DCHECK(unique_event); const std::wstring event_name = base::ASCIIToWide(base::GenerateGUID()); NamedObjectAttributes attr; - GetNamedObjectAttributes(event_name.c_str(), is_machine, &attr); + GetNamedObjectAttributes(event_name.c_str(), scope, &attr); HRESULT hr = CreateEvent(&attr, unique_event); if (FAILED(hr)) @@ -190,7 +191,7 @@ HRESULT CreateUniqueEventInEnvironment(const std::wstring& var_name, } HRESULT OpenUniqueEventFromEnvironment(const std::wstring& var_name, - bool is_machine, + UpdaterScope scope, HANDLE* unique_event) { DCHECK(unique_event); @@ -202,7 +203,7 @@ HRESULT OpenUniqueEventFromEnvironment(const std::wstring& var_name, } NamedObjectAttributes attr; - GetNamedObjectAttributes(event_name, is_machine, &attr); + GetNamedObjectAttributes(event_name, scope, &attr); *unique_event = ::OpenEvent(EVENT_ALL_ACCESS, false, attr.name.c_str()); if (!*unique_event) { @@ -231,21 +232,25 @@ HRESULT CreateEvent(NamedObjectAttributes* event_attr, HANDLE* event_handle) { } void GetNamedObjectAttributes(const wchar_t* base_name, - bool is_machine, + UpdaterScope scope, NamedObjectAttributes* attr) { DCHECK(base_name); DCHECK(attr); attr->name = kGlobalPrefix; - if (!is_machine) { - std::wstring user_sid; - GetProcessUser(nullptr, nullptr, &user_sid); - attr->name += user_sid; - GetCurrentUserDefaultSecurityAttributes(&attr->sa); - } else { - // Grant access to administrators and system. - GetAdminDaclSecurityAttributes(&attr->sa, GENERIC_ALL); + switch (scope) { + case UpdaterScope::kUser: { + std::wstring user_sid; + GetProcessUser(nullptr, nullptr, &user_sid); + attr->name += user_sid; + GetCurrentUserDefaultSecurityAttributes(&attr->sa); + break; + } + case UpdaterScope::kSystem: + // Grant access to administrators and system. + GetAdminDaclSecurityAttributes(&attr->sa, GENERIC_ALL); + break; } attr->name += base_name; diff --git a/chrome/updater/win/util.h b/chrome/updater/win/util.h index 933a453be276b1..6cba2bc858ef1b 100644 --- a/chrome/updater/win/util.h +++ b/chrome/updater/win/util.h @@ -15,6 +15,7 @@ #include "base/win/atl.h" #include "base/win/scoped_handle.h" #include "base/win/windows_types.h" +#include "chrome/updater/updater_scope.h" namespace updater { @@ -51,12 +52,12 @@ HMODULE GetCurrentModuleHandle(); // Creates a unique event name and stores it in the specified environment var. HRESULT CreateUniqueEventInEnvironment(const std::wstring& var_name, - bool is_machine, + UpdaterScope scope, HANDLE* unique_event); // Obtains a unique event name from specified environment var and opens it. HRESULT OpenUniqueEventFromEnvironment(const std::wstring& var_name, - bool is_machine, + UpdaterScope scope, HANDLE* unique_event); struct NamedObjectAttributes { @@ -72,7 +73,7 @@ struct NamedObjectAttributes { // both Admins and SYSTEM. This allows for cases where SYSTEM creates the named // object first. The default DACL for SYSTEM will not allow Admins access. void GetNamedObjectAttributes(const wchar_t* base_name, - bool is_machine, + UpdaterScope scope, NamedObjectAttributes* attr); // Creates an event based on the provided attributes.