Updater: Do not updatecheck in ControlService wake during --install

Bug: 1128397
Change-Id: Ifdaf5d6e5734a53f83ef73758df9cb4eb0a7e262
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414559
Commit-Queue: Mila Green <milagreen@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808564}

chrome/updater/app/app_install.cc

@@ -113,12 +113,20 @@ void AppInstall::InstallCandidateDone(int result) {
     return;
   }
 
-  // Invoke ControlService::Run to wake this version of the updater, do an
-  // update check, and possibly promote this version as a result.
-  // The instance of |CreateControlService| has sequence affinity. Bind it
-  // in the closure to ensure it is released in this sequence.
+  // Invoke ControlService::InitializeUpdateService to wake this version of the
+  // updater, qualify, and possibly promote this version as a result. The
+  // instance of |CreateControlService| has sequence affinity. Bind it in the
+  // closure to ensure it is released in this sequence.
   scoped_refptr<ControlService> control_service = CreateControlService();
-  control_service->Run(base::BindOnce(
+  control_service->
+#if defined(OS_MAC)
+      InitializeUpdateService
+#else
+      // TODO(crbug.com/1128397): As substitute the call to Run with a call to
+      // InitializeUpdateService on Win.
+      Run
+#endif
+      (base::BindOnce(
           [](scoped_refptr<ControlService> /*control_service*/,
              scoped_refptr<AppInstall> app_install) {
             app_install->RegisterUpdater();

chrome/updater/app/app_register.cc

@@ -159,7 +159,7 @@ void AppRegister::WakeCandidate() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   ResetControlService();
   control_service_ = CreateControlService();
-  control_service_->Run(
+  control_service_->InitializeUpdateService(
       base::BindOnce(&AppRegister::ControlServiceRunDone, this, 0));
 }
 

chrome/updater/app/server/mac/service_delegate.mm

@@ -236,6 +236,21 @@
       base::BindOnce(&updater::ControlService::Run, _service, std::move(cb)));
 }
 
+- (void)performInitializeUpdateServiceWithReply:(void (^)(void))reply {
+  auto cb = base::BindOnce(base::RetainBlock(^(void) {
+    if (reply)
+      reply();
+
+    _appServer->TaskCompleted();
+  }));
+
+  _appServer->TaskStarted();
+  _callbackRunner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&updater::ControlService::InitializeUpdateService,
+                     _service, std::move(cb)));
+}
+
 @end
 
 @implementation CRUUpdateCheckServiceXPCDelegate {

chrome/updater/app/server/mac/service_protocol.h

@@ -54,6 +54,10 @@
 // that is relevant to the state of the Updater.
 - (void)performControlTasksWithReply:(void (^_Nullable)(void))reply;
 
+// Performs the control task that is relevant to the state of the Updater.
+// Does not perform an UpdateCheck.
+- (void)performInitializeUpdateServiceWithReply:(void (^_Nullable)(void))reply;
+
 @end
 
 namespace updater {

chrome/updater/control_service.h

@@ -14,8 +14,17 @@ namespace updater {
 // tasks on the updater.
 class ControlService : public base::RefCountedThreadSafe<ControlService> {
  public:
+  // Runs the ControlService and checks for updates if needed.
   virtual void Run(base::OnceClosure callback) = 0;
 
+  // When ControlServiceOutOfProcess::InitializeUpdateService is invoked, the
+  // server will wake and do its ModeCheck. As a result, the candidate can be
+  // qualified and promoted (thus initializing the UpdateService for this
+  // candidate). This is intended as a way for --install and --register to have
+  // a way of ensuring there is an active updater on the system, without
+  // performing expensive operations such as checking for updates.
+  virtual void InitializeUpdateService(base::OnceClosure callback) = 0;
+
   // Provides a way to commit data or clean up resources before the task
   // scheduler is shutting down.
   virtual void Uninitialize() = 0;

chrome/updater/control_service_in_process.cc

@@ -37,6 +37,13 @@ void ControlServiceInProcess::Run(base::OnceClosure callback) {
   MaybeCheckForUpdates(std::move(callback));
 }
 
+void ControlServiceInProcess::InitializeUpdateService(
+    base::OnceClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  VLOG(1) << __func__;
+  std::move(callback).Run();
+}
+
 void ControlServiceInProcess::MaybeCheckForUpdates(base::OnceClosure callback) {
   const base::Time lastUpdateTime =
       config_->GetPrefService()->GetTime(kPrefUpdateTime);

chrome/updater/control_service_in_process.h

@@ -26,6 +26,7 @@ class ControlServiceInProcess : public ControlService {
 
   // Overrides for updater::ControlService.
   void Run(base::OnceClosure callback) override;
+  void InitializeUpdateService(base::OnceClosure callback) override;
 
   void Uninitialize() override;
 

chrome/updater/mac/control_service_out_of_process.h

@@ -29,6 +29,7 @@ class ControlServiceOutOfProcess : public ControlService {
 
   // Overrides for ControlService.
   void Run(base::OnceClosure callback) override;
+  void InitializeUpdateService(base::OnceClosure callback) override;
   void Uninitialize() override;
 
  private:

chrome/updater/mac/control_service_out_of_process.mm

@@ -73,6 +73,17 @@
       performControlTasksWithReply:reply];
 }
 
+- (void)performInitializeUpdateServiceWithReply:(void (^)(void))reply {
+  auto errorHandler = ^(NSError* xpcError) {
+    LOG(ERROR) << "XPC connection failed: "
+               << base::SysNSStringToUTF8([xpcError description]);
+    reply();
+  };
+
+  [[_controlXPCConnection remoteObjectProxyWithErrorHandler:errorHandler]
+      performInitializeUpdateServiceWithReply:reply];
+}
+
 @end
 
 namespace updater {
@@ -101,6 +112,19 @@ void ControlServiceOutOfProcess::Run(base::OnceClosure callback) {
   [client_ performControlTasksWithReply:reply];
 }
 
+void ControlServiceOutOfProcess::InitializeUpdateService(
+    base::OnceClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  __block base::OnceClosure block_callback = std::move(callback);
+  auto reply = ^() {
+    callback_runner_->PostTask(FROM_HERE,
+                               base::BindOnce(std::move(block_callback)));
+  };
+
+  [client_ performInitializeUpdateServiceWithReply:reply];
+}
+
 void ControlServiceOutOfProcess::Uninitialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }

chrome/updater/mac/update_service_out_of_process_test.mm

@@ -280,7 +280,7 @@ class MacUpdateServiceOutOfProcessTest : public ::testing::Test {
 
   // Create an UpdateServiceOutOfProcess and store in service_. Must be
   // called only on the task_environment_ sequence. SetUp() posts it.
-  void InitializeService();
+  void InitializeUpdateService();
 
   base::test::SingleThreadTaskEnvironment task_environment_{
       base::test::TaskEnvironment::ThreadPoolExecutionMode::QUEUED};

chrome/updater/win/control_service_out_of_process.cc

@@ -158,6 +158,13 @@ void ControlServiceOutOfProcess::Run(base::OnceClosure callback) {
               base::SequencedTaskRunnerHandle::Get(), std::move(callback))));
 }
 
+void ControlServiceOutOfProcess::InitializeUpdateService(
+    base::OnceClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // TODO(crbug.com/1128397): Implement.
+}
+
 void ControlServiceOutOfProcess::RunOnSTA(base::OnceClosure callback) {
   DCHECK(com_task_runner_->BelongsToCurrentThread());
 

chrome/updater/win/control_service_out_of_process.h

@@ -24,6 +24,7 @@ class ControlServiceOutOfProcess : public ControlService {
 
   // Overrides for ControlService.
   void Run(base::OnceClosure callback) override;
+  void InitializeUpdateService(base::OnceClosure callback) override;
   void Uninitialize() override;
 
  private: