Adds error handling support for the SwReporter launcher.

chrome/browser/safe_browsing/srt_fetcher_win.cc

Index: chrome/browser/safe_browsing/srt_fetcher_win.cc
diff --git a/chrome/browser/safe_browsing/srt_fetcher_win.cc b/chrome/browser/safe_browsing/srt_fetcher_win.cc
index 6f2ee0f88e9f5b90d245d708ba646f5d6e69535e..7a6346cef62a2ce81ed3b08f80fb3671bf083a21 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_win.cc
@@ -35,7 +35,6 @@
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
-#include "chrome/browser/safe_browsing/srt_chrome_prompt_impl.h"
 #include "chrome/browser/safe_browsing/srt_client_info_win.h"
 #include "chrome/browser/safe_browsing/srt_global_error_win.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -591,8 +590,13 @@ class SwReporterProcess : public base::RefCountedThreadSafe<SwReporterProcess> {
   base::Process LaunchConnectedReporterProcess();
 
   // Starts a new instance of ChromePromptImpl to receive requests from the
-  // reporter. Must be run on the IO thread.
+  // reporter and establishes the mojo connection to it.
+  // Must be run on the IO thread.
   void CreateChromePromptImpl(
+      base::ProcessHandle process,
+      mojo::edk::ConnectionParams connection_params,
+      std::unique_ptr<mojo::edk::PendingProcessConnection>
+          pending_process_connection,
       chrome_cleaner::mojom::ChromePromptRequest chrome_prompt_request);
 
   // Releases the instance of ChromePromptImpl. Must be run on the IO thread.
@@ -620,7 +624,7 @@ int SwReporterProcess::LaunchAndWaitForExitOnBackgroundThread() {
 
   // This exit code is used to identify that a reporter run didn't happen, so
   // the result should be ignored and a rerun scheduled for the usual delay.
-  int exit_code = kReporterFailureExitCode;
+  int exit_code = kReporterNotLaunchedExitCode;
   UMAHistogramReporter uma(invocation_.suffix);
   if (reporter_process.IsValid()) {
     uma.RecordReporterStep(SW_REPORTER_START_EXECUTION);
@@ -644,10 +648,12 @@ void SwReporterProcess::OnReporterDone() {
 base::Process SwReporterProcess::LaunchConnectedReporterProcess() {
   DCHECK(base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature));
 
-  mojo::edk::PendingProcessConnection pending_process_connection;
+  std::unique_ptr<mojo::edk::PendingProcessConnection>
+      pending_process_connection =
+          base::MakeUnique<mojo::edk::PendingProcessConnection>();
   std::string mojo_pipe_token;
   mojo::ScopedMessagePipeHandle mojo_pipe =
-      pending_process_connection.CreateMessagePipe(&mojo_pipe_token);
+      pending_process_connection->CreateMessagePipe(&mojo_pipe_token);
   invocation_.command_line.AppendSwitchASCII(
       chrome_cleaner::kChromeMojoPipeTokenSwitch, mojo_pipe_token);
   invocation_.command_line.AppendSwitchASCII(
@@ -668,21 +674,21 @@ base::Process SwReporterProcess::LaunchConnectedReporterProcess() {
   if (!reporter_process.IsValid())
     return reporter_process;
 
-  pending_process_connection.Connect(
-      reporter_process.Handle(),
-      mojo::edk::ConnectionParams(channel.PassServerHandle()));
-
   chrome_cleaner::mojom::ChromePromptRequest chrome_prompt_request;
   chrome_prompt_request.Bind(std::move(mojo_pipe));
 
   // 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.
+  // It's safe to pass the process handle here

[Joe Mason, 2017/04/24 14:53:48]

Nit: End this line with a period, and either put it on the same line as the
previous sentence or add a blank line to separate paragraphs.

[ftirelo, 2017/04/24 15:47:46]

This was still wip, not needed anymore.

   BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)
       ->PostTask(FROM_HERE,
-                 base::BindOnce(&SwReporterProcess::CreateChromePromptImpl,
-                                base::RetainedRef(this),
-                                std::move(chrome_prompt_request)));
+                 base::BindOnce(
+                     &SwReporterProcess::CreateChromePromptImpl,
+                     base::RetainedRef(this), reporter_process.Handle(),
+                     mojo::edk::ConnectionParams(channel.PassServerHandle()),
+                     std::move(pending_process_connection),
+                     std::move(chrome_prompt_request)));
 
   return reporter_process;
 }
@@ -696,12 +702,24 @@ base::Process SwReporterProcess::LaunchReporterProcess(
 }
 
 void SwReporterProcess::CreateChromePromptImpl(
+    base::ProcessHandle process,
+    mojo::edk::ConnectionParams connection_params,
+    std::unique_ptr<mojo::edk::PendingProcessConnection>
+        pending_process_connection,
     chrome_cleaner::mojom::ChromePromptRequest chrome_prompt_request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature));
 
-  chrome_prompt_impl_ =
-      base::MakeUnique<ChromePromptImpl>(std::move(chrome_prompt_request));
+  chrome_prompt_impl_ = g_testing_delegate_
+                            ? g_testing_delegate_->CreateChromePromptImpl(
+                                  std::move(chrome_prompt_request))
+                            : base::MakeUnique<ChromePromptImpl>(
+                                  std::move(chrome_prompt_request));
+
+  pending_process_connection->Connect(
+      process, std::move(connection_params),
+      base::Bind(&ChromePromptImpl::OnConnectionError,
+                 base::Unretained(chrome_prompt_impl_.get())));

[Joe Mason, 2017/04/24 14:53:48]

Because I know grt will ask: please add a comment explaining why Unretained is
safe here.

[ftirelo, 2017/04/24 15:47:46]

Obsolete.

[grt (UTC plus 2), 2017/04/25 07:06:39]

Impact! Influence! ;-)

 }
 
 void SwReporterProcess::ReleaseChromePromptImpl() {
@@ -934,7 +952,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
     base::TimeDelta reporter_running_time = now - reporter_start_time;
 
     // Don't continue the current queue of reporters if one failed to launch.
-    if (exit_code == kReporterFailureExitCode)
+    if (exit_code == kReporterNotLaunchedExitCode)
       current_invocations_ = SwReporterQueue();
 
     // As soon as we're not running this queue, schedule the next overall queue
@@ -954,7 +972,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
     // code itself doesn't need to be logged in this case because
     // SW_REPORTER_FAILED_TO_START is logged in
     // |LaunchAndWaitForExitOnBackgroundThread|.)
-    if (exit_code == kReporterFailureExitCode)
+    if (exit_code == kReporterNotLaunchedExitCode)
       return;
 
     const auto& finished_invocation = sw_reporter_process->invocation();