Chrome Cleaner UI: reporter no longer uses mojo.

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.cc

Index: chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.cc
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.cc b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..02feac223bcebcd820d59f4f8342f20a02b48d27
--- /dev/null
+++ b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.cc
@@ -0,0 +1,207 @@
+// Copyright 2017 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/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.h"
+
+#include <utility>
+
+#include "base/base_paths.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/memory/ptr_util.h"
+#include "base/path_service.h"
+#include "base/process/launch.h"
+#include "base/process/process.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/task_scheduler/post_task.h"
+#include "chrome/browser/safe_browsing/chrome_cleaner/srt_client_info_win.h"
+#include "chrome/installer/util/install_util.h"
+#include "components/chrome_cleaner/public/constants/constants.h"
+#include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"
+#include "components/version_info/version_info.h"
+#include "content/public/browser/browser_thread.h"
+#include "mojo/edk/embedder/connection_params.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "mojo/edk/embedder/outgoing_broker_client_invitation.h"
+#include "mojo/edk/embedder/platform_channel_pair.h"
+#include "mojo/edk/embedder/transport_protocol.h"
+#include "mojo/public/cpp/system/message_pipe.h"
+
+using chrome_cleaner::mojom::ChromePrompt;
+using chrome_cleaner::mojom::ChromePromptRequest;
+using content::BrowserThread;
+
+namespace safe_browsing {
+
+void RunChromeCleanerAndReplyWithExitCode(
+    const base::FilePath& executable_path,
+    bool metrics_enabled,
+    bool sber_enabled,
+    ChromePromptImpl::OnPromptUserCallback on_prompt_user,
+    base::OnceClosure on_connection_closed,
+    base::OnceClosure on_connection_error,
+    base::Callback<void(int /*exit code*/)> done_callback) {
+  auto cleaner_runner = base::MakeShared<ChromeCleanerRunner>(
+      executable_path, metrics_enabled, sber_enabled, std::move(on_prompt_user),
+      std::move(on_connection_closed), std::move(on_connection_error));
+  auto launch_and_wait =
+      base::Bind(&ChromeCleanerRunner::LaunchAndWaitForExitOnBackgroundThread,
+                 cleaner_runner);
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE,
+      // LaunchAndWaitForExitOnBackgroundThread creates (MayBlock()) and joins
+      // (WithBaseSyncPrimitives()) a process.
+      {base::MayBlock(), base::WithBaseSyncPrimitives(),
+       base::TaskPriority::BACKGROUND,
+       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+      std::move(launch_and_wait), done_callback);
+}
+
+ChromeCleanerRunner::ChromeCleanerRunner(
+    const base::FilePath& executable_path,
+    bool metrics_enabled,
+    bool sber_enabled,
+    ChromePromptImpl::OnPromptUserCallback on_prompt_user,
+    base::OnceClosure on_connection_closed,
+    base::OnceClosure on_connection_error)
+    : command_line_(executable_path),
+      on_prompt_user_(std::move(on_prompt_user)),
+      on_connection_closed_(std::move(on_connection_closed)),
+      on_connection_error_(std::move(on_connection_error)) {
+  DCHECK(on_prompt_user_);
+  DCHECK(on_connection_closed_);
+  DCHECK(on_connection_error_);
+  DCHECK(!executable_path.empty());
+
+  // Add the non-IPC switches that should be passed to the Cleaner process.
+
+  // Add switches that pass information about this Chrome installation.

[ftirelo, 2017/05/18 18:27:50]

No kChromePromptSwitch?

[alito, 2017/05/18 23:01:17]

Well, the user hasn't been prompted at this point. We are just running to scan.
So I'm thinking we shouldn't add that flag when the cleaner is running in scan
mode. When the process relaunches itself in cleanup mode, then we know that the
user has accepted the prompt and can pass that there.

+  command_line_.AppendSwitchASCII(chrome_cleaner::kChromeVersionSwitch,
+                                  version_info::GetVersionNumber());
+  command_line_.AppendSwitchASCII(chrome_cleaner::kChromeChannelSwitch,
+                                  base::IntToString(ChannelAsInt()));
+  base::FilePath chrome_exe_path;
+  PathService::Get(base::FILE_EXE, &chrome_exe_path);
+  command_line_.AppendSwitchPath(chrome_cleaner::kChromeExePathSwitch,
+                                 chrome_exe_path);
+  if (!InstallUtil::IsPerUserInstall())
+    command_line_.AppendSwitch(chrome_cleaner::kChromeSystemInstallSwitch);
+
+  // If metrics is enabled, we can enable crash reporting in the Chrome Cleaner
+  // process.
+  if (metrics_enabled) {
+    command_line_.AppendSwitch(chrome_cleaner::kUmaUserSwitch);
+    command_line_.AppendSwitch(chrome_cleaner::kEnableCrashReportingSwitch);
+  }
+
+  // Enable losg upload for users who have opted into safe browsing extended
+  // reporting V2.
+  if (sber_enabled) {
+    command_line_.AppendSwitch(
+        chrome_cleaner::kExtendedSafeBrowsingEnabledSwitch);

[ftirelo, 2017/05/18 18:27:50]

As discussed, this can become kCleanerLoggingEnabled.

[alito, 2017/05/18 23:01:17]

Done.

+  }
+}
+
+int ChromeCleanerRunner::LaunchAndWaitForExitOnBackgroundThread() {
+  mojo::edk::OutgoingBrokerClientInvitation invitation;
+  std::string mojo_pipe_token = mojo::edk::GenerateRandomToken();
+  mojo::ScopedMessagePipeHandle mojo_pipe =
+      invitation.AttachMessagePipe(mojo_pipe_token);
+  command_line_.AppendSwitchASCII(chrome_cleaner::kChromeMojoPipeTokenSwitch,
+                                  mojo_pipe_token);
+  command_line_.AppendSwitchASCII(
+      chrome_cleaner::kExecutionModeSwitch,
+      base::IntToString(
+          static_cast<int>(chrome_cleaner::ExecutionMode::kScanning)));

[ftirelo, 2017/05/18 18:27:50]

Please add this in the method above with the other non-mojo flags.

[alito, 2017/05/18 23:01:17]

Done.

+
+  mojo::edk::PlatformChannelPair channel;
+  base::HandlesToInheritVector handles_to_inherit;
+  channel.PrepareToPassClientHandleToChildProcess(&command_line_,
+                                                  &handles_to_inherit);
+  base::LaunchOptions launch_options;
+  launch_options.handles_to_inherit = &handles_to_inherit;
+
+  base::Process cleaner_process =
+      base::LaunchProcess(command_line_, launch_options);
+
+  if (!cleaner_process.IsValid())
+    return kNotLaunchedExitCode;
+
+  // ChromePromptImpl tasks will need to run on the IO thread. There is no
+  // need to synchronize its creation, since the client end will wait for this
+  // initialization to be done before sending requests.
+  BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)
+      ->PostTask(FROM_HERE,
+                 base::BindOnce(&ChromeCleanerRunner::CreateChromePromptImpl,
+                                base::RetainedRef(this),
+                                chrome_cleaner::mojom::ChromePromptRequest(
+                                    std::move(mojo_pipe))));
+
+  // mojo::edk::ProcessErrorCallback on_connection_error =
+  //     base::Bind(&OnConnectionError);

[ftirelo, 2017/05/18 18:27:50]

Please delete.

[alito, 2017/05/18 23:01:17]

Done.

+  invitation.Send(
+      cleaner_process.Handle(),
+      mojo::edk::ConnectionParams(mojo::edk::TransportProtocol::kLegacy,
+                                  channel.PassServerHandle()),
+      base::Bind(&ChromeCleanerRunner::OnConnectionError,
+                 base::RetainedRef(this)));
+
+  int exit_code = kNotLaunchedExitCode;
+  bool success = cleaner_process.WaitForExit(&exit_code);
+  DCHECK(success);
+  BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)
+      ->PostTask(FROM_HERE,
+                 base::Bind(&ChromeCleanerRunner::ReleaseChromePromptImpl,
+                            base::RetainedRef(this)));
+  return exit_code;
+}
+
+ChromeCleanerRunner::~ChromeCleanerRunner() = default;
+
+void ChromeCleanerRunner::CreateChromePromptImpl(
+    ChromePromptRequest chrome_prompt_request) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);

