Commit 5805ac7f authored by Joe Mason's avatar Joe Mason Committed by Commit Bot

Reland "Implement ProtoChromePromptIPC."

This is a reland of e5f1b06d

It was reverted in https://crrev.com/c/1760644 for causing test failures on CI
builders that couldn't be reproduced on other machines.

This patch disables the failing tests. A followup will reenable the tests with
extra debugging to diagnose the failures on the CI builders.

TBR=wfh
TBR_REASON=Reland of test-only proto change that was already reviewed.

Original change's description:
> Implement ProtoChromePromptIPC.
>
> Bug: 969139
> Change-Id: I35d551b6bdb41b4e6bd7689120c21d9d683ad72f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733747
> Commit-Queue: Oliver Li <olivierli@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Joe Mason <joenotcharles@google.com>
> Cr-Commit-Position: refs/heads/master@{#688357}

Bug: 969139
Change-Id: Iafdc22cc06fbedd5de73bf8bb7881fc46e01f350
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1759154Reviewed-by: default avatarJoe Mason <joenotcharles@google.com>
Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/master@{#689074}
parent e25567b2
...@@ -5,14 +5,6 @@ ...@@ -5,14 +5,6 @@
import("//build/config/jumbo.gni") import("//build/config/jumbo.gni")
import("//third_party/protobuf/proto_library.gni") import("//third_party/protobuf/proto_library.gni")
proto_library("test_only_proto") {
testonly = true
generate_python = false
sources = [
"chrome_prompt_for_tests.proto",
]
}
source_set("public") { source_set("public") {
sources = [ sources = [
"chrome_cleaner_controller_win.cc", "chrome_cleaner_controller_win.cc",
......
...@@ -28,8 +28,8 @@ ...@@ -28,8 +28,8 @@
#include "base/win/scoped_handle.h" #include "base/win/scoped_handle.h"
#include "base/win/win_util.h" #include "base/win/win_util.h"
#include "chrome/browser/safe_browsing/chrome_cleaner/chrome_prompt_actions_win.h" #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_prompt_actions_win.h"
#include "chrome/browser/safe_browsing/chrome_cleaner/chrome_prompt_for_tests.pb.h"
#include "components/chrome_cleaner/public/constants/constants.h" #include "components/chrome_cleaner/public/constants/constants.h"
#include "components/chrome_cleaner/public/proto/chrome_prompt_for_tests.pb.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
......
...@@ -24,14 +24,18 @@ source_set("chrome_prompt_ipc") { ...@@ -24,14 +24,18 @@ source_set("chrome_prompt_ipc") {
"chrome_prompt_ipc.h", "chrome_prompt_ipc.h",
"mojo_chrome_prompt_ipc.cc", "mojo_chrome_prompt_ipc.cc",
"mojo_chrome_prompt_ipc.h", "mojo_chrome_prompt_ipc.h",
"proto_chrome_prompt_ipc.cc",
"proto_chrome_prompt_ipc.h",
] ]
deps = [ deps = [
":mojo_task_runner", ":mojo_task_runner",
"//base", "//base",
"//components/chrome_cleaner/public/interfaces", "//components/chrome_cleaner/public/interfaces",
"//components/chrome_cleaner/public/proto",
"//mojo/public/cpp/platform", "//mojo/public/cpp/platform",
"//mojo/public/cpp/system", "//mojo/public/cpp/system",
"//third_party/protobuf:protobuf_lite",
] ]
} }
...@@ -67,6 +71,7 @@ source_set("ipc_test_util") { ...@@ -67,6 +71,7 @@ source_set("ipc_test_util") {
] ]
deps = [ deps = [
":chrome_prompt_ipc",
":mojo_task_runner", ":mojo_task_runner",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
...@@ -82,9 +87,10 @@ source_set("unittest_sources") { ...@@ -82,9 +87,10 @@ source_set("unittest_sources") {
testonly = true testonly = true
sources = [ sources = [
"chrome_prompt_ipc_unittest.cc", "mojo_chrome_prompt_ipc_unittest.cc",
"mojo_sandbox_hooks_unittest.cc", "mojo_sandbox_hooks_unittest.cc",
"mojo_task_runner_unittest.cc", "mojo_task_runner_unittest.cc",
"proto_chrome_prompt_ipc_unittest.cc",
"sandbox_unittest.cc", "sandbox_unittest.cc",
] ]
...@@ -100,7 +106,10 @@ source_set("unittest_sources") { ...@@ -100,7 +106,10 @@ source_set("unittest_sources") {
"//chrome/chrome_cleaner/mojom:mojo_sandbox_hooks_test_interface", "//chrome/chrome_cleaner/mojom:mojo_sandbox_hooks_test_interface",
"//chrome/chrome_cleaner/os:common_os", "//chrome/chrome_cleaner/os:common_os",
"//chrome/chrome_cleaner/test:test_util", "//chrome/chrome_cleaner/test:test_util",
"//components/chrome_cleaner/public/constants",
"//components/chrome_cleaner/public/interfaces", "//components/chrome_cleaner/public/interfaces",
"//components/chrome_cleaner/public/proto",
"//components/chrome_cleaner/public/proto:test_only_proto",
"//components/chrome_cleaner/test:test_name_helper", "//components/chrome_cleaner/test:test_name_helper",
"//mojo/core/embedder", "//mojo/core/embedder",
"//sandbox/win:sandbox", "//sandbox/win:sandbox",
......
...@@ -85,42 +85,6 @@ class ChromePromptIPC { ...@@ -85,42 +85,6 @@ class ChromePromptIPC {
kDoneInteraction, kDoneInteraction,
}; };
virtual void RunPromptUserTask(
const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::PromptUserCallback callback) = 0;
virtual void RunDisableExtensionsTask(
const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::DisableExtensionsCallback callback) = 0;
// Callback for ChromePrompt::PromptUser, internal state must be
// State::kWaitingForResponseFromChrome. Invokes callback(prompt_acceptance)
// and transitions to state State::kDoneInteraction.
virtual void OnChromeResponseReceived(
mojom::ChromePrompt::PromptUserCallback callback,
mojom::PromptAcceptance prompt_acceptance) = 0;
// Callback for ChromePrompt::DisableExtensions, internal state must be
// State::kDoneInteraction. Invokes callback(extensions_deleted_callback).
virtual void OnChromeResponseReceivedExtensions(
mojom::ChromePrompt::DisableExtensionsCallback callback,
bool extensions_deleted_callback) = 0;
// Connection error handler. Invokes either
// error_handler_->OnConnectionClosed() or
// error_handler_->OnConnectionClosedAfterDone(), depending on the internal
// state.
virtual void OnConnectionError() = 0;
virtual void PromptUserCheckVersion(
const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::PromptUserCallback callback,
uint32_t version) = 0;
State state_ = State::kUninitialized; State state_ = State::kUninitialized;
ErrorHandler* error_handler_ = nullptr; ErrorHandler* error_handler_ = nullptr;
......
...@@ -63,6 +63,14 @@ class MojoSandboxSetupHooks : public SandboxSetupHooks { ...@@ -63,6 +63,14 @@ class MojoSandboxSetupHooks : public SandboxSetupHooks {
SandboxedParentProcess* parent_process_; SandboxedParentProcess* parent_process_;
}; };
} // namespace
namespace internal {
base::FilePath::StringPieceType GetLogPathSuffix() {
return kIPCTestUtilLogSuffix;
}
base::FilePath GetLogPath() { base::FilePath GetLogPath() {
return ScopedLogging::GetLogFilePath(kIPCTestUtilLogSuffix); return ScopedLogging::GetLogFilePath(kIPCTestUtilLogSuffix);
} }
...@@ -107,7 +115,7 @@ void PrintChildProcessLogs() { ...@@ -107,7 +115,7 @@ void PrintChildProcessLogs() {
} }
} }
} // namespace } // namespace internal
ParentProcess::ParentProcess(scoped_refptr<MojoTaskRunner> mojo_task_runner) ParentProcess::ParentProcess(scoped_refptr<MojoTaskRunner> mojo_task_runner)
: command_line_(base::GetMultiProcessTestChildBaseCommandLine()), : command_line_(base::GetMultiProcessTestChildBaseCommandLine()),
...@@ -166,7 +174,7 @@ bool ParentProcess::LaunchConnectedChildProcess( ...@@ -166,7 +174,7 @@ bool ParentProcess::LaunchConnectedChildProcess(
const std::string& child_main_function, const std::string& child_main_function,
base::TimeDelta timeout, base::TimeDelta timeout,
int32_t* exit_code) { int32_t* exit_code) {
if (!DeleteChildProcessLogs()) if (!internal::DeleteChildProcessLogs())
return false; return false;
if (!PrepareAndLaunchTestChildProcess(child_main_function)) if (!PrepareAndLaunchTestChildProcess(child_main_function))
...@@ -182,7 +190,7 @@ bool ParentProcess::LaunchConnectedChildProcess( ...@@ -182,7 +190,7 @@ bool ParentProcess::LaunchConnectedChildProcess(
DestroyImplOnIPCThread(); DestroyImplOnIPCThread();
if (!success || *exit_code != 0) if (!success || *exit_code != 0)
PrintChildProcessLogs(); internal::PrintChildProcessLogs();
return success; return success;
} }
...@@ -289,4 +297,20 @@ std::string ChildProcess::mojo_pipe_token() const { ...@@ -289,4 +297,20 @@ std::string ChildProcess::mojo_pipe_token() const {
return command_line_->GetSwitchValueASCII(kMojoPipeTokenSwitch); return command_line_->GetSwitchValueASCII(kMojoPipeTokenSwitch);
} }
ChromePromptIPCTestErrorHandler::ChromePromptIPCTestErrorHandler(
base::OnceClosure on_closed,
base::OnceClosure on_closed_after_done)
: on_closed_(std::move(on_closed)),
on_closed_after_done_(std::move(on_closed_after_done)) {}
ChromePromptIPCTestErrorHandler::~ChromePromptIPCTestErrorHandler() = default;
void ChromePromptIPCTestErrorHandler::OnConnectionClosed() {
std::move(on_closed_).Run();
}
void ChromePromptIPCTestErrorHandler::OnConnectionClosedAfterDone() {
std::move(on_closed_after_done_).Run();
}
} // namespace chrome_cleaner } // namespace chrome_cleaner
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/process/launch.h" #include "base/process/launch.h"
#include "base/process/process.h" #include "base/process/process.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/chrome_cleaner/ipc/chrome_prompt_ipc.h"
#include "chrome/chrome_cleaner/ipc/mojo_task_runner.h" #include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
#include "chrome/chrome_cleaner/logging/scoped_logging.h" #include "chrome/chrome_cleaner/logging/scoped_logging.h"
#include "mojo/public/cpp/platform/platform_channel.h" #include "mojo/public/cpp/platform/platform_channel.h"
...@@ -126,6 +127,27 @@ class ChildProcess : public base::RefCountedThreadSafe<ChildProcess> { ...@@ -126,6 +127,27 @@ class ChildProcess : public base::RefCountedThreadSafe<ChildProcess> {
bool target_services_initialized_ = false; bool target_services_initialized_ = false;
}; };
class ChromePromptIPCTestErrorHandler : public ChromePromptIPC::ErrorHandler {
public:
ChromePromptIPCTestErrorHandler(base::OnceClosure on_closed,
base::OnceClosure on_closed_after_done);
~ChromePromptIPCTestErrorHandler() override;
void OnConnectionClosed() override;
void OnConnectionClosedAfterDone() override;
private:
base::OnceClosure on_closed_;
base::OnceClosure on_closed_after_done_;
};
namespace internal {
base::FilePath::StringPieceType GetLogPathSuffix();
bool DeleteChildProcessLogs();
void PrintChildProcessLogs();
} // namespace internal
} // namespace chrome_cleaner } // namespace chrome_cleaner
#endif // CHROME_CHROME_CLEANER_IPC_IPC_TEST_UTIL_H_ #endif // CHROME_CHROME_CLEANER_IPC_IPC_TEST_UTIL_H_
...@@ -8,8 +8,7 @@ ...@@ -8,8 +8,7 @@
namespace chrome_cleaner { namespace chrome_cleaner {
MockChromePromptIPC::MockChromePromptIPC() MockChromePromptIPC::MockChromePromptIPC() = default;
: MojoChromePromptIPC(std::string(), nullptr) {}
MockChromePromptIPC::~MockChromePromptIPC() = default; MockChromePromptIPC::~MockChromePromptIPC() = default;
...@@ -22,4 +21,10 @@ void MockChromePromptIPC::PostPromptUserTask( ...@@ -22,4 +21,10 @@ void MockChromePromptIPC::PostPromptUserTask(
&callback); &callback);
} }
void MockChromePromptIPC::PostDisableExtensionsTask(
const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::DisableExtensionsCallback callback) {
MockPostDisableExtensionsTask(extension_ids, &callback);
}
} // namespace chrome_cleaner } // namespace chrome_cleaner
...@@ -8,13 +8,13 @@ ...@@ -8,13 +8,13 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "chrome/chrome_cleaner/ipc/mojo_chrome_prompt_ipc.h" #include "chrome/chrome_cleaner/ipc/chrome_prompt_ipc.h"
#include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h" #include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
namespace chrome_cleaner { namespace chrome_cleaner {
class MockChromePromptIPC : public MojoChromePromptIPC { class MockChromePromptIPC : public ChromePromptIPC {
public: public:
MockChromePromptIPC(); MockChromePromptIPC();
~MockChromePromptIPC() override; ~MockChromePromptIPC() override;
...@@ -25,19 +25,26 @@ class MockChromePromptIPC : public MojoChromePromptIPC { ...@@ -25,19 +25,26 @@ class MockChromePromptIPC : public MojoChromePromptIPC {
void(base::OnceClosure delete_allowed_callback, void(base::OnceClosure delete_allowed_callback,
base::OnceClosure delete_not_allowed_callback)); base::OnceClosure delete_not_allowed_callback));
// Workaround for GMock's limitation, in which MOCK_METHOD* doesn't accept // Workaround for GMock's limitation, in which MOCK_METHOD* doesn't
// base::OnceCallback parameters. Will forward any calls to // accept base::OnceCallback parameters. Will forward any calls to
// MockPostPromptUserTask() and pass along a raw pointer for |callback|. // MockPost*() and pass along a raw pointer for |callback|.
void PostPromptUserTask( void PostPromptUserTask(
const std::vector<base::FilePath>& files_to_delete, const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys, const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids, const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::PromptUserCallback callback) override; mojom::ChromePrompt::PromptUserCallback callback) override;
void PostDisableExtensionsTask(
const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::DisableExtensionsCallback callback) override;
MOCK_METHOD4(MockPostPromptUserTask, MOCK_METHOD4(MockPostPromptUserTask,
void(const std::vector<base::FilePath>& files_to_delete, void(const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys, const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids, const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::PromptUserCallback* callback)); mojom::ChromePrompt::PromptUserCallback* callback));
MOCK_METHOD2(MockPostDisableExtensionsTask,
void(const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::DisableExtensionsCallback* callback));
}; };
} // namespace chrome_cleaner } // namespace chrome_cleaner
......
// Copyright 2018 The Chromium Authors. All rights reserved. // Copyright 2018 The Chromium Authors. All rights reserved.
// 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.
#ifndef CHROME_CHROME_CLEANER_IPC_MOJO_CHROME_PROMPT_IPC_H_ #ifndef CHROME_CHROME_CLEANER_IPC_MOJO_CHROME_PROMPT_IPC_H_
#define CHROME_CHROME_CLEANER_IPC_MOJO_CHROME_PROMPT_IPC_H_ #define CHROME_CHROME_CLEANER_IPC_MOJO_CHROME_PROMPT_IPC_H_
...@@ -87,41 +86,40 @@ class MojoChromePromptIPC : public ChromePromptIPC { ...@@ -87,41 +86,40 @@ class MojoChromePromptIPC : public ChromePromptIPC {
// Runs |chrome_prompt_service_->PromptUser()|. Must be called on the IPC // Runs |chrome_prompt_service_->PromptUser()|. Must be called on the IPC
// thread. // thread.
void RunPromptUserTask( void RunPromptUserTask(const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::FilePath>& files_to_delete, const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& registry_keys, const std::vector<base::string16>& extension_ids,
const std::vector<base::string16>& extension_ids, mojom::ChromePrompt::PromptUserCallback callback);
mojom::ChromePrompt::PromptUserCallback callback) override;
void RunDisableExtensionsTask( void RunDisableExtensionsTask(
const std::vector<base::string16>& extension_ids, const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::DisableExtensionsCallback callback) override; mojom::ChromePrompt::DisableExtensionsCallback callback);
// Callback for ChromePrompt::PromptUser, internal state must be // Callback for ChromePrompt::PromptUser, internal state must be
// State::kWaitingForResponseFromChrome. Invokes callback(prompt_acceptance) // State::kWaitingForResponseFromChrome. Invokes callback(prompt_acceptance)
// and transitions to state State::kDoneInteraction. // and transitions to state State::kDoneInteraction.
void OnChromeResponseReceived( void OnChromeResponseReceived(
mojom::ChromePrompt::PromptUserCallback callback, mojom::ChromePrompt::PromptUserCallback callback,
mojom::PromptAcceptance prompt_acceptance) override; mojom::PromptAcceptance prompt_acceptance);
// Callback for ChromePrompt::DisableExtensions, internal state must be // Callback for ChromePrompt::DisableExtensions, internal state must be
// State::kDoneInteraction. Invokes callback(extensions_deleted_callback). // State::kDoneInteraction. Invokes callback(extensions_deleted_callback).
void OnChromeResponseReceivedExtensions( void OnChromeResponseReceivedExtensions(
mojom::ChromePrompt::DisableExtensionsCallback callback, mojom::ChromePrompt::DisableExtensionsCallback callback,
bool extensions_deleted_callback) override; bool extensions_deleted_callback);
// Connection error handler. Invokes either // Connection error handler. Invokes either
// error_handler_->OnConnectionClosed() or // error_handler_->OnConnectionClosed() or
// error_handler_->OnConnectionClosedAfterDone(), depending on the internal // error_handler_->OnConnectionClosedAfterDone(), depending on the internal
// state. // state.
void OnConnectionError() override; void OnConnectionError();
void PromptUserCheckVersion( void PromptUserCheckVersion(
const std::vector<base::FilePath>& files_to_delete, const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys, const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids, const std::vector<base::string16>& extension_ids,
mojom::ChromePrompt::PromptUserCallback callback, mojom::ChromePrompt::PromptUserCallback callback,
uint32_t version) override; uint32_t version);
private: private:
scoped_refptr<MojoTaskRunner> task_runner_; scoped_refptr<MojoTaskRunner> task_runner_;
......
...@@ -87,7 +87,7 @@ struct TestConfig { ...@@ -87,7 +87,7 @@ struct TestConfig {
ParentDisconnected expected_parent_disconnected; ParentDisconnected expected_parent_disconnected;
}; };
// Parent process. // Class that lives in the parent process and handles that side of the IPC.
class MockChromePrompt : public mojom::ChromePrompt { class MockChromePrompt : public mojom::ChromePrompt {
public: public:
MockChromePrompt(TestConfig test_config, mojom::ChromePromptRequest request) MockChromePrompt(TestConfig test_config, mojom::ChromePromptRequest request)
...@@ -151,6 +151,7 @@ class ChromePromptIPCParentProcess : public ParentProcess { ...@@ -151,6 +151,7 @@ class ChromePromptIPCParentProcess : public ParentProcess {
} }
if (test_config.with_registry_keys) if (test_config.with_registry_keys)
AppendSwitch(kIncludeRegistryKeysSwitch); AppendSwitch(kIncludeRegistryKeysSwitch);
AppendSwitch(kExpectedPromptResultSwitch, AppendSwitch(kExpectedPromptResultSwitch,
base::NumberToString( base::NumberToString(
static_cast<int>(test_config.expected_prompt_acceptance))); static_cast<int>(test_config.expected_prompt_acceptance)));
...@@ -178,26 +179,7 @@ class ChromePromptIPCParentProcess : public ParentProcess { ...@@ -178,26 +179,7 @@ class ChromePromptIPCParentProcess : public ParentProcess {
std::unique_ptr<MockChromePrompt> mock_chrome_prompt_; std::unique_ptr<MockChromePrompt> mock_chrome_prompt_;
}; };
class ChromePromptIPCTestErrorHandler : public ChromePromptIPC::ErrorHandler { // Class that lives in the child process and handles that side of the IPC.
public:
ChromePromptIPCTestErrorHandler(base::OnceClosure on_closed,
base::OnceClosure on_closed_after_done)
: on_closed_(std::move(on_closed)),
on_closed_after_done_(std::move(on_closed_after_done)) {}
~ChromePromptIPCTestErrorHandler() override = default;
void OnConnectionClosed() override { std::move(on_closed_).Run(); }
void OnConnectionClosedAfterDone() override {
std::move(on_closed_after_done_).Run();
}
base::OnceClosure on_closed_;
base::OnceClosure on_closed_after_done_;
};
// Child process.
class ChromePromptIPCChildProcess : public ChildProcess { class ChromePromptIPCChildProcess : public ChildProcess {
public: public:
explicit ChromePromptIPCChildProcess( explicit ChromePromptIPCChildProcess(
...@@ -297,6 +279,7 @@ MULTIPROCESS_TEST_MAIN(ChromePromptIPCClientMain) { ...@@ -297,6 +279,7 @@ MULTIPROCESS_TEST_MAIN(ChromePromptIPCClientMain) {
auto child_process = auto child_process =
base::MakeRefCounted<ChromePromptIPCChildProcess>(mojo_task_runner); base::MakeRefCounted<ChromePromptIPCChildProcess>(mojo_task_runner);
base::RunLoop on_done_run_loop; base::RunLoop on_done_run_loop;
// The parent process can disconnect while the pipe is required or after it's // The parent process can disconnect while the pipe is required or after it's
// no longer needed. In the former case, the child process will immediately // no longer needed. In the former case, the child process will immediately
// exit; in the latter, it will break |on_done_run_loop|, which will be // exit; in the latter, it will break |on_done_run_loop|, which will be
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc.h"
#include <windows.h>
#include "base/strings/utf_string_conversions.h"
#include "base/win/win_util.h"
#include "components/chrome_cleaner/public/proto/chrome_prompt.pb.h"
namespace {
// chrome_cleaner <-> chrome protocol version.
constexpr uint8_t kVersion = 1;
} // namespace
namespace chrome_cleaner {
ProtoChromePromptIPC::ProtoChromePromptIPC(
base::win::ScopedHandle response_read_handle,
base::win::ScopedHandle request_write_handle)
: response_read_handle_(std::move(response_read_handle)),
request_write_handle_(std::move(request_write_handle)) {
// All uses of this class, and more specifically its state member need to
// happen on the same sequence but one that is not the construction
// sequence.
DETACH_FROM_SEQUENCE(sequence_checker_);
}
ProtoChromePromptIPC::~ProtoChromePromptIPC() = default;
void ProtoChromePromptIPC::Initialize(ErrorHandler* error_handler) {
DCHECK(task_runner_);
error_handler_ = error_handler;
task_runner_->PostTask(FROM_HERE,
base::BindOnce(&ProtoChromePromptIPC::InitializeImpl,
base::Unretained(this)));
}
void ProtoChromePromptIPC::PostPromptUserTask(
const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
PromptUserCallback callback) {
DCHECK(task_runner_);
task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&ProtoChromePromptIPC::RunPromptUserTask,
base::Unretained(this), files_to_delete, registry_keys,
extension_ids, std::move(callback)));
}
void ProtoChromePromptIPC::PostDisableExtensionsTask(
const std::vector<base::string16>& extension_ids,
DisableExtensionsCallback callback) {
NOTIMPLEMENTED();
OnConnectionError();
}
void ProtoChromePromptIPC::TryDeleteExtensions(
base::OnceClosure delete_allowed_callback,
base::OnceClosure delete_not_allowed_callback) {
NOTIMPLEMENTED();
OnConnectionError();
}
void ProtoChromePromptIPC::InitializeImpl() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(State::kUninitialized, state_);
state_ = State::kWaitingForScanResults;
// Initialize communication with chrome by sending the version.
WriteByValue(kVersion);
}
void ProtoChromePromptIPC::RunPromptUserTask(
const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
PromptUserCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_NE(state_, State::kUninitialized);
DCHECK_NE(state_, State::kWaitingForResponseFromChrome);
// This can be true if any connection error occurred already in which case
// We don't not want to go forward with the prompting.
if (state_ == State::kDoneInteraction) {
return;
}
state_ = State::kWaitingForResponseFromChrome;
// If the contents of the message cannot be represented in a sane way avoid
// sending it on the wire. Returns a denied prompt since Chrome would run
// similar checks and deny it anyway.
// Build the prompt message.
chrome_cleaner::PromptUserRequest prompt_user_message;
for (const base::FilePath& file_to_delete : files_to_delete) {
std::string file_path_utf8;
if (!base::UTF16ToUTF8(file_to_delete.value().c_str(),
file_to_delete.value().size(), &file_path_utf8)) {
std::move(callback).Run(PromptAcceptance::DENIED);
return;
} else {
prompt_user_message.add_files_to_delete(file_path_utf8);
}
}
for (const base::string16& registry_key : registry_keys) {
std::string registry_key_utf8;
if (!base::UTF16ToUTF8(registry_key.c_str(), registry_key.size(),
&registry_key_utf8)) {
std::move(callback).Run(PromptAcceptance::DENIED);
return;
} else {
prompt_user_message.add_registry_keys(registry_key_utf8);
}
}
for (const base::string16& extension_id : extension_ids) {
std::string extension_id_utf8;
if (!base::UTF16ToUTF8(extension_id.c_str(), extension_id.size(),
&extension_id_utf8)) {
std::move(callback).Run(PromptAcceptance::DENIED);
return;
} else {
prompt_user_message.add_extension_ids(extension_id_utf8);
}
}
// This is the top-level message that Chrome is expecting.
ChromePromptRequest chrome_prompt_request;
*chrome_prompt_request.mutable_prompt_user() = prompt_user_message;
std::string request_content;
DCHECK(chrome_prompt_request.SerializeToString(&request_content));
SendBuffer(request_content);
// Sending the request can cause communication errors. If any happened don't
// bother waiting for a response.
if (state_ == State::kDoneInteraction) {
return;
}
// Receive the response from Chrome.
PromptAcceptance prompt_acceptance = WaitForPromptAcceptance();
if (state_ == State::kDoneInteraction) {
return;
}
// Send a message confirming to Chrome that the communication is over.
chrome_cleaner::CloseConnectionRequest close_connection_request;
chrome_prompt_request = ChromePromptRequest();
*chrome_prompt_request.mutable_close_connection() = close_connection_request;
std::string response_content;
DCHECK(chrome_prompt_request.SerializeToString(&response_content));
SendBuffer(response_content);
if (state_ == State::kDoneInteraction) {
return;
}
// Invoke callback with the result.
std::move(callback).Run(prompt_acceptance);
state_ = State::kDoneInteraction;
}
void ProtoChromePromptIPC::OnConnectionError() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_NE(State::kUninitialized, state_);
DCHECK_NE(State::kDoneInteraction, state_);
if (error_handler_) {
error_handler_->OnConnectionClosed();
}
state_ = State::kDoneInteraction;
}
void ProtoChromePromptIPC::SendBuffer(const std::string& request_content) {
DCHECK_NE(State::kDoneInteraction, state_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Write the message size.
const uint32_t kMessageLength = request_content.size();
WriteByValue(kMessageLength);
// Writing the message length failed. Do not send body.
if (state_ == State::kDoneInteraction) {
return;
}
// Write the message content.
WriteByPointer(request_content.data(), kMessageLength);
}
ProtoChromePromptIPC::PromptAcceptance
ProtoChromePromptIPC::WaitForPromptAcceptance() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK_EQ(State::kWaitingForResponseFromChrome, state_);
// On any error condition, invoke the error handler.
base::ScopedClosureRunner call_connection_closed(base::BindOnce(
&ProtoChromePromptIPC::OnConnectionError, base::Unretained(this)));
// Read the response length.
DWORD bytes_read = 0;
uint32_t response_length = 0;
if (!::ReadFile(response_read_handle_.Get(), &response_length,
sizeof(response_length), &bytes_read, nullptr)) {
PLOG(ERROR) << "Reading the prompt acceptance message length failed.";
return PromptAcceptance::DENIED;
}
if (bytes_read != sizeof(response_length)) {
PLOG(ERROR) << "Short read on the prompt acceptance message length.";
return PromptAcceptance::DENIED;
}
if (response_length == 0 || response_length > kMaxMessageLength) {
PLOG(ERROR) << "Invalid message length received: " << response_length;
return PromptAcceptance::DENIED;
}
// Read the response.
std::string response_content;
if (!::ReadFile(response_read_handle_.Get(),
base::WriteInto(&response_content, response_length + 1),
response_length, &bytes_read, nullptr)) {
PLOG(ERROR) << "Reading the prompt acceptance message failed";
return PromptAcceptance::DENIED;
}
if (bytes_read != response_length) {
PLOG(ERROR) << "Short read on the prompt acceptance message.";
return PromptAcceptance::DENIED;
}
chrome_cleaner::PromptUserResponse response;
if (!response.ParseFromString(response_content)) {
LOG(ERROR) << "Parsing of prompt acceptance failed.";
return PromptAcceptance::DENIED;
}
// Successful execution.
call_connection_closed.ReplaceClosure(base::DoNothing());
return static_cast<PromptAcceptance>(response.prompt_acceptance());
}
} // namespace chrome_cleaner
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_CHROME_CLEANER_IPC_PROTO_CHROME_PROMPT_IPC_H_
#define CHROME_CHROME_CLEANER_IPC_PROTO_CHROME_PROMPT_IPC_H_
#include <windows.h>
#include "base/sequenced_task_runner.h"
#include "base/task/post_task.h"
#include "base/win/scoped_handle.h"
#include "chrome/chrome_cleaner/ipc/chrome_prompt_ipc.h"
#include "components/chrome_cleaner/public/proto/chrome_prompt.pb.h"
namespace chrome_cleaner {
class ProtoChromePromptIPC : public ChromePromptIPC {
public:
static constexpr uint32_t kMaxMessageLength = 1 * 1024 * 1024; // 1M bytes
// Currently some mojom types are used to provide as drop-in replacement
// for the existing mojo based implementation. Since they are very simple
// they will stay essentially identical once the PromptAcceptance enum is
// replaced with a hand rolled one.
using PromptAcceptance = mojom::PromptAcceptance;
using PromptUserCallback = base::OnceCallback<void(PromptAcceptance)>;
using DisableExtensionsCallback = base::OnceCallback<void(bool)>;
ProtoChromePromptIPC(base::win::ScopedHandle response_read_handle,
base::win::ScopedHandle request_write_handle);
~ProtoChromePromptIPC() override;
void Initialize(ErrorHandler* error_handler) override;
void PostPromptUserTask(const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
PromptUserCallback callback) override;
void PostDisableExtensionsTask(
const std::vector<base::string16>& extension_ids,
DisableExtensionsCallback callback) override;
void TryDeleteExtensions(
base::OnceClosure delete_allowed_callback,
base::OnceClosure delete_not_allowed_callback) override;
private:
// Implements the initialization that needs to happen on the task_runner
// sequence.
void InitializeImpl();
void RunPromptUserTask(const std::vector<base::FilePath>& files_to_delete,
const std::vector<base::string16>& registry_keys,
const std::vector<base::string16>& extension_ids,
PromptUserCallback callback);
// Invokes error_handler_->OnConnectionClosed() and updates state_. This
// should not be called more than once.
void OnConnectionError();
void SendBuffer(const std::string& request_content);
PromptAcceptance WaitForPromptAcceptance();
template <typename T>
void WriteByValue(T value) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DWORD bytes_written = 0;
if (!::WriteFile(request_write_handle_.Get(), &value, sizeof(value),
&bytes_written, nullptr)) {
PLOG(ERROR) << "Writing a message to the pipe failed.";
OnConnectionError();
return;
}
if (bytes_written != sizeof(value)) {
LOG(ERROR) << "Incorrect number of bytes written to the pipe. Should be: "
<< sizeof(value) << " but is :" << bytes_written;
OnConnectionError();
}
}
template <typename T>
void WriteByPointer(const T* ptr, uint32_t size) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DWORD bytes_written = 0;
if (!::WriteFile(request_write_handle_.Get(), ptr, size, &bytes_written,
nullptr)) {
PLOG(ERROR) << "Writing a message to the pipe failed.";
OnConnectionError();
return;
}
if (bytes_written != size) {
LOG(ERROR) << "Incorrect number of bytes written to the pipe. Should be: "
<< size << " but is :" << bytes_written;
OnConnectionError();
}
}
base::win::ScopedHandle response_read_handle_;
base::win::ScopedHandle request_write_handle_;
scoped_refptr<base::SequencedTaskRunner> task_runner_ =
base::CreateSequencedTaskRunner({base::MayBlock()});
};
} // namespace chrome_cleaner
#endif // CHROME_CHROME_CLEANER_IPC_PROTO_CHROME_PROMPT_IPC_H_
This diff is collapsed.
...@@ -3864,8 +3864,7 @@ test("unit_tests") { ...@@ -3864,8 +3864,7 @@ test("unit_tests") {
"//ui/native_theme:test_support", "//ui/native_theme:test_support",
] ]
if (is_win) { if (is_win) {
deps += deps += [ "//components/chrome_cleaner/public/proto:test_only_proto" ]
[ "//chrome/browser/safe_browsing/chrome_cleaner:test_only_proto" ]
} }
if (is_mac) { if (is_mac) {
deps += [ ":firefox_importer_interface" ] deps += [ ":firefox_importer_interface" ]
......
...@@ -9,3 +9,11 @@ proto_library("proto") { ...@@ -9,3 +9,11 @@ proto_library("proto") {
"chrome_prompt.proto", "chrome_prompt.proto",
] ]
} }
proto_library("test_only_proto") {
testonly = true
generate_python = false
sources = [
"chrome_prompt_for_tests.proto",
]
}
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
// contains added fields to validate that they do not break the parsing. This is // contains added fields to validate that they do not break the parsing. This is
// to simulate an outdated Chrome handling messages from an updated cleaner. // to simulate an outdated Chrome handling messages from an updated cleaner.
// This file contains minimal comments for descriptions of non-test // This file contains minimal comments. For descriptions of non-test
// fields/messages please refer to the original file. // fields/messages please refer to the original file.
syntax = "proto2"; syntax = "proto2";
...@@ -35,6 +35,7 @@ message PromptUserResponse { ...@@ -35,6 +35,7 @@ message PromptUserResponse {
ACCEPTED_WITH_LOGS = 1; ACCEPTED_WITH_LOGS = 1;
ACCEPTED_WITHOUT_LOGS = 2; ACCEPTED_WITHOUT_LOGS = 2;
DENIED = 3; DENIED = 3;
FOR_TESTS_ONLY = 255;
}; };
optional PromptAcceptance prompt_acceptance = 1 [default = UNSPECIFIED]; optional PromptAcceptance prompt_acceptance = 1 [default = UNSPECIFIED];
......
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