Revert "Reland "Implement ProtoChromePromptIPC.""

This reverts commit 5805ac7f.

Reason for revert: breaking Windows compile:

In file included from ../..\base/task/lazy_task_runner.h:17:
../..\base/task/task_traits.h(263,5): error: static_assert failed due to requirement 'has_thread_pool ^ has_extension' "Traits must explicitly specify a destination (e.g. ThreadPool or a named thread like BrowserThread)"
    static_assert(
    ^
../..\chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc.h(108,39): note: in instantiation of function template specialization 'base::TaskTraits::TaskTraits<base::MayBlock, void>' requested here
      base::CreateSequencedTaskRunner({base::MayBlock()});
                                      ^
1 error generated.

Original change's description:
> 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/+/1759154
> Reviewed-by: Joe Mason <joenotcharles@google.com>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Joe Mason <joenotcharles@google.com>
> Cr-Commit-Position: refs/heads/master@{#689074}

TBR=joenotcharles@google.com,wfh@chromium.org,olivierli@chromium.org

Change-Id: I42f06162b45d41212816410027153f89f95a28f1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 969139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764124Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689151}

chrome/browser/safe_browsing/chrome_cleaner/BUILD.gn

@@ -5,6 +5,14 @@
 import("//build/config/jumbo.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") {
   sources = [
     "chrome_cleaner_controller_win.cc",

chrome/browser/safe_browsing/chrome_cleaner/chrome_prompt_channel_win_unittest.cc

@@ -28,8 +28,8 @@
 #include "base/win/scoped_handle.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_for_tests.pb.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/gtest/include/gtest/gtest.h"
 

components/chrome_cleaner/public/proto/chrome_prompt_for_tests.proto → chrome/browser/safe_browsing/chrome_cleaner/chrome_prompt_for_tests.proto

@@ -6,7 +6,7 @@
 // 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.
 
-// 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.
 
 syntax = "proto2";
@@ -35,7 +35,6 @@ message PromptUserResponse {
     ACCEPTED_WITH_LOGS = 1;
     ACCEPTED_WITHOUT_LOGS = 2;
     DENIED = 3;
-    FOR_TESTS_ONLY = 255;
   };
 
   optional PromptAcceptance prompt_acceptance = 1 [default = UNSPECIFIED];

chrome/chrome_cleaner/ipc/BUILD.gn

@@ -24,18 +24,14 @@ source_set("chrome_prompt_ipc") {
     "chrome_prompt_ipc.h",
     "mojo_chrome_prompt_ipc.cc",
     "mojo_chrome_prompt_ipc.h",
-    "proto_chrome_prompt_ipc.cc",
-    "proto_chrome_prompt_ipc.h",
   ]
 
   deps = [
     ":mojo_task_runner",
     "//base",
     "//components/chrome_cleaner/public/interfaces",
-    "//components/chrome_cleaner/public/proto",
     "//mojo/public/cpp/platform",
     "//mojo/public/cpp/system",
-    "//third_party/protobuf:protobuf_lite",
   ]
 }
 
@@ -71,7 +67,6 @@ source_set("ipc_test_util") {
   ]
 
   deps = [
-    ":chrome_prompt_ipc",
     ":mojo_task_runner",
     "//base",
     "//base/test:test_support",
@@ -87,10 +82,9 @@ source_set("unittest_sources") {
   testonly = true
 
   sources = [
-    "mojo_chrome_prompt_ipc_unittest.cc",
+    "chrome_prompt_ipc_unittest.cc",
     "mojo_sandbox_hooks_unittest.cc",
     "mojo_task_runner_unittest.cc",
-    "proto_chrome_prompt_ipc_unittest.cc",
     "sandbox_unittest.cc",
   ]
 
@@ -106,10 +100,7 @@ source_set("unittest_sources") {
     "//chrome/chrome_cleaner/mojom:mojo_sandbox_hooks_test_interface",
     "//chrome/chrome_cleaner/os:common_os",
     "//chrome/chrome_cleaner/test:test_util",
-    "//components/chrome_cleaner/public/constants",
     "//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",
     "//mojo/core/embedder",
     "//sandbox/win:sandbox",

chrome/chrome_cleaner/ipc/chrome_prompt_ipc.h

@@ -85,6 +85,42 @@ class ChromePromptIPC {
     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;
   ErrorHandler* error_handler_ = nullptr;
 

chrome/chrome_cleaner/ipc/mojo_chrome_prompt_ipc_unittest.cc → chrome/chrome_cleaner/ipc/chrome_prompt_ipc_unittest.cc

@@ -87,7 +87,7 @@ struct TestConfig {
   ParentDisconnected expected_parent_disconnected;
 };
 
-// Class that lives in the parent process and handles that side of the IPC.
+// Parent process.
 class MockChromePrompt : public mojom::ChromePrompt {
  public:
   MockChromePrompt(TestConfig test_config, mojom::ChromePromptRequest request)
@@ -151,7 +151,6 @@ class ChromePromptIPCParentProcess : public ParentProcess {
     }
     if (test_config.with_registry_keys)
       AppendSwitch(kIncludeRegistryKeysSwitch);
-
     AppendSwitch(kExpectedPromptResultSwitch,
                  base::NumberToString(
                      static_cast<int>(test_config.expected_prompt_acceptance)));
@@ -179,7 +178,26 @@ class ChromePromptIPCParentProcess : public ParentProcess {
   std::unique_ptr<MockChromePrompt> mock_chrome_prompt_;
 };
 
-// Class that lives in the child process and handles that side of the IPC.
+class ChromePromptIPCTestErrorHandler : public ChromePromptIPC::ErrorHandler {
+ 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 {
  public:
   explicit ChromePromptIPCChildProcess(
@@ -279,7 +297,6 @@ MULTIPROCESS_TEST_MAIN(ChromePromptIPCClientMain) {
   auto child_process =
       base::MakeRefCounted<ChromePromptIPCChildProcess>(mojo_task_runner);
   base::RunLoop on_done_run_loop;
-
   // 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
   // exit; in the latter, it will break |on_done_run_loop|, which will be

chrome/chrome_cleaner/ipc/ipc_test_util.cc

@@ -63,14 +63,6 @@ class MojoSandboxSetupHooks : public SandboxSetupHooks {
   SandboxedParentProcess* parent_process_;
 };
 
-}  // namespace
-
-namespace internal {
-
-base::FilePath::StringPieceType GetLogPathSuffix() {
-  return kIPCTestUtilLogSuffix;
-}
-
 base::FilePath GetLogPath() {
   return ScopedLogging::GetLogFilePath(kIPCTestUtilLogSuffix);
 }
@@ -115,7 +107,7 @@ void PrintChildProcessLogs() {
   }
 }
 
-}  // namespace internal
+}  // namespace
 
 ParentProcess::ParentProcess(scoped_refptr<MojoTaskRunner> mojo_task_runner)
     : command_line_(base::GetMultiProcessTestChildBaseCommandLine()),
@@ -174,7 +166,7 @@ bool ParentProcess::LaunchConnectedChildProcess(
     const std::string& child_main_function,
     base::TimeDelta timeout,
     int32_t* exit_code) {
-  if (!internal::DeleteChildProcessLogs())
+  if (!DeleteChildProcessLogs())
     return false;
 
   if (!PrepareAndLaunchTestChildProcess(child_main_function))
@@ -190,7 +182,7 @@ bool ParentProcess::LaunchConnectedChildProcess(
   DestroyImplOnIPCThread();
 
   if (!success || *exit_code != 0)
-    internal::PrintChildProcessLogs();
+    PrintChildProcessLogs();
 
   return success;
 }
@@ -297,20 +289,4 @@ std::string ChildProcess::mojo_pipe_token() const {
   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

chrome/chrome_cleaner/ipc/ipc_test_util.h

@@ -14,7 +14,6 @@
 #include "base/process/launch.h"
 #include "base/process/process.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/logging/scoped_logging.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
@@ -127,27 +126,6 @@ class ChildProcess : public base::RefCountedThreadSafe<ChildProcess> {
   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
 
 #endif  // CHROME_CHROME_CLEANER_IPC_IPC_TEST_UTIL_H_

chrome/chrome_cleaner/ipc/mock_chrome_prompt_ipc.cc

@@ -8,7 +8,8 @@
 
 namespace chrome_cleaner {
 
-MockChromePromptIPC::MockChromePromptIPC() = default;
+MockChromePromptIPC::MockChromePromptIPC()
+    : MojoChromePromptIPC(std::string(), nullptr) {}
 
 MockChromePromptIPC::~MockChromePromptIPC() = default;
 
@@ -21,10 +22,4 @@ void MockChromePromptIPC::PostPromptUserTask(
                          &callback);
 }
 
-void MockChromePromptIPC::PostDisableExtensionsTask(
-    const std::vector<base::string16>& extension_ids,
-    mojom::ChromePrompt::DisableExtensionsCallback callback) {
-  MockPostDisableExtensionsTask(extension_ids, &callback);
-}
-
 }  // namespace chrome_cleaner

chrome/chrome_cleaner/ipc/mock_chrome_prompt_ipc.h

@@ -8,13 +8,13 @@
 #include <memory>
 #include <vector>
 
-#include "chrome/chrome_cleaner/ipc/chrome_prompt_ipc.h"
+#include "chrome/chrome_cleaner/ipc/mojo_chrome_prompt_ipc.h"
 #include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace chrome_cleaner {
 
-class MockChromePromptIPC : public ChromePromptIPC {
+class MockChromePromptIPC : public MojoChromePromptIPC {
  public:
   MockChromePromptIPC();
   ~MockChromePromptIPC() override;
@@ -25,26 +25,19 @@ class MockChromePromptIPC : public ChromePromptIPC {
                void(base::OnceClosure delete_allowed_callback,
                     base::OnceClosure delete_not_allowed_callback));
 
-  // Workaround for GMock's limitation, in which MOCK_METHOD* doesn't
-  // accept base::OnceCallback parameters. Will forward any calls to
-  // MockPost*() and pass along a raw pointer for |callback|.
+  // Workaround for GMock's limitation, in which MOCK_METHOD* doesn't accept
+  // base::OnceCallback parameters. Will forward any calls to
+  // MockPostPromptUserTask() and pass along a raw pointer for |callback|.
   void PostPromptUserTask(
       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) override;
-  void PostDisableExtensionsTask(
-      const std::vector<base::string16>& extension_ids,
-      mojom::ChromePrompt::DisableExtensionsCallback callback) override;
-
   MOCK_METHOD4(MockPostPromptUserTask,
                void(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));
-  MOCK_METHOD2(MockPostDisableExtensionsTask,
-               void(const std::vector<base::string16>& extension_ids,
-                    mojom::ChromePrompt::DisableExtensionsCallback* callback));
 };
 
 }  // namespace chrome_cleaner

chrome/chrome_cleaner/ipc/mojo_chrome_prompt_ipc.h

 // Copyright 2018 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_MOJO_CHROME_PROMPT_IPC_H_
 #define CHROME_CHROME_CLEANER_IPC_MOJO_CHROME_PROMPT_IPC_H_
 
@@ -86,40 +87,41 @@ class MojoChromePromptIPC : public ChromePromptIPC {
 
   // Runs |chrome_prompt_service_->PromptUser()|. Must be called on the IPC
   // thread.
-  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);
+  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) override;
 
   void RunDisableExtensionsTask(
       const std::vector<base::string16>& extension_ids,
-      mojom::ChromePrompt::DisableExtensionsCallback callback);
+      mojom::ChromePrompt::DisableExtensionsCallback callback) override;
 
   // Callback for ChromePrompt::PromptUser, internal state must be
   // State::kWaitingForResponseFromChrome. Invokes callback(prompt_acceptance)
   // and transitions to state State::kDoneInteraction.
   void OnChromeResponseReceived(
       mojom::ChromePrompt::PromptUserCallback callback,
-      mojom::PromptAcceptance prompt_acceptance);
+      mojom::PromptAcceptance prompt_acceptance) override;
 
   // Callback for ChromePrompt::DisableExtensions, internal state must be
   // State::kDoneInteraction. Invokes callback(extensions_deleted_callback).
   void OnChromeResponseReceivedExtensions(
       mojom::ChromePrompt::DisableExtensionsCallback callback,
-      bool extensions_deleted_callback);
+      bool extensions_deleted_callback) override;
 
   // Connection error handler. Invokes either
   // error_handler_->OnConnectionClosed() or
   // error_handler_->OnConnectionClosedAfterDone(), depending on the internal
   // state.
-  void OnConnectionError();
+  void OnConnectionError() override;
 
   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);
+      uint32_t version) override;
 
  private:
   scoped_refptr<MojoTaskRunner> task_runner_;

chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc.cc

-// 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

chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc.h

-// 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_

chrome/test/BUILD.gn

@@ -3864,7 +3864,8 @@ test("unit_tests") {
       "//ui/native_theme:test_support",
     ]
     if (is_win) {
-      deps += [ "//components/chrome_cleaner/public/proto:test_only_proto" ]
+      deps +=
+          [ "//chrome/browser/safe_browsing/chrome_cleaner:test_only_proto" ]
     }
     if (is_mac) {
       deps += [ ":firefox_importer_interface" ]

components/chrome_cleaner/public/proto/BUILD.gn

@@ -9,11 +9,3 @@ proto_library("proto") {
     "chrome_prompt.proto",
   ]
 }
-
-proto_library("test_only_proto") {
-  testonly = true
-  generate_python = false
-  sources = [
-    "chrome_prompt_for_tests.proto",
-  ]
-}