Reland "Updater: Implement candidate self-uninstall."

This is a reland of 31c7133d

The change from the first patch set is to reinstate the call to Uninstall.

Original change's description:
> Updater: Implement candidate self-uninstall.
>
> Bug: 1098934
> Change-Id: I226fc7201e04dfbf1722399434e3900cedc35120
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487940
> Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
> Reviewed-by: Sorin Jianu <sorin@chromium.org>
> Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#821372}

Bug: 1098934
Change-Id: I5affeb8ef8a2ba447e33a9d4efc1059184fef58c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505459
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821851}

chrome/updater/app/server/win/server.cc

@@ -24,6 +24,7 @@
 #include "chrome/updater/prefs.h"
 #include "chrome/updater/update_service.h"
 #include "chrome/updater/win/constants.h"
+#include "chrome/updater/win/setup/uninstall.h"
 #include "chrome/updater/win/wrl_module.h"
 #include "components/prefs/pref_service.h"
 
@@ -170,7 +171,8 @@ void ComServerApp::ActiveDuty(scoped_refptr<UpdateService> update_service,
 }
 
 void ComServerApp::UninstallSelf() {
-  // TODO(crbug.com/1098934): Uninstall this candidate version of the updater.
+  // TODO(crbug.com/1096654): Add support for is_machine.
+  UninstallCandidate(false);
 }
 
 bool ComServerApp::SwapRPCInterfaces() {

chrome/updater/test/integration_tests.cc

@@ -54,8 +54,6 @@ void PrintLog() {
 
 }  // namespace
 
-#if defined(OS_MAC)
-// TODO(crbug.com/1138014): These functions should be enabled on Win too.
 void RunWake(int expected_exit_code) {
   const base::FilePath installed_executable_path = GetInstalledExecutablePath();
   EXPECT_TRUE(base::PathExists(installed_executable_path));
@@ -103,7 +101,6 @@ void SetupFakeUpdaterLowerVersion() {
 void SetupFakeUpdaterHigherVersion() {
   SetupFakeUpdaterVersion(1);
 }
-#endif  // OS_MAC
 
 bool Run(base::CommandLine command_line, int* exit_code) {
   command_line.AppendSwitch("enable-logging");
@@ -136,8 +133,6 @@ class IntegrationTest : public ::testing::Test {
   }
 
   void TearDown() override {
-    if (::testing::Test::HasFailure())
-      PrintLog();
     ExpectClean();
     if (::testing::Test::HasFailure())
       PrintLog();
@@ -156,7 +151,6 @@ TEST_F(IntegrationTest, InstallUninstall) {
   Uninstall();
 }
 
-#if defined(OS_MAC)
 TEST_F(IntegrationTest, SelfUninstallOutdatedUpdater) {
   Install();
   ExpectInstalled();
@@ -171,9 +165,15 @@ TEST_F(IntegrationTest, SelfUninstallOutdatedUpdater) {
   SleepFor(11);
 
   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");
+
   Uninstall();
+  Clean();
 }
 
+#if defined(OS_MAC)
 TEST_F(IntegrationTest, RegisterTestApp) {
   RegisterTestApp();
   ExpectInstalled();

chrome/updater/test/integration_tests_win.cc

@@ -10,6 +10,7 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
+#include "base/version.h"
 #include "base/win/registry.h"
 #include "chrome/updater/constants.h"
 #include "chrome/updater/test/integration_tests.h"
@@ -38,11 +39,15 @@ base::FilePath GetProductPath() {
       .AppendASCII(UPDATER_VERSION_STRING);
 }
 
-base::FilePath GetExecutablePath() {
+}  // namespace
+
+base::FilePath GetInstalledExecutablePath() {
   return GetProductPath().AppendASCII("updater.exe");
 }
 
-}  // namespace
+base::FilePath GetFakeUpdaterInstallFolderPath(const base::Version& version) {
+  return GetProductPath().AppendASCII(version.GetString());
+}
 
 base::FilePath GetDataDirPath() {
   base::FilePath app_data_dir;
@@ -103,6 +108,14 @@ void ExpectInstalled() {
   EXPECT_TRUE(base::PathExists(GetProductPath()));
 }
 
+void ExpectCandidateUninstalled() {
+  // 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()));
+}
+
 void ExpectActive() {
   // TODO(crbug.com/1062288): Assert that COM interfaces point to this version.
 
@@ -110,16 +123,6 @@ void ExpectActive() {
   EXPECT_TRUE(base::PathExists(GetProductPath()));
 }
 
-void RunWake(int expected_exit_code) {
-  const base::FilePath path = GetExecutablePath();
-  ASSERT_FALSE(path.empty());
-  base::CommandLine command_line(path);
-  command_line.AppendSwitch(kWakeSwitch);
-  int exit_code = -1;
-  ASSERT_TRUE(Run(command_line, &exit_code));
-  EXPECT_EQ(exit_code, expected_exit_code);
-}
-
 void Install() {
   const base::FilePath path = GetInstallerPath();
   ASSERT_FALSE(path.empty());
@@ -131,7 +134,11 @@ void Install() {
 }
 
 void Uninstall() {
-  base::FilePath path = GetExecutablePath();
+  // 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
+  // system even if installation has failed or the installed binaries have
+  // already been removed.
+  base::FilePath path = GetInstallerPath().DirName().AppendASCII("updater.exe");
   ASSERT_FALSE(path.empty());
   base::CommandLine command_line(path);
   command_line.AppendSwitch("uninstall");

chrome/updater/win/setup/uninstall.cc

@@ -30,6 +30,7 @@
 #include "chrome/updater/win/task_scheduler.h"
 
 namespace updater {
+namespace {
 
 void DeleteComServer(HKEY root) {
   for (const auto& clsid : {__uuidof(UpdaterClass), CLSID_UpdaterControlClass,
@@ -63,6 +64,40 @@ void DeleteComInterfaces(HKEY root) {
   }
 }
 
+int RunUninstallScript(bool uninstall_all) {
+  base::FilePath versioned_dir;
+  if (!GetVersionedDirectory(&versioned_dir)) {
+    LOG(ERROR) << "GetVersionedDirectory failed.";
+    return -1;
+  }
+
+  base::char16 cmd_path[MAX_PATH] = {0};
+  DWORD size = ExpandEnvironmentStrings(L"%SystemRoot%\\System32\\cmd.exe",
+                                        cmd_path, base::size(cmd_path));
+  if (!size || size >= MAX_PATH)
+    return -1;
+
+  base::FilePath script_path = versioned_dir.AppendASCII(kUninstallScript);
+
+  base::string16 cmdline = cmd_path;
+  base::StringAppendF(&cmdline, L" /Q /C \"%ls\" %ls",
+                      script_path.value().c_str(),
+                      uninstall_all ? L"all" : L"local");
+  base::LaunchOptions options;
+  options.start_hidden = true;
+
+  VLOG(1) << "Running " << cmdline;
+
+  base::Process process = base::LaunchProcess(cmdline, options);
+  if (!process.IsValid()) {
+    LOG(ERROR) << "Failed to create process " << cmdline;
+    return -1;
+  }
+  return 0;
+}
+
+}  // namespace
+
 // Reverses the changes made by setup. This is a best effort uninstall:
 // 1. Deletes the scheduled task.
 // 2. Deletes the Clients and ClientState keys.
@@ -94,34 +129,22 @@ int Uninstall(bool is_machine) {
     DeleteComService();
   DeleteComServer(key);
 
-  base::FilePath versioned_dir;
-  if (!GetVersionedDirectory(&versioned_dir)) {
-    LOG(ERROR) << "GetVersionedDirectory failed.";
-    return -1;
-  }
-
-  base::char16 cmd_path[MAX_PATH] = {0};
-  auto size = ExpandEnvironmentStrings(L"%SystemRoot%\\System32\\cmd.exe",
-                                       cmd_path, base::size(cmd_path));
-  if (!size || size >= MAX_PATH)
-    return -1;
-
-  base::FilePath script_path = versioned_dir.AppendASCII(kUninstallScript);
-
-  base::string16 cmdline = cmd_path;
-  base::StringAppendF(&cmdline, L" /Q /C \"%ls\"", script_path.value().c_str());
-  base::LaunchOptions options;
-  options.start_hidden = true;
-
-  VLOG(1) << "Running " << cmdline;
+  return RunUninstallScript(true);
+}
 
-  auto process = base::LaunchProcess(cmdline, options);
-  if (!process.IsValid()) {
-    LOG(ERROR) << "Failed to create process " << cmdline;
-    return -1;
+// 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) {
+  {
+    auto scoped_com_initializer =
+        std::make_unique<base::win::ScopedCOMInitializer>(
+            base::win::ScopedCOMInitializer::kMTA);
+    updater::UnregisterWakeTask();
   }
 
-  return 0;
+  // TODO(crbug.com/1140562): Remove the ControlService server as well.
+
+  return RunUninstallScript(false);
 }
 
 }  // namespace updater

chrome/updater/win/setup/uninstall.cmd

@@ -4,7 +4,9 @@ rem directory path.  Sleeps 3 seconds and tries 3 times to delete the
 rem directory.
 @echo off
 set Directory=%~dp0
-FOR %%a IN ("%Directory:~0,-1%") DO set Directory=%%~dpa
+IF "%1%" EQU "all" (
+  FOR %%a IN ("%Directory:~0,-1%") DO set Directory=%%~dpa
+)
 @echo %Directory% | FindStr /R \\AppData\\Local\\@COMPANY_SHORTNAME@\\@PRODUCT_FULLNAME@\\ > nul
 IF %ERRORLEVEL% NEQ 0 exit 1
 @echo Deleting "%Directory%"...

chrome/updater/win/setup/uninstall.h

@@ -9,6 +9,8 @@ namespace updater {
 
 int Uninstall(bool is_machine);
 
+int UninstallCandidate(bool is_machine);
+
 }  // namespace updater
 
 #endif  // CHROME_UPDATER_WIN_SETUP_UNINSTALL_H_