Commit 0a091b70 authored by Henrique Ferreiro's avatar Henrique Ferreiro Committed by Commit Bot

Migrate engine_sandbox.mojom to the new Mojo types

Convert the implementation and all users of the
chrome_cleaner::mojom::EngineScanResults, EngineCleanupResults and
EngineCommands interfaces.

EngineRequests and EngineFileRequests will be migrated in a different
CL.

Bug: 955171
Change-Id: I792cdc4753626621ba9d53d908133297f4520999
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831805Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Reviewed-by: default avatarJoe Mason <joenotcharles@chromium.org>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/master@{#705082}
parent b56f500d
......@@ -10,14 +10,14 @@ namespace chrome_cleaner {
EngineCleanupResultsImpl::EngineCleanupResultsImpl(
InterfaceMetadataObserver* metadata_observer)
: binding_(this), metadata_observer_(metadata_observer) {}
: metadata_observer_(metadata_observer) {}
EngineCleanupResultsImpl::~EngineCleanupResultsImpl() = default;
void EngineCleanupResultsImpl::BindToCallbacks(
mojom::EngineCleanupResultsAssociatedPtrInfo* ptr_info,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults>* cleanup_results,
DoneCallback done_callback) {
binding_.Bind(mojo::MakeRequest(ptr_info));
receiver_.Bind(cleanup_results->InitWithNewEndpointAndPassReceiver());
// There's no need to call set_connection_error_handler on this since it's an
// associated interface. Any errors will be handled on the main EngineCommands
// interface.
......
......@@ -7,7 +7,8 @@
#include "chrome/chrome_cleaner/engines/broker/interface_metadata_observer.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
namespace chrome_cleaner {
......@@ -19,14 +20,15 @@ class EngineCleanupResultsImpl : public mojom::EngineCleanupResults {
using DoneCallback = base::OnceCallback<void(uint32_t result_code)>;
void BindToCallbacks(mojom::EngineCleanupResultsAssociatedPtrInfo* ptr_info,
void BindToCallbacks(mojo::PendingAssociatedRemote<
mojom::EngineCleanupResults>* cleanup_results,
DoneCallback done_callback);
// mojom::EngineCleanupResults
void Done(uint32_t result_code) override;
private:
mojo::AssociatedBinding<mojom::EngineCleanupResults> binding_;
mojo::AssociatedReceiver<mojom::EngineCleanupResults> receiver_{this};
DoneCallback done_callback_;
InterfaceMetadataObserver* metadata_observer_ = nullptr;
};
......
......@@ -22,11 +22,11 @@
#include "base/synchronization/waitable_event.h"
#include "chrome/chrome_cleaner/buildflags.h"
#include "chrome/chrome_cleaner/constants/quarantine_constants.h"
#include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
#include "chrome/chrome_cleaner/logging/logging_service_api.h"
#include "chrome/chrome_cleaner/mojom/engine_file_requests.mojom.h"
#include "chrome/chrome_cleaner/mojom/engine_requests.mojom.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
#include "chrome/chrome_cleaner/logging/logging_service_api.h"
#include "chrome/chrome_cleaner/os/layered_service_provider_wrapper.h"
#include "chrome/chrome_cleaner/os/system_util_cleaner.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
......@@ -34,6 +34,9 @@
#include "chrome/chrome_cleaner/zip_archiver/sandboxed_zip_archiver.h"
#include "components/chrome_cleaner/public/constants/result_codes.h"
#include "mojo/public/cpp/bindings/callback_helpers.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
using base::WaitableEvent;
......@@ -47,7 +50,7 @@ const uint32_t kIncreasedWatchdogTimeoutInSeconds = 15 * 60;
using ResultCallback = base::OnceCallback<void(uint32_t)>;
// Wraps a CallbackWithDeleteHelper around |callback| which is to be passed to
// |engine_commands_ptr_|. If the connection dies before |callback| is invoked,
// |engine_commands_|. If the connection dies before |callback| is invoked,
// Mojo will delete it without running it. In that case call it with default
// arguments to ensure that side effects (such as unblocking a WaitableEvent)
// still happen.
......@@ -119,7 +122,7 @@ EngineClient::EngineClient(
registry_logging_callback_(logging_callback),
connection_error_callback_(connection_error_callback),
mojo_task_runner_(mojo_task_runner),
engine_commands_ptr_(std::make_unique<mojom::EngineCommandsPtr>()),
engine_commands_(std::make_unique<mojo::Remote<mojom::EngineCommands>>()),
interface_metadata_observer_(std::move(metadata_observer)) {
DCHECK(mojo_task_runner_);
InitializeReadOnlyCallbacks();
......@@ -185,7 +188,7 @@ EngineClient::~EngineClient() {
mojo_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
[](std::unique_ptr<mojom::EngineCommandsPtr> commands,
[](std::unique_ptr<mojo::Remote<mojom::EngineCommands>> commands,
std::unique_ptr<EngineScanResultsImpl> scan_results,
std::unique_ptr<EngineCleanupResultsImpl> cleanup_results,
std::unique_ptr<EngineFileRequestsImpl> file_requests,
......@@ -206,8 +209,7 @@ EngineClient::~EngineClient() {
// sandbox_cleaner_requests in order to avoid invalid references.
metadata_observer.reset();
},
base::Passed(&engine_commands_ptr_),
base::Passed(&scan_results_impl_),
base::Passed(&engine_commands_), base::Passed(&scan_results_impl_),
base::Passed(&cleanup_results_impl_),
base::Passed(&sandbox_file_requests_),
base::Passed(&sandbox_requests_),
......@@ -219,19 +221,20 @@ uint32_t EngineClient::ScanningWatchdogTimeoutInSeconds() const {
return kIncreasedWatchdogTimeoutInSeconds;
}
void EngineClient::PostBindEngineCommandsPtr(
void EngineClient::PostBindEngineCommandsRemote(
mojo::ScopedMessagePipeHandle pipe) {
mojo_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&EngineClient::BindEngineCommandsPtr,
FROM_HERE, base::BindOnce(&EngineClient::BindEngineCommandsRemote,
base::RetainedRef(this), std::move(pipe),
base::BindOnce(connection_error_callback_,
SandboxType::kEngine)));
}
void EngineClient::BindEngineCommandsPtr(mojo::ScopedMessagePipeHandle pipe,
base::OnceClosure error_handler) {
engine_commands_ptr_->Bind(mojom::EngineCommandsPtrInfo(std::move(pipe), 0));
engine_commands_ptr_->set_connection_error_handler(std::move(error_handler));
void EngineClient::BindEngineCommandsRemote(mojo::ScopedMessagePipeHandle pipe,
base::OnceClosure error_handler) {
engine_commands_->Bind(
mojo::PendingRemote<mojom::EngineCommands>(std::move(pipe), 0));
engine_commands_->set_disconnect_handler(std::move(error_handler));
}
void EngineClient::MaybeLogResultCode(EngineClient::Operation operation,
......@@ -299,7 +302,7 @@ void EngineClient::InitializeAsync(InitializeCallback result_callback) {
LOG(ERROR) << "Couldn't get development log directory for sandboxed engine";
#endif
(*engine_commands_ptr_)
(*engine_commands_)
->Initialize(std::move(file_requests_info), logging_path,
CallbackWithErrorHandling(std::move(result_callback)));
}
......@@ -352,9 +355,9 @@ void EngineClient::StartScanAsync(
// Create a binding to the EngineScanResults interface that will receive
// results and pass them on to |found_callback| and |done_callback|.
mojom::EngineScanResultsAssociatedPtrInfo scan_results_info;
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results;
scan_results_impl_->BindToCallbacks(&scan_results_info, found_callback,
scan_results_impl_->BindToCallbacks(&scan_results, found_callback,
std::move(done_callback));
if (interface_metadata_observer_)
interface_metadata_observer_->ObserveCall(CURRENT_FILE_AND_METHOD);
......@@ -364,10 +367,10 @@ void EngineClient::StartScanAsync(
// EngineResultCode::kSuccess, scan_results_impl_->FoundUwS (which in turn
// calls |found_callback|) and scan_results_impl_->Done (which in turn calls
// |done_callback|) with further results.
(*engine_commands_ptr_)
(*engine_commands_)
->StartScan(enabled_uws, enabled_locations, include_details,
std::move(file_requests_info),
std::move(engine_requests_info), std::move(scan_results_info),
std::move(engine_requests_info), std::move(scan_results),
CallbackWithErrorHandling(std::move(result_callback)));
}
......@@ -418,18 +421,18 @@ void EngineClient::StartCleanupAsync(const std::vector<UwSId>& enabled_uws,
// Create a binding to the EngineCleanupResults interface that will
// receive results and pass them on to |done_callback|.
mojom::EngineCleanupResultsAssociatedPtrInfo cleanup_results_info;
cleanup_results_impl_->BindToCallbacks(&cleanup_results_info,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults> cleanup_results;
cleanup_results_impl_->BindToCallbacks(&cleanup_results,
std::move(done_callback));
if (interface_metadata_observer_)
interface_metadata_observer_->ObserveCall(CURRENT_FILE_AND_METHOD);
(*engine_commands_ptr_)
(*engine_commands_)
->StartCleanup(enabled_uws, std::move(file_requests_info),
std::move(engine_requests_info),
std::move(cleaner_engine_requests_info),
std::move(cleanup_results_info),
std::move(cleanup_results),
CallbackWithErrorHandling(std::move(result_callback)));
}
......@@ -452,7 +455,7 @@ uint32_t EngineClient::Finalize() {
void EngineClient::FinalizeAsync(FinalizeCallback result_callback) {
if (interface_metadata_observer_)
interface_metadata_observer_->ObserveCall(CURRENT_FILE_AND_METHOD);
(*engine_commands_ptr_)
(*engine_commands_)
->Finalize(CallbackWithErrorHandling(std::move(result_callback)));
}
......
......@@ -24,12 +24,14 @@
#include "chrome/chrome_cleaner/engines/broker/engine_scan_results_impl.h"
#include "chrome/chrome_cleaner/engines/broker/interface_metadata_observer.h"
#include "chrome/chrome_cleaner/engines/common/engine_result_codes.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
#include "chrome/chrome_cleaner/ipc/sandbox.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
#include "chrome/chrome_cleaner/settings/settings_types.h"
#include "chrome/chrome_cleaner/zip_archiver/zip_archiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/message_pipe.h"
namespace chrome_cleaner {
......@@ -86,11 +88,11 @@ class EngineClient : public base::RefCountedThreadSafe<EngineClient> {
// client.
uint32_t ScanningWatchdogTimeoutInSeconds() const;
mojom::EngineCommandsPtr* engine_commands_ptr() const {
return engine_commands_ptr_.get();
mojo::Remote<mojom::EngineCommands>* engine_commands_remote() const {
return engine_commands_.get();
}
// Posts a task to the mojo thread to bind an EngineCommandsPtr to |pipe|.
// Posts a task to the mojo thread to bind an EngineCommands remote to |pipe|.
// |error_handler| will be called for errors on this connection.
//
// TODO(joenotcharles): When the EngineClient interface is updated to be
......@@ -106,7 +108,7 @@ class EngineClient : public base::RefCountedThreadSafe<EngineClient> {
// could happen during shutdown when ScannerImpl has been deleted but
// EngineClient is still being kept alive because a StartScanAsync task
// that's still queued has a reference to it.)
virtual void PostBindEngineCommandsPtr(mojo::ScopedMessagePipeHandle pipe);
virtual void PostBindEngineCommandsRemote(mojo::ScopedMessagePipeHandle pipe);
using FoundUwSCallback = EngineScanResultsImpl::FoundUwSCallback;
using DoneCallback = EngineScanResultsImpl::DoneCallback;
......@@ -160,8 +162,8 @@ class EngineClient : public base::RefCountedThreadSafe<EngineClient> {
using StartCleanupCallback = mojom::EngineCommands::StartCleanupCallback;
using FinalizeCallback = mojom::EngineCommands::FinalizeCallback;
void BindEngineCommandsPtr(mojo::ScopedMessagePipeHandle pipe,
base::OnceClosure error_handler);
void BindEngineCommandsRemote(mojo::ScopedMessagePipeHandle pipe,
base::OnceClosure error_handler);
void InitializeReadOnlyCallbacks();
bool InitializeCleaningCallbacks();
......@@ -211,7 +213,7 @@ class EngineClient : public base::RefCountedThreadSafe<EngineClient> {
// Proxy object that implements the EngineCommands interface by sending the
// commands over IPC to the sandbox target process.
std::unique_ptr<mojom::EngineCommandsPtr> engine_commands_ptr_;
std::unique_ptr<mojo::Remote<mojom::EngineCommands>> engine_commands_;
// Handler for scan results returned over the Mojo pipe.
std::unique_ptr<EngineScanResultsImpl> scan_results_impl_;
......
......@@ -21,13 +21,14 @@ class MockEngineClient : public EngineClient {
// cleanup code for tests.
MockEngineClient();
// To test |PostBindEngineCommandsPtr|, mock the method
// |MockedPostBindEngineCommandsPtr|. This is needed because |pipe| is
// To test |PostBindEngineCommandsRemote|, mock the method
// |MockedPostBindEngineCommandsRemote|. This is needed because |pipe| is
// move-only and gmock generates a call to the copy constructor.
void PostBindEngineCommandsPtr(mojo::ScopedMessagePipeHandle pipe) override {
MockedPostBindEngineCommandsPtr(&pipe);
void PostBindEngineCommandsRemote(
mojo::ScopedMessagePipeHandle pipe) override {
MockedPostBindEngineCommandsRemote(&pipe);
}
MOCK_METHOD1(MockedPostBindEngineCommandsPtr,
MOCK_METHOD1(MockedPostBindEngineCommandsRemote,
void(mojo::ScopedMessagePipeHandle* pipe));
MOCK_CONST_METHOD0(GetEnabledUwS, std::vector<UwSId>());
......
......@@ -10,15 +10,15 @@ namespace chrome_cleaner {
EngineScanResultsImpl::EngineScanResultsImpl(
InterfaceMetadataObserver* metadata_observer)
: binding_(this), metadata_observer_(metadata_observer) {}
: metadata_observer_(metadata_observer) {}
EngineScanResultsImpl::~EngineScanResultsImpl() = default;
void EngineScanResultsImpl::BindToCallbacks(
mojom::EngineScanResultsAssociatedPtrInfo* ptr_info,
mojo::PendingAssociatedRemote<mojom::EngineScanResults>* scan_results,
FoundUwSCallback found_uws_callback,
DoneCallback done_callback) {
binding_.Bind(mojo::MakeRequest(ptr_info));
receiver_.Bind(scan_results->InitWithNewEndpointAndPassReceiver());
// There's no need to call set_connection_error_handler on this since it's an
// associated interface. Any errors will be handled on the main EngineCommands
// interface.
......
......@@ -11,7 +11,8 @@
#include "chrome/chrome_cleaner/engines/broker/interface_metadata_observer.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
namespace chrome_cleaner {
......@@ -27,9 +28,10 @@ class EngineScanResultsImpl : public mojom::EngineScanResults {
base::RepeatingCallback<void(UwSId pup_id, const PUPData::PUP& pup)>;
using DoneCallback = base::OnceCallback<void(uint32_t result_code)>;
void BindToCallbacks(mojom::EngineScanResultsAssociatedPtrInfo* ptr_info,
FoundUwSCallback found_uws_callback,
DoneCallback done_callback);
void BindToCallbacks(
mojo::PendingAssociatedRemote<mojom::EngineScanResults>* scan_results,
FoundUwSCallback found_uws_callback,
DoneCallback done_callback);
// mojom::EngineScanResults
......@@ -37,7 +39,7 @@ class EngineScanResultsImpl : public mojom::EngineScanResults {
void Done(uint32_t result_code) override;
private:
mojo::AssociatedBinding<mojom::EngineScanResults> binding_;
mojo::AssociatedReceiver<mojom::EngineScanResults> receiver_{this};
FoundUwSCallback found_uws_callback_;
DoneCallback done_callback_;
InterfaceMetadataObserver* metadata_observer_ = nullptr;
......
......@@ -52,10 +52,10 @@ ResultCode SpawnWithoutSandboxForTesting(
// leaks deliberately since this is only for testing and it needs to outlive
// the EngineClient object.
//
// When using a sandbox the ptr is bound to the broker end of a pipe in
// EngineClient::PostBindEngineCommandsPtr and the impl is bound to the
// target end in EngineMojoSandboxTargetHooks::BindEngineCommandsRequest.
// This binds the ptr directly to the impl. There's no need for an error
// When using a sandbox, the remote is bound to the broker end of a pipe in
// EngineClient::PostBindEngineCommandsRemote and the impl is bound to the
// target end in EngineMojoSandboxTargetHooks::BindEngineCommandsReceiver.
// This binds the remote directly to the impl. There's no need for an error
// handling callback because there's no pipe that can have errors.
mojo_task_runner->PostTask(
FROM_HERE,
......@@ -63,10 +63,10 @@ ResultCode SpawnWithoutSandboxForTesting(
[](scoped_refptr<EngineClient> engine_client,
scoped_refptr<EngineDelegate> engine_delegate,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
new EngineCommandsImpl(
engine_delegate,
mojo::MakeRequest(engine_client->engine_commands_ptr()),
task_runner, base::DoNothing::Repeatedly());
new EngineCommandsImpl(engine_delegate,
engine_client->engine_commands_remote()
->BindNewPipeAndPassReceiver(),
task_runner, base::DoNothing::Repeatedly());
},
engine_client, CreateEngineDelegate(engine_name), mojo_task_runner));
......@@ -97,7 +97,7 @@ ResultCode EngineSandboxSetupHooks::UpdateSandboxPolicy(
mojo::ScopedMessagePipeHandle mojo_pipe =
SetupSandboxMessagePipe(policy, command_line);
engine_client_->PostBindEngineCommandsPtr(std::move(mojo_pipe));
engine_client_->PostBindEngineCommandsRemote(std::move(mojo_pipe));
// Propagate engine selection switches to the sandbox target.
command_line->AppendSwitchNative(
......
......@@ -73,7 +73,7 @@ class ExtensionTestSandboxHooks : public MojoSandboxSetupHooks {
mojo::ScopedMessagePipeHandle pipe_handle =
SetupSandboxMessagePipe(policy, command_line);
engine_client_->PostBindEngineCommandsPtr(std::move(pipe_handle));
engine_client_->PostBindEngineCommandsRemote(std::move(pipe_handle));
return RESULT_CODE_SUCCESS;
}
......
......@@ -42,6 +42,7 @@ source_set("common") {
"//chrome/chrome_cleaner/pup_data:pup_data_base",
"//chrome/chrome_cleaner/strings",
"//components/chrome_cleaner/public/constants:constants",
"//mojo/public/cpp/bindings",
"//sandbox/win:sandbox",
]
......@@ -110,9 +111,11 @@ source_set("test_support") {
"//chrome/chrome_cleaner/engines/common",
"//chrome/chrome_cleaner/engines/common:resources_header",
"//chrome/chrome_cleaner/ipc:ipc_test_util",
"//chrome/chrome_cleaner/mojom:engine_sandbox_interface",
"//chrome/chrome_cleaner/os:cleaner_os",
"//chrome/chrome_cleaner/os:common_os",
"//chrome/chrome_cleaner/pup_data:pup_data_base",
"//mojo/public/cpp/bindings",
]
}
......
......@@ -12,13 +12,14 @@
namespace chrome_cleaner {
EngineCleanupResultsProxy::EngineCleanupResultsProxy(
mojom::EngineCleanupResultsAssociatedPtr cleanup_results_ptr,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults> cleanup_results,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: cleanup_results_ptr_(std::move(cleanup_results_ptr)),
task_runner_(task_runner) {}
: task_runner_(task_runner) {
cleanup_results_.Bind(std::move(cleanup_results));
}
void EngineCleanupResultsProxy::UnbindCleanupResultsPtr() {
cleanup_results_ptr_.reset();
void EngineCleanupResultsProxy::UnbindCleanupResults() {
cleanup_results_.reset();
}
void EngineCleanupResultsProxy::CleanupDone(uint32_t result) {
......@@ -30,11 +31,11 @@ void EngineCleanupResultsProxy::CleanupDone(uint32_t result) {
EngineCleanupResultsProxy::~EngineCleanupResultsProxy() = default;
void EngineCleanupResultsProxy::OnDone(uint32_t result) {
if (!cleanup_results_ptr_.is_bound()) {
if (!cleanup_results_.is_bound()) {
LOG(ERROR) << "Cleanup result reported after the engine was shut down";
return;
}
cleanup_results_ptr_->Done(result);
cleanup_results_->Done(result);
}
} // namespace chrome_cleaner
......@@ -10,6 +10,8 @@
#include "base/memory/ref_counted.h"
#include "base/single_thread_task_runner.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
namespace chrome_cleaner {
......@@ -18,14 +20,15 @@ class EngineCleanupResultsProxy
: public base::RefCountedThreadSafe<EngineCleanupResultsProxy> {
public:
EngineCleanupResultsProxy(
mojom::EngineCleanupResultsAssociatedPtr cleanup_results_ptr,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults>
cleanup_results,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
return task_runner_;
}
void UnbindCleanupResultsPtr();
void UnbindCleanupResults();
// Sends a cleanup done signal to the broken process. Will be called on an
// arbitrary thread from the sandboxed engine.
......@@ -35,12 +38,12 @@ class EngineCleanupResultsProxy
friend class base::RefCountedThreadSafe<EngineCleanupResultsProxy>;
~EngineCleanupResultsProxy();
// Invokes cleanup_results_ptr_->Done from the IPC thread.
// Invokes cleanup_results_->Done from the IPC thread.
void OnDone(uint32_t result);
// An EngineCleanupResults that will send the results over the Mojo
// connection.
mojom::EngineCleanupResultsAssociatedPtr cleanup_results_ptr_;
mojo::AssociatedRemote<mojom::EngineCleanupResults> cleanup_results_;
// A task runner for the IPC thread.
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
......
......@@ -20,14 +20,10 @@ namespace chrome_cleaner {
using mojom::CleanerEngineRequestsAssociatedPtr;
using mojom::CleanerEngineRequestsAssociatedPtrInfo;
using mojom::EngineCleanupResultsAssociatedPtr;
using mojom::EngineCleanupResultsAssociatedPtrInfo;
using mojom::EngineFileRequestsAssociatedPtr;
using mojom::EngineFileRequestsAssociatedPtrInfo;
using mojom::EngineRequestsAssociatedPtr;
using mojom::EngineRequestsAssociatedPtrInfo;
using mojom::EngineScanResultsAssociatedPtr;
using mojom::EngineScanResultsAssociatedPtrInfo;
namespace {
......@@ -54,13 +50,13 @@ class ScopedCrashStageRecorder {
EngineCommandsImpl::EngineCommandsImpl(
scoped_refptr<EngineDelegate> engine_delegate,
mojom::EngineCommandsRequest request,
mojo::PendingReceiver<mojom::EngineCommands> receiver,
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::OnceClosure error_handler)
: engine_delegate_(engine_delegate),
binding_(this, std::move(request)),
receiver_(this, std::move(receiver)),
task_runner_(task_runner) {
binding_.set_connection_error_handler(std::move(error_handler));
receiver_.set_disconnect_handler(std::move(error_handler));
}
EngineCommandsImpl::~EngineCommandsImpl() = default;
......@@ -95,7 +91,7 @@ void EngineCommandsImpl::StartScan(
bool include_details,
mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
EngineScanResultsAssociatedPtrInfo scan_results_info,
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
StartScanCallback callback) {
ScopedCrashStageRecorder crash_stage(__func__);
......@@ -114,10 +110,8 @@ void EngineCommandsImpl::StartScan(
// Create an EngineScanResults proxy to send results back over the
// Mojo connection.
EngineScanResultsAssociatedPtr scan_results_ptr;
scan_results_ptr.Bind(std::move(scan_results_info));
scoped_refptr<EngineScanResultsProxy> scan_results_proxy =
base::MakeRefCounted<EngineScanResultsProxy>(std::move(scan_results_ptr),
base::MakeRefCounted<EngineScanResultsProxy>(std::move(scan_results),
task_runner_);
uint32_t result_code = engine_delegate_->StartScan(
......@@ -132,7 +126,7 @@ void EngineCommandsImpl::StartCleanup(
mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
mojom::CleanerEngineRequestsAssociatedPtrInfo
sandboxed_cleaner_engine_requests,
mojom::EngineCleanupResultsAssociatedPtrInfo clean_results_info,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults> cleanup_results,
StartCleanupCallback callback) {
ScopedCrashStageRecorder crash_stage(__func__);
......@@ -158,11 +152,9 @@ void EngineCommandsImpl::StartCleanup(
// Create an EngineCleanupResults proxy to send results back over the
// Mojo connection.
mojom::EngineCleanupResultsAssociatedPtr cleanup_results_ptr;
cleanup_results_ptr.Bind(std::move(clean_results_info));
scoped_refptr<EngineCleanupResultsProxy> cleanup_results_proxy =
base::MakeRefCounted<EngineCleanupResultsProxy>(
std::move(cleanup_results_ptr), task_runner_);
std::move(cleanup_results), task_runner_);
uint32_t result_code = engine_delegate_->StartCleanup(
enabled_uws, file_requests_proxy, engine_requests_proxy,
......
......@@ -16,14 +16,16 @@
#include "chrome/chrome_cleaner/mojom/engine_requests.mojom.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
namespace chrome_cleaner {
class EngineCommandsImpl : public mojom::EngineCommands {
public:
EngineCommandsImpl(scoped_refptr<EngineDelegate> engine_delegate,
mojom::EngineCommandsRequest request,
mojo::PendingReceiver<mojom::EngineCommands> receiver,
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::OnceClosure error_handler);
~EngineCommandsImpl() override;
......@@ -39,7 +41,7 @@ class EngineCommandsImpl : public mojom::EngineCommands {
bool include_details,
mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
mojom::EngineScanResultsAssociatedPtrInfo scan_results_info,
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
StartScanCallback callback) override;
void StartCleanup(
const std::vector<UwSId>& enabled_uws,
......@@ -47,7 +49,8 @@ class EngineCommandsImpl : public mojom::EngineCommands {
mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
mojom::CleanerEngineRequestsAssociatedPtrInfo
sandboxed_cleaner_engine_requests,
mojom::EngineCleanupResultsAssociatedPtrInfo cleanup_results_info,
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults>
cleanup_results,
StartCleanupCallback callback) override;
void Finalize(FinalizeCallback callback) override;
......@@ -58,7 +61,7 @@ class EngineCommandsImpl : public mojom::EngineCommands {
uint32_t result_code);
scoped_refptr<EngineDelegate> engine_delegate_;
mojo::Binding<mojom::EngineCommands> binding_;
mojo::Receiver<mojom::EngineCommands> receiver_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
};
......
......@@ -13,13 +13,14 @@
namespace chrome_cleaner {
EngineScanResultsProxy::EngineScanResultsProxy(
mojom::EngineScanResultsAssociatedPtr scan_results_ptr,
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: scan_results_ptr_(std::move(scan_results_ptr)),
task_runner_(task_runner) {}
: task_runner_(task_runner) {
scan_results_.Bind(std::move(scan_results));
}
void EngineScanResultsProxy::UnbindScanResultsPtr() {
scan_results_ptr_.reset();
void EngineScanResultsProxy::UnbindScanResults() {
scan_results_.reset();
}
void EngineScanResultsProxy::FoundUwS(UwSId pup_id, const PUPData::PUP& pup) {
......@@ -35,22 +36,22 @@ void EngineScanResultsProxy::ScanDone(uint32_t result) {
EngineScanResultsProxy::~EngineScanResultsProxy() = default;
// Invokes scan_results_ptr_->FoundUwS from the IPC thread.
// Invokes scan_results_->FoundUwS from the IPC thread.
void EngineScanResultsProxy::OnFoundUwS(UwSId pup_id, const PUPData::PUP& pup) {
if (!scan_results_ptr_.is_bound()) {
if (!scan_results_.is_bound()) {
LOG(ERROR) << "Found UwS reported after the engine was shut down";
return;
}
scan_results_ptr_->FoundUwS(pup_id, pup);
scan_results_->FoundUwS(pup_id, pup);
}
// Invokes scan_results_ptr_->Done from the IPC thread.
// Invokes scan_results_->Done from the IPC thread.
void EngineScanResultsProxy::OnDone(uint32_t result) {
if (!scan_results_ptr_.is_bound()) {
if (!scan_results_.is_bound()) {
LOG(ERROR) << "Scan result reported after the engine was shut down";
return;
}
scan_results_ptr_->Done(result);
scan_results_->Done(result);
}
} // namespace chrome_cleaner
......@@ -9,6 +9,8 @@
#include "base/single_thread_task_runner.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
namespace chrome_cleaner {
......@@ -17,14 +19,14 @@ class EngineScanResultsProxy
: public base::RefCountedThreadSafe<EngineScanResultsProxy> {
public:
EngineScanResultsProxy(
mojom::EngineScanResultsAssociatedPtr scan_results_ptr,
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
return task_runner_;
}
void UnbindScanResultsPtr();
void UnbindScanResults();
// Notifies the broker process that UwS was found. Will be called on an
// arbitrary thread from the sandboxed engine.
......@@ -47,7 +49,7 @@ class EngineScanResultsProxy
void OnDone(uint32_t result);
// An EngineScanResults that will send the results over the Mojo connection.
mojom::EngineScanResultsAssociatedPtr scan_results_ptr_;
mojo::AssociatedRemote<mojom::EngineScanResults> scan_results_;
// A task runner for the IPC thread.
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
......
......@@ -19,7 +19,9 @@
#include "chrome/chrome_cleaner/engines/target/libraries.h"
#include "chrome/chrome_cleaner/ipc/mojo_sandbox_hooks.h"
#include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/os/early_exit.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
namespace chrome_cleaner {
......@@ -31,7 +33,8 @@ class EngineMojoSandboxTargetHooks : public MojoSandboxTargetHooks {
MojoTaskRunner* mojo_task_runner);
~EngineMojoSandboxTargetHooks() override;
void BindEngineCommandsRequest(mojom::EngineCommandsRequest request);
void BindEngineCommandsReceiver(
mojo::PendingReceiver<mojom::EngineCommands> receiver);
// SandboxTargetHooks
......@@ -66,22 +69,23 @@ EngineMojoSandboxTargetHooks::~EngineMojoSandboxTargetHooks() {
ResultCode EngineMojoSandboxTargetHooks::TargetDroppedPrivileges(
const base::CommandLine& command_line) {
// Connect to the Mojo message pipe from the parent process.
mojom::EngineCommandsRequest request(ExtractSandboxMessagePipe(command_line));
mojo::PendingReceiver<mojom::EngineCommands> receiver(
ExtractSandboxMessagePipe(command_line));
// This loop will run forever. Once the communication channel with the broker
// process is broken, mojo error handler will abort this process.
base::RunLoop run_loop;
mojo_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&EngineMojoSandboxTargetHooks::BindEngineCommandsRequest,
base::Unretained(this), base::Passed(&request)));
base::BindOnce(&EngineMojoSandboxTargetHooks::BindEngineCommandsReceiver,
base::Unretained(this), base::Passed(&receiver)));
run_loop.Run();
return RESULT_CODE_SUCCESS;
}
void EngineMojoSandboxTargetHooks::BindEngineCommandsRequest(
mojom::EngineCommandsRequest request) {
void EngineMojoSandboxTargetHooks::BindEngineCommandsReceiver(
mojo::PendingReceiver<mojom::EngineCommands> receiver) {
// If the connection dies, the parent process has terminated unexpectedly.
// Exit immediately. The child process should be killed automatically if the
// parent dies, so this is just a fallback. The exit code is arbitrary since
......@@ -89,7 +93,7 @@ void EngineMojoSandboxTargetHooks::BindEngineCommandsRequest(
auto error_handler = base::BindOnce(&EarlyExit, 1);
engine_commands_impl_ = std::make_unique<EngineCommandsImpl>(
std::move(engine_delegate_), std::move(request), mojo_task_runner_,
std::move(engine_delegate_), std::move(receiver), mojo_task_runner_,
std::move(error_handler));
}
......
......@@ -11,6 +11,8 @@
#include "chrome/chrome_cleaner/engines/common/engine_result_codes.h"
#include "chrome/chrome_cleaner/os/early_exit.h"
#include "chrome/chrome_cleaner/os/initializer.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/system/message_pipe.h"
namespace chrome_cleaner {
......@@ -102,24 +104,24 @@ SandboxChildProcess::SandboxChildProcess(
mojo::ScopedMessagePipeHandle message_pipe_handle =
CreateMessagePipeFromCommandLine();
mojom::EngineCommandsRequest engine_commands_request(
mojo::PendingReceiver<mojom::EngineCommands> engine_commands_receiver(
std::move(message_pipe_handle));
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
mojo_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&SandboxChildProcess::BindEngineCommandsRequest,
base::BindOnce(&SandboxChildProcess::BindEngineCommandsReceiver,
base::Unretained(this),
base::Passed(&engine_commands_request), &event));
base::Passed(&engine_commands_receiver), &event));
event.Wait();
}
void SandboxChildProcess::SandboxChildProcess::BindEngineCommandsRequest(
mojom::EngineCommandsRequest request,
void SandboxChildProcess::SandboxChildProcess::BindEngineCommandsReceiver(
mojo::PendingReceiver<mojom::EngineCommands> receiver,
base::WaitableEvent* event) {
fake_engine_delegate_ = base::MakeRefCounted<FakeEngineDelegate>(event);
engine_commands_impl_ = std::make_unique<EngineCommandsImpl>(
fake_engine_delegate_, std::move(request), mojo_task_runner_,
fake_engine_delegate_, std::move(receiver), mojo_task_runner_,
/*error_handler=*/base::BindOnce(&EarlyExit, kConnectionErrorExitCode));
}
......
......@@ -24,9 +24,14 @@
#include "chrome/chrome_cleaner/engines/target/engine_file_requests_proxy.h"
#include "chrome/chrome_cleaner/engines/target/engine_requests_proxy.h"
#include "chrome/chrome_cleaner/ipc/ipc_test_util.h"
#include "chrome/chrome_cleaner/mojom/engine_sandbox.mojom.h"
#include "chrome/chrome_cleaner/os/file_remover.h"
#include "chrome/chrome_cleaner/os/layered_service_provider_wrapper.h"
#include "chrome/chrome_cleaner/pup_data/pup_data.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
namespace chrome_cleaner {
......@@ -46,7 +51,8 @@ class MaybeSandboxedParentProcess : public BaseClass {
InterfaceMetadataObserver* metadata_observer = nullptr)
: BaseClass(std::move(mojo_task_runner)),
requests_to_setup_(requests_to_setup),
engine_commands_ptr_(std::make_unique<mojom::EngineCommandsPtr>()),
engine_commands_(
std::make_unique<mojo::Remote<mojom::EngineCommands>>()),
file_requests_impl_(
std::make_unique<EngineFileRequestsImpl>(this->mojo_task_runner(),
metadata_observer)) {
......@@ -76,56 +82,54 @@ class MaybeSandboxedParentProcess : public BaseClass {
protected:
void CreateImpl(mojo::ScopedMessagePipeHandle mojo_pipe) override {
engine_commands_ptr_->Bind(
mojom::EngineCommandsPtrInfo(std::move(mojo_pipe), 0));
engine_commands_->Bind(
mojo::PendingRemote<mojom::EngineCommands>(std::move(mojo_pipe), 0));
mojom::EngineFileRequestsAssociatedPtrInfo file_requests_info;
file_requests_impl_->Bind(&file_requests_info);
// Bind to empty callbacks as we don't care about the result.
mojom::EngineRequestsAssociatedPtrInfo scanner_info;
mojom::EngineScanResultsAssociatedPtrInfo scanner_results_info;
mojo::PendingAssociatedRemote<mojom::EngineScanResults> scanner_results;
if (requests_to_setup_ == CallbacksToSetup::kScanAndCleanupRequests ||
requests_to_setup_ == CallbacksToSetup::kCleanupRequests) {
scanner_impl_->Bind(&scanner_info);
scan_results_impl_->BindToCallbacks(
&scanner_results_info,
&scanner_results,
base::BindRepeating(
base::DoNothing::Repeatedly<UwSId, const PUPData::PUP&>()),
base::BindOnce(base::DoNothing::Once<uint32_t>()));
}
mojom::CleanerEngineRequestsAssociatedPtrInfo cleaner_info;
mojom::EngineCleanupResultsAssociatedPtrInfo cleaner_results_info;
mojo::PendingAssociatedRemote<mojom::EngineCleanupResults> cleanup_results;
if (requests_to_setup_ == CallbacksToSetup::kCleanupRequests) {
cleaner_impl_->Bind(&cleaner_info);
cleanup_results_impl_->BindToCallbacks(
&cleaner_results_info,
base::BindOnce(base::DoNothing::Once<uint32_t>()));
&cleanup_results, base::BindOnce(base::DoNothing::Once<uint32_t>()));
}
// Now call the target process to signal that setup is finished.
auto operation_started = base::BindOnce([](uint32_t unused_result_code) {});
if (requests_to_setup_ == CallbacksToSetup::kFileRequests) {
(*engine_commands_ptr_)
(*engine_commands_)
->Initialize(std::move(file_requests_info), base::FilePath(),
std::move(operation_started));
} else if (requests_to_setup_ ==
CallbacksToSetup::kScanAndCleanupRequests) {
(*engine_commands_ptr_)
(*engine_commands_)
->StartScan(/*enabled_uws=*/std::vector<UwSId>{},
/*enabled_locations=*/std::vector<UwS::TraceLocation>{},
/*include_details=*/false, std::move(file_requests_info),
std::move(scanner_info), std::move(scanner_results_info),
std::move(scanner_info), std::move(scanner_results),
std::move(operation_started));
} else if (requests_to_setup_ == CallbacksToSetup::kCleanupRequests) {
(*engine_commands_ptr_)
(*engine_commands_)
->StartCleanup(/*enabled_uws=*/std::vector<UwSId>(),
std::move(file_requests_info), std::move(scanner_info),
std::move(cleaner_info),
std::move(cleaner_results_info),
std::move(cleaner_info), std::move(cleanup_results),
std::move(operation_started));
}
}
......@@ -133,7 +137,7 @@ class MaybeSandboxedParentProcess : public BaseClass {
void DestroyImpl() override {
// Reset everything in the reverse order. Reset the associated pointers
// first since they will error if they are closed after
// |engine_commands_ptr|.
// |engine_commands_|.
cleanup_results_impl_.reset();
cleaner_impl_.reset();
......@@ -141,12 +145,12 @@ class MaybeSandboxedParentProcess : public BaseClass {
scanner_impl_.reset();
file_requests_impl_.reset();
engine_commands_ptr_.reset();
engine_commands_.reset();
}
CallbacksToSetup requests_to_setup_;
std::unique_ptr<mojom::EngineCommandsPtr> engine_commands_ptr_;
std::unique_ptr<mojo::Remote<mojom::EngineCommands>> engine_commands_;
std::unique_ptr<EngineFileRequestsImpl> file_requests_impl_;
std::unique_ptr<EngineRequestsImpl> scanner_impl_;
......@@ -160,8 +164,9 @@ class SandboxChildProcess : public ChildProcess {
public:
explicit SandboxChildProcess(scoped_refptr<MojoTaskRunner> mojo_task_runner);
void BindEngineCommandsRequest(mojom::EngineCommandsRequest request,
base::WaitableEvent* event);
void BindEngineCommandsReceiver(
mojo::PendingReceiver<mojom::EngineCommands> receiver,
base::WaitableEvent* event);
scoped_refptr<EngineFileRequestsProxy> GetFileRequestsProxy();
scoped_refptr<EngineRequestsProxy> GetEngineRequestsProxy();
......
......@@ -52,12 +52,14 @@ interface EngineCommands {
// and |results| is not used. Otherwise a success result code is returned and
// any further errors will be reported by calling Done on the |results|
// interface.
StartScan(array<uint32> enabled_uws,
array<TraceLocation> enabled_trace_locations,
bool include_details,
associated EngineFileRequests file_requests,
associated EngineRequests sandboxed_engine_requests,
associated EngineScanResults results) => (uint32 result_code);
StartScan(
array<uint32> enabled_uws,
array<TraceLocation> enabled_trace_locations,
bool include_details,
associated EngineFileRequests file_requests,
associated EngineRequests sandboxed_engine_requests,
pending_associated_remote<EngineScanResults> results) =>
(uint32 result_code);
// Starts cleaning up the user's system. |enabled_uws| contains a list of UwS
// IDs to cleanup.
......@@ -66,12 +68,13 @@ interface EngineCommands {
// and |results| is not used. Otherwise a success result code is returned and
// any further errors will be reported by calling Done on the |results|
// interface.
StartCleanup(array<uint32> enabled_uws,
associated EngineFileRequests file_requests,
associated EngineRequests sandboxed_engine_requests,
associated CleanerEngineRequests
sandboxed_cleaner_engine_requests,
associated EngineCleanupResults results) => (uint32 result_code);
StartCleanup(
array<uint32> enabled_uws,
associated EngineFileRequests file_requests,
associated EngineRequests sandboxed_engine_requests,
associated CleanerEngineRequests sandboxed_cleaner_engine_requests,
pending_associated_remote<EngineCleanupResults> results) =>
(uint32 result_code);
// Runs the engine's finalization routine.
Finalize() => (uint32 result_code);
......
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