Commit e7745ead authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

Revert "Initialize the WRL::Module only once."

This reverts commit e69195e1.

Reason for revert:
This is a speculative revert. updater_tests has been failing on Win7
Tests (dbg) since the CL range containing this change:
https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%28dbg%29%281%29/85110

Original change's description:
> 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/+/2414847
> Reviewed-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}

TBR=ganesh@chromium.org,sorin@chromium.org,waffles@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1128748
Change-Id: I63884d9bc165130c7eaab196520eb4583c87970d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426149Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810030}
parent 4fd930e1
...@@ -2,42 +2,18 @@ ...@@ -2,42 +2,18 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include <wrl/module.h>
#include "base/logging.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/no_destructor.h"
#include "chrome/updater/service_scope.h" #include "chrome/updater/service_scope.h"
#include "chrome/updater/win/control_service_out_of_process.h" #include "chrome/updater/win/control_service_out_of_process.h"
#include "chrome/updater/win/update_service_out_of_process.h" #include "chrome/updater/win/update_service_out_of_process.h"
namespace updater { 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() { scoped_refptr<UpdateService> CreateUpdateService() {
WRLModuleInitializer::Get();
return base::MakeRefCounted<UpdateServiceOutOfProcess>(GetProcessScope()); return base::MakeRefCounted<UpdateServiceOutOfProcess>(GetProcessScope());
} }
scoped_refptr<ControlService> CreateControlService() { scoped_refptr<ControlService> CreateControlService() {
WRLModuleInitializer::Get();
return base::MakeRefCounted<ControlServiceOutOfProcess>(GetProcessScope()); return base::MakeRefCounted<ControlServiceOutOfProcess>(GetProcessScope());
} }
......
...@@ -111,7 +111,6 @@ source_set("lib") { ...@@ -111,7 +111,6 @@ source_set("lib") {
defines = [ "SECURITY_WIN32" ] defines = [ "SECURITY_WIN32" ]
libs = [ libs = [
"RuntimeObject.lib",
"secur32.lib", "secur32.lib",
"taskschd.lib", "taskschd.lib",
"winhttp.lib", "winhttp.lib",
......
...@@ -130,12 +130,19 @@ void UpdaterControlObserver::OnCompleteOnSTA() { ...@@ -130,12 +130,19 @@ void UpdaterControlObserver::OnCompleteOnSTA() {
ControlServiceOutOfProcess::ControlServiceOutOfProcess(ServiceScope /*scope*/) ControlServiceOutOfProcess::ControlServiceOutOfProcess(ServiceScope /*scope*/)
: com_task_runner_( : com_task_runner_(
base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {} base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {
Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
&ControlServiceOutOfProcess::ModuleStop);
}
ControlServiceOutOfProcess::~ControlServiceOutOfProcess() { ControlServiceOutOfProcess::~ControlServiceOutOfProcess() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
} }
void ControlServiceOutOfProcess::ModuleStop() {
DVLOG(2) << __func__ << ": COM client is shutting down.";
}
void ControlServiceOutOfProcess::Uninitialize() { void ControlServiceOutOfProcess::Uninitialize() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
} }
......
...@@ -34,6 +34,8 @@ class ControlServiceOutOfProcess : public ControlService { ...@@ -34,6 +34,8 @@ class ControlServiceOutOfProcess : public ControlService {
void RunOnSTA(base::OnceClosure callback); void RunOnSTA(base::OnceClosure callback);
void InitializeUpdateServiceOnSTA(base::OnceClosure callback); void InitializeUpdateServiceOnSTA(base::OnceClosure callback);
static void ModuleStop();
// Bound to the main sequence. // Bound to the main sequence.
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
......
...@@ -288,10 +288,16 @@ UpdateServiceOutOfProcess::UpdateServiceOutOfProcess(ServiceScope service_scope) ...@@ -288,10 +288,16 @@ UpdateServiceOutOfProcess::UpdateServiceOutOfProcess(ServiceScope service_scope)
com_task_runner_( com_task_runner_(
base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) { base::ThreadPool::CreateCOMSTATaskRunner(kComClientTraits)) {
DCHECK_EQ(service_scope, ServiceScope::kUser); DCHECK_EQ(service_scope, ServiceScope::kUser);
Microsoft::WRL::Module<Microsoft::WRL::OutOfProc>::Create(
&UpdateServiceOutOfProcess::ModuleStop);
} }
UpdateServiceOutOfProcess::~UpdateServiceOutOfProcess() = default; UpdateServiceOutOfProcess::~UpdateServiceOutOfProcess() = default;
void UpdateServiceOutOfProcess::ModuleStop() {
DVLOG(2) << __func__ << ": COM client is shutting down.";
}
void UpdateServiceOutOfProcess::RegisterApp( void UpdateServiceOutOfProcess::RegisterApp(
const RegistrationRequest& request, const RegistrationRequest& request,
base::OnceCallback<void(const RegistrationResponse&)> callback) { base::OnceCallback<void(const RegistrationResponse&)> callback) {
......
...@@ -56,6 +56,8 @@ class UpdateServiceOutOfProcess : public UpdateService { ...@@ -56,6 +56,8 @@ class UpdateServiceOutOfProcess : public UpdateService {
StateChangeCallback state_update, StateChangeCallback state_update,
Callback callback); Callback callback);
static void ModuleStop();
// Bound to the main sequence. // Bound to the main sequence.
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment