Make updater::UpdateService ref-counted.

Bug: 1064869

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

chrome/updater/app/app_update_all.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/updater/app/app_update_all.h"
 
-#include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ -29,7 +28,7 @@ class AppUpdateAll : public App {
   void Uninitialize() override;
 
   scoped_refptr<Configurator> config_;
-  std::unique_ptr<UpdateService> update_service_;
+  scoped_refptr<UpdateService> update_service_;
 };
 
 void AppUpdateAll::Initialize() {

chrome/updater/mac/update_service_out_of_process.h

@@ -7,14 +7,21 @@
 
 #import <Foundation/Foundation.h>
 
+#include <string>
+
 #include "base/callback_forward.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/updater/update_service.h"
 
 @class CRUUpdateServiceOutOfProcessImpl;
 
+namespace base {
+class SequencedTaskRunner;
+}  // namespace base
+
 namespace update_client {
 enum class Error;
 }  // namespace update_client
@@ -22,25 +29,20 @@ enum class Error;
 namespace updater {
 
 using StateChangeCallback =
-    base::RepeatingCallback<void(updater::UpdateService::UpdateState)>;
+    base::RepeatingCallback<void(UpdateService::UpdateState)>;
 
 // All functions and callbacks must be called on the same sequence.
 class UpdateServiceOutOfProcess : public UpdateService {
  public:
   UpdateServiceOutOfProcess();
 
-  UpdateServiceOutOfProcess(const UpdateServiceOutOfProcess&) = delete;
-  UpdateServiceOutOfProcess& operator=(const UpdateServiceOutOfProcess&) =
-      delete;
-
-  ~UpdateServiceOutOfProcess() override;
-
-  // Overrides for updater::UpdateService.
-  // Update-checks all registered applications. Calls |callback| once the
-  // operation is complete.
+  // Overrides for UpdateService.
   void RegisterApp(
       const RegistrationRequest& request,
       base::OnceCallback<void(const RegistrationResponse&)> callback) override;
+
+  // Checks all registered applications for updates. Calls |callback| once the
+  // operation is complete.
   void UpdateAll(base::OnceCallback<void(Result)> callback) override;
   void Update(const std::string& app_id,
               Priority priority,
@@ -49,6 +51,8 @@ class UpdateServiceOutOfProcess : public UpdateService {
   void Uninitialize() override;
 
  private:
+  ~UpdateServiceOutOfProcess() override;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::scoped_nsobject<CRUUpdateServiceOutOfProcessImpl> client_;

chrome/updater/server/mac/server.mm

@@ -8,16 +8,12 @@
 #include <xpc/xpc.h>
 
 #include "base/mac/scoped_nsobject.h"
-#include "base/strings/sys_string_conversions.h"
-#include "base/task/task_traits.h"
-#include "base/task/thread_pool.h"
+#include "base/memory/ref_counted.h"
 #include "chrome/updater/app/app.h"
 #import "chrome/updater/configurator.h"
 #import "chrome/updater/mac/xpc_service_names.h"
 #include "chrome/updater/server/mac/service_delegate.h"
-#import "chrome/updater/update_service.h"
 #include "chrome/updater/update_service_in_process.h"
-#include "chrome/updater/updater_version.h"
 
 namespace updater {
 
@@ -45,7 +41,7 @@ void AppServer::Initialize() {
 void AppServer::FirstTaskRun() {
   @autoreleasepool {
     delegate_.reset([[CRUUpdateCheckXPCServiceDelegate alloc]
-        initWithUpdateService:std::make_unique<UpdateServiceInProcess>(
+        initWithUpdateService:base::MakeRefCounted<UpdateServiceInProcess>(
                                   config_)]);
 
     listener_.reset([[NSXPCListener alloc]

chrome/updater/server/mac/service_delegate.h

@@ -7,7 +7,7 @@
 
 #import <Foundation/Foundation.h>
 
-#include <memory>
+#include "base/memory/scoped_refptr.h"
 
 namespace updater {
 class UpdateService;
@@ -17,7 +17,7 @@ class UpdateService;
 
 // Designated initializer.
 - (instancetype)initWithUpdateService:
-    (std::unique_ptr<updater::UpdateService>)service NS_DESIGNATED_INITIALIZER;
+    (scoped_refptr<updater::UpdateService>)service NS_DESIGNATED_INITIALIZER;
 
 @end
 

chrome/updater/server/mac/service_delegate.mm

@@ -93,13 +93,13 @@
 @end
 
 @implementation CRUUpdateCheckXPCServiceDelegate {
-  std::unique_ptr<updater::UpdateService> _service;
+  scoped_refptr<updater::UpdateService> _service;
 }
 
 - (instancetype)initWithUpdateService:
-    (std::unique_ptr<updater::UpdateService>)service {
+    (scoped_refptr<updater::UpdateService>)service {
   if (self = [super init]) {
-    _service = std::move(service);
+    _service = service;
   }
   return self;
 }

chrome/updater/server/win/server.cc

@@ -16,9 +16,9 @@
 #include <wrl/module.h>
 
 #include <algorithm>
-#include <memory>
 
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/stl_util.h"
 #include "base/system/sys_info.h"
 #include "base/task/thread_pool/thread_pool_instance.h"
@@ -68,7 +68,7 @@ class ComServer : public App {
   base::win::ScopedCOMInitializer com_initializer_;
 
   // The UpdateService to use for handling COM requests.
-  std::unique_ptr<UpdateService> service_;
+  scoped_refptr<UpdateService> service_;
 
   // The updater's Configurator.
   scoped_refptr<Configurator> config_;
@@ -198,7 +198,7 @@ void ComServer::FirstTaskRun() {
     Shutdown(-1);
     return;
   }
-  service_ = std::make_unique<UpdateServiceInProcess>(config_);
+  service_ = base::MakeRefCounted<UpdateServiceInProcess>(config_);
   CreateWRLModule();
   HRESULT hr = RegisterClassObject();
   if (FAILED(hr))

chrome/updater/update_apps.h

@@ -5,8 +5,6 @@
 #ifndef CHROME_UPDATER_UPDATE_APPS_H_
 #define CHROME_UPDATER_UPDATE_APPS_H_
 
-#include <memory>
-
 #include "base/memory/scoped_refptr.h"
 
 namespace update_client {
@@ -14,10 +12,11 @@ class Configurator;
 }  // namespace update_client
 
 namespace updater {
+
 class UpdateService;
 
 // A factory method to create an UpdateService class instance.
-std::unique_ptr<UpdateService> CreateUpdateService(
+scoped_refptr<UpdateService> CreateUpdateService(
     scoped_refptr<update_client::Configurator> config);
 
 }  // namespace updater

chrome/updater/update_apps_mac.mm

@@ -4,14 +4,15 @@
 
 #include "chrome/updater/update_apps.h"
 
+#include "base/memory/ref_counted.h"
 #include "chrome/updater/configurator.h"
 #include "chrome/updater/mac/update_service_out_of_process.h"
 
 namespace updater {
 
-std::unique_ptr<UpdateService> CreateUpdateService(
+scoped_refptr<UpdateService> CreateUpdateService(
     scoped_refptr<update_client::Configurator> config) {
-  return std::make_unique<UpdateServiceOutOfProcess>();
+  return base::MakeRefCounted<UpdateServiceOutOfProcess>();
 }
 
 }  // namespace updater

chrome/updater/update_apps_win.cc

@@ -5,6 +5,7 @@
 #include "chrome/updater/update_apps.h"
 
 #include "base/command_line.h"
+#include "base/memory/ref_counted.h"
 #include "chrome/updater/configurator.h"
 #include "chrome/updater/constants.h"
 #include "chrome/updater/update_service_in_process.h"
@@ -12,12 +13,12 @@
 
 namespace updater {
 
-std::unique_ptr<UpdateService> CreateUpdateService(
+scoped_refptr<UpdateService> CreateUpdateService(
     scoped_refptr<update_client::Configurator> config) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSingleProcessSwitch))
-    return std::make_unique<UpdateServiceInProcess>(config);
+    return base::MakeRefCounted<UpdateServiceInProcess>(config);
   else
-    return UpdateServiceOutOfProcess::CreateInstance();
+    return base::MakeRefCounted<UpdateServiceOutOfProcess>();
 }
 
 }  // namespace updater

chrome/updater/update_service.h

@@ -8,18 +8,20 @@
 #include <string>
 
 #include "base/callback_forward.h"
+#include "base/memory/ref_counted.h"
 
 namespace update_client {
 enum class Error;
 }  // namespace update_client
 
 namespace updater {
+
 struct RegistrationRequest;
 struct RegistrationResponse;
 
 // The UpdateService is the cross-platform core of the updater.
 // All functions and callbacks must be called on the same sequence.
-class UpdateService {
+class UpdateService : public base::RefCountedThreadSafe<UpdateService> {
  public:
   using Result = update_client::Error;
 
@@ -67,11 +69,6 @@ class UpdateService {
     kForeground = 2,
   };
 
-  UpdateService(const UpdateService&) = delete;
-  UpdateService& operator=(const UpdateService&) = delete;
-
-  virtual ~UpdateService() = default;
-
   // Registers given request to the updater.
   virtual void RegisterApp(
       const RegistrationRequest& request,
@@ -110,7 +107,9 @@ class UpdateService {
   virtual void Uninitialize() = 0;
 
  protected:
-  UpdateService() = default;
+  friend class base::RefCountedThreadSafe<UpdateService>;
+
+  virtual ~UpdateService() = default;
 };
 
 }  // namespace updater

chrome/updater/update_service_in_process.h

@@ -33,11 +33,6 @@ class UpdateServiceInProcess : public UpdateService {
   explicit UpdateServiceInProcess(
       scoped_refptr<update_client::Configurator> config);
 
-  UpdateServiceInProcess(const UpdateServiceInProcess&) = delete;
-  UpdateServiceInProcess& operator=(const UpdateServiceInProcess&) = delete;
-
-  ~UpdateServiceInProcess() override;
-
   // Overrides for updater::UpdateService.
   // Registers given request to the updater.
   void RegisterApp(
@@ -58,6 +53,8 @@ class UpdateServiceInProcess : public UpdateService {
   void Uninitialize() override;
 
  private:
+  ~UpdateServiceInProcess() override;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   scoped_refptr<update_client::Configurator> config_;

chrome/updater/win/update_service_out_of_process.cc

@@ -49,7 +49,11 @@ HRESULT UpdaterObserverImpl::OnComplete(ICompleteStatus* status) {
   return S_OK;
 }
 
-UpdateServiceOutOfProcess::UpdateServiceOutOfProcess() = default;
+UpdateServiceOutOfProcess::UpdateServiceOutOfProcess() {
+  Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
+      &UpdateServiceOutOfProcess::ModuleStop);
+  com_task_runner_ = base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits);
+}
 
 UpdateServiceOutOfProcess::~UpdateServiceOutOfProcess() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -59,20 +63,6 @@ void UpdateServiceOutOfProcess::ModuleStop() {
   VLOG(2) << "UpdateServiceOutOfProcess::ModuleStop";
 }
 
-std::unique_ptr<UpdateServiceOutOfProcess>
-UpdateServiceOutOfProcess::CreateInstance() {
-  Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
-      &UpdateServiceOutOfProcess::ModuleStop);
-
-  struct Creator : public UpdateServiceOutOfProcess {};
-  auto instance = std::make_unique<Creator>();
-  instance->com_task_runner_ =
-      base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits);
-  if (!instance->com_task_runner_)
-    return nullptr;
-  return instance;
-}
-
 void UpdateServiceOutOfProcess::RegisterApp(
     const RegistrationRequest& request,
     base::OnceCallback<void(const RegistrationResponse&)> callback) {

chrome/updater/win/update_service_out_of_process.h

@@ -7,7 +7,6 @@
 
 #include <wrl/implements.h>
 
-#include <memory>
 #include <string>
 
 #include "base/callback_forward.h"
@@ -50,13 +49,7 @@ using StateChangeCallback =
 // All functions and callbacks must be called on the same sequence.
 class UpdateServiceOutOfProcess : public UpdateService {
  public:
-  UpdateServiceOutOfProcess(const UpdateServiceOutOfProcess&) = delete;
-  UpdateServiceOutOfProcess& operator=(const UpdateServiceOutOfProcess&) =
-      delete;
-  ~UpdateServiceOutOfProcess() override;
-
-  static std::unique_ptr<UpdateServiceOutOfProcess> CreateInstance();
-  static void ModuleStop();
+  UpdateServiceOutOfProcess();
 
   // Overrides for updater::UpdateService.
   // Update-checks all registered applications. Calls |callback| once the
@@ -71,8 +64,11 @@ class UpdateServiceOutOfProcess : public UpdateService {
               base::OnceCallback<void(Result)> done) override;
   void Uninitialize() override;
 
+  static void ModuleStop();
+
  private:
-  UpdateServiceOutOfProcess();
+  ~UpdateServiceOutOfProcess() override;
+
   void UpdateAllOnSTA(base::OnceCallback<void(Result)> callback);
 
   SEQUENCE_CHECKER(sequence_checker_);