Updater: Delete the launchd services in integration test's Clean().

This should reduce flakiness of the updater_tests's IntegrationTest.* suite.
Also re-enables one such disabled test, which should now be more reliable.

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

chrome/updater/app/app_install_mac.mm

@@ -14,7 +14,8 @@ namespace updater {
 
 void AppInstall::WakeCandidateDone() {
   PollLaunchctlList(
-      kUpdateServiceLaunchdName, base::TimeDelta::FromSeconds(5),
+      kUpdateServiceLaunchdName, LaunchctlPresence::kPresent,
+      base::TimeDelta::FromSeconds(5),
       base::BindOnce([](scoped_refptr<AppInstall> installer,
                         bool unused) { installer->RegisterUpdater(); },
                      base::WrapRefCounted(this)));

chrome/updater/launchd_util.cc

@@ -21,6 +21,7 @@ namespace updater {
 namespace {
 
 void PollLaunchctlListImpl(const std::string& service,
+                           LaunchctlPresence expectation,
                            base::TimeTicks deadline,
                            base::OnceCallback<void(bool)> callback) {
   if (base::TimeTicks::Now() > deadline) {
@@ -42,13 +43,14 @@ void PollLaunchctlListImpl(const std::string& service,
     std::move(callback).Run(false);
     return;
   }
-  if (exit_code == 0) {
+  if ((exit_code == 0 && expectation == LaunchctlPresence::kPresent) ||
+      (exit_code != 0 && expectation == LaunchctlPresence::kAbsent)) {
     std::move(callback).Run(true);
     return;
   }
   base::ThreadPool::PostDelayedTask(
       FROM_HERE, {base::WithBaseSyncPrimitives()},
-      base::BindOnce(&PollLaunchctlListImpl, service, deadline,
+      base::BindOnce(&PollLaunchctlListImpl, service, expectation, deadline,
                      std::move(callback)),
       base::TimeDelta::FromMilliseconds(500));
 }
@@ -56,12 +58,14 @@ void PollLaunchctlListImpl(const std::string& service,
 }  // namespace
 
 void PollLaunchctlList(const std::string& service,
+                       LaunchctlPresence expectation,
                        base::TimeDelta timeout,
                        base::OnceCallback<void(bool)> callback) {
   base::ThreadPool::PostTask(
       FROM_HERE, {base::WithBaseSyncPrimitives()},
       base::BindOnce(
-          &PollLaunchctlListImpl, service, base::TimeTicks::Now() + timeout,
+          &PollLaunchctlListImpl, service, expectation,
+          base::TimeTicks::Now() + timeout,
           base::BindOnce(
               [](scoped_refptr<base::TaskRunner> runner,
                  base::OnceCallback<void(bool)> callback, bool result) {

chrome/updater/launchd_util.h

@@ -15,10 +15,18 @@ class TimeDelta;
 
 namespace updater {
 
-// Query `launchctl list |service|` with retries, calling |callback| with
-// 'true' if launchctl has the service, and false if |timeout| is reached.
-// Must be called in a sequence. The callback is posted to the same sequence.
+enum class LaunchctlPresence {
+  kAbsent,
+  kPresent,
+};
+
+// Query `launchctl list |service|` with retries, until either the
+// `expectation` about the presence of `service` is met, or `timeout` is
+// 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,
+                       LaunchctlPresence expectation,
                        base::TimeDelta timeout,
                        base::OnceCallback<void(bool)> callback);
 

chrome/updater/setup_mac.mm

@@ -23,7 +23,8 @@ void SetupDone(base::OnceCallback<void(int)> callback, int result) {
     return;
   }
   PollLaunchctlList(
-      kUpdateServiceInternalLaunchdName, base::TimeDelta::FromSeconds(3),
+      kUpdateServiceInternalLaunchdName, LaunchctlPresence::kPresent,
+      base::TimeDelta::FromSeconds(3),
       base::BindOnce(
           [](base::OnceCallback<void(int)> callback, bool service_exists) {
             std::move(callback).Run(

chrome/updater/test/integration_tests.cc

@@ -183,7 +183,7 @@ TEST_F(IntegrationTest, RegisterTestApp) {
   Uninstall();
 }
 
-TEST_F(IntegrationTest, DISABLED_UnregisterUninstalledApp) {
+TEST_F(IntegrationTest, UnregisterUninstalledApp) {
   RegisterTestApp();
   ExpectInstalled();
   ExpectActiveVersion(UPDATER_VERSION_STRING);

chrome/updater/test/integration_tests_mac.mm

@@ -9,9 +9,13 @@
 #include "base/files/file_util.h"
 #include "base/mac/foundation_util.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"
+#include "base/strings/sys_string_conversions.h"
+#include "base/test/bind.h"
 #include "base/version.h"
 #include "chrome/common/mac/launchd.h"
 #include "chrome/updater/constants.h"
+#include "chrome/updater/launchd_util.h"
 #import "chrome/updater/mac/util.h"
 #include "chrome/updater/mac/xpc_service_names.h"
 #include "chrome/updater/prefs.h"
@@ -30,6 +34,17 @@ namespace test {
 
 namespace {
 
+void RemoveJobFromLaunchd(Launchd::Domain domain,
+                          Launchd::Type type,
+                          base::ScopedCFTypeRef<CFStringRef> name) {
+  EXPECT_TRUE(Launchd::GetInstance()->DeletePlist(domain, type, name))
+      << "Failed to delete plist for " << name;
+
+  // Return value is ignored, since RemoveJob returns false if the job already
+  // doesn't exist.
+  Launchd::GetInstance()->RemoveJob(base::SysCFStringRefToUTF8(name));
+}
+
 base::FilePath GetExecutablePath() {
   base::FilePath test_executable;
   if (!base::PathService::Get(base::FILE_EXE, &test_executable))
@@ -58,6 +73,19 @@ base::FilePath GetProductPath() {
       .AppendASCII(PRODUCT_FULLNAME_STRING);
 }
 
+void ExpectServiceAbsent(const std::string& service) {
+  bool success = false;
+  base::RunLoop loop;
+  PollLaunchctlList(service, LaunchctlPresence::kAbsent,
+                    base::TimeDelta::FromSeconds(7),
+                    base::BindLambdaForTesting([&](bool result) {
+                      success = result;
+                      loop.QuitClosure().Run();
+                    }));
+  loop.Run();
+  EXPECT_TRUE(success) << service << " is unexpectedly present.";
+}
+
 }  // namespace
 
 base::FilePath GetDataDirPath() {
@@ -85,6 +113,13 @@ void Clean() {
     [userDefaults
         removeObjectForKey:[NSString
                                stringWithUTF8String:kDevOverrideKeyUseCUP]];
+
+    // TODO(crbug.com/1096654): support machine case (Launchd::Domain::Local and
+    // Launchd::Type::Daemon).
+    RemoveJobFromLaunchd(Launchd::Domain::User, Launchd::Type::Agent,
+                         CopyUpdateServiceLaunchdName());
+    RemoveJobFromLaunchd(Launchd::Domain::User, Launchd::Type::Agent,
+                         CopyUpdateServiceInternalLaunchdName());
   }
 }
 
@@ -99,6 +134,8 @@ void ExpectClean() {
   EXPECT_FALSE(Launchd::GetInstance()->PlistExists(
       Launchd::User, Launchd::Agent, updater::CopyUpdateServiceLaunchdName()));
   EXPECT_FALSE(base::PathExists(GetDataDirPath()));
+  ExpectServiceAbsent(kUpdateServiceLaunchdName);
+  ExpectServiceAbsent(kUpdateServiceInternalLaunchdName);
 }
 
 void EnterTestMode() {