[ftirelo, 2017/05/18 18:27:50]

Worth DCHECK'ing that the feature is enabled?

[alito, 2017/05/18 23:01:17]

Since this class should only be used if the feature is enabled, I added a DCHECK
in the constructor.

+  DCHECK(!chrome_prompt_impl_);
+
+  chrome_prompt_impl_ = base::MakeUnique<ChromePromptImpl>(
+      std::move(chrome_prompt_request),
+      base::Bind(&ChromeCleanerRunner::OnConnectionClosed,
+                 base::RetainedRef(this)),
+      base::Bind(&ChromeCleanerRunner::OnPromptUser, base::RetainedRef(this)));
+}
+
+void ChromeCleanerRunner::ReleaseChromePromptImpl() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(chrome_prompt_impl_);
+
+  chrome_prompt_impl_.release();
+}
+
+void ChromeCleanerRunner::OnPromptUser(
+    std::unique_ptr<std::set<base::FilePath>> files_to_delete,
+    ChromePrompt::PromptUserCallback prompt_user_callback) {
+  if (on_prompt_user_) {
+    BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)
+        ->PostTask(FROM_HERE, base::Bind(std::move(on_prompt_user_),
+                                         base::Passed(&files_to_delete),
+                                         base::Passed(&prompt_user_callback)));
+  }
+}
+
+void ChromeCleanerRunner::OnConnectionClosed() {
+  if (on_connection_closed_) {
+    BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)
+        ->PostTask(FROM_HERE, std::move(on_connection_closed_));
+  }
+}
+
+void ChromeCleanerRunner::OnConnectionError(const std::string& error) {
+  if (on_connection_error_) {
+    BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)
+        ->PostTask(FROM_HERE, std::move(on_connection_error_));
+  }
+}
+
+}  // namespace safe_browsing