Initialize the WRL::Module only once.

The issue with the crashing code is that on the RPC client side
of the updater, both update and control services must be instantiated.
They both called WRL::Module::Create, which caused WRL code to assert.

The server side of RPC also initializes the module, in a slightly
different way. Therefore, module initialization in the App base
class is not practical. It has to be done in a way specific to
the client, or the server, or the mode App is instantiated as.

Using a leaky singleton is not great. However, there is singleton
already in the WRL::Module, so a leaking singleton in the updater
just shows that fact.

Bug: 1128748
Change-Id: I646c5ef779c4affe7f2fe8f751d1dea1f16f8608
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414847Reviewed-by: S. Ganesh <ganesh@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807682}

chrome/updater/service_factory_win.cc

@@ -2,18 +2,42 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <wrl/module.h>
+
+#include "base/logging.h"
 #include "base/memory/ref_counted.h"
+#include "base/no_destructor.h"
 #include "chrome/updater/service_scope.h"
 #include "chrome/updater/win/control_service_out_of_process.h"
 #include "chrome/updater/win/update_service_out_of_process.h"
 
 namespace updater {
+namespace {
+
+// Allows one time creation of the WRL::Module instance. The WRL library
+// contains a global instance of a class, which must be created only once.
+class WRLModuleInitializer {
+ public:
+  WRLModuleInitializer() {
+    Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
+        []() { DVLOG(2) << "COM client is shutting down."; });
+  }
+
+  static const WRLModuleInitializer& Get() {
+    static const base::NoDestructor<WRLModuleInitializer> module;
+    return *module;
+  }
+};
+
+}  // namespace
 
 scoped_refptr<UpdateService> CreateUpdateService() {
+  WRLModuleInitializer::Get();
   return base::MakeRefCounted<UpdateServiceOutOfProcess>(GetProcessScope());
 }
 
 scoped_refptr<ControlService> CreateControlService() {
+  WRLModuleInitializer::Get();
   return base::MakeRefCounted<ControlServiceOutOfProcess>(GetProcessScope());
 }
 

chrome/updater/win/BUILD.gn

@@ -111,6 +111,7 @@ source_set("lib") {
   defines = [ "SECURITY_WIN32" ]
 
   libs = [
+    "RuntimeObject.lib",
     "secur32.lib",
     "taskschd.lib",
     "winhttp.lib",

chrome/updater/win/control_service_out_of_process.cc

@@ -130,19 +130,12 @@ void UpdaterControlObserver::OnCompleteOnSTA() {
 
 ControlServiceOutOfProcess::ControlServiceOutOfProcess(ServiceScope /*scope*/)
     : com_task_runner_(
-          base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {
-  Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
-      &ControlServiceOutOfProcess::ModuleStop);
-}
+          base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {}
 
 ControlServiceOutOfProcess::~ControlServiceOutOfProcess() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-void ControlServiceOutOfProcess::ModuleStop() {
-  DVLOG(2) << __func__ << ": COM client is shutting down.";
-}
-
 void ControlServiceOutOfProcess::Uninitialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }

chrome/updater/win/control_service_out_of_process.h

@@ -32,8 +32,6 @@ class ControlServiceOutOfProcess : public ControlService {
   // Runs on the |com_task_runner_|.
   void RunOnSTA(base::OnceClosure callback);
 
-  static void ModuleStop();
-
   // Bound to the main sequence.
   SEQUENCE_CHECKER(sequence_checker_);
 

chrome/updater/win/update_service_out_of_process.cc

@@ -288,16 +288,10 @@ UpdateServiceOutOfProcess::UpdateServiceOutOfProcess(ServiceScope service_scope)
       com_task_runner_(
           base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {
   DCHECK_EQ(service_scope, ServiceScope::kUser);
-  Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
-      &UpdateServiceOutOfProcess::ModuleStop);
 }
 
 UpdateServiceOutOfProcess::~UpdateServiceOutOfProcess() = default;
 
-void UpdateServiceOutOfProcess::ModuleStop() {
-  DVLOG(2) << __func__ << ": COM client is shutting down.";
-}
-
 void UpdateServiceOutOfProcess::RegisterApp(
     const RegistrationRequest& request,
     base::OnceCallback<void(const RegistrationResponse&)> callback) {

chrome/updater/win/update_service_out_of_process.h

@@ -56,8 +56,6 @@ class UpdateServiceOutOfProcess : public UpdateService {
                    StateChangeCallback state_update,
                    Callback callback);
 
-  static void ModuleStop();
-
   // Bound to the main sequence.
   SEQUENCE_CHECKER(sequence_checker_);