Basic IPC communication between Chrome and the SW Reporter.

chrome/browser/safe_browsing/srt_fetcher_win.cc

 // Copyright 2015 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/srt_fetcher_win.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 18 matching lines @@
 #include "base/task_runner_util.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/time/time.h"
 #include "base/version.h"
 #include "base/win/registry.h"
 #include "chrome/browser/browser_process.h"
 #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"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "components/component_updater/pref_names.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/net/variations_http_headers.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/pending_process_connection.h"
+#include "mojo/edk/embedder/platform_channel_pair.h"
+#include "mojo/public/cpp/system/message_pipe.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "third_party/pipa/src/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"
 
 using content::BrowserThread;
 
 namespace safe_browsing {
 
 // TODO(b/647763) Change the registry key to properly handle cases when the user
 // runs Google Chrome stable alongside Google Chrome SxS.
 const wchar_t kSoftwareRemovalToolRegistryKey[] =
     L"Software\\Google\\Software Removal Tool";
 
 const wchar_t kCleanerSubKey[] = L"Cleaner";
 
 const wchar_t kEndTimeValueName[] = L"EndTime";
 const wchar_t kStartTimeValueName[] = L"StartTime";
 
+const base::Feature kInBrowserCleanerUIFeature{
+    "InBrowserCleanerUI", base::FEATURE_DISABLED_BY_DEFAULT};
+
 namespace {
 
 // Used to send UMA information about missing start and end time registry
 // values for the reporter. Replicated in the histograms.xml file, so the order
 // MUST NOT CHANGE.
 enum SwReporterRunningTimeRegistryError {
   REPORTER_RUNNING_TIME_ERROR_NO_ERROR = 0,
   REPORTER_RUNNING_TIME_ERROR_REGISTRY_KEY_INVALID = 1,
   REPORTER_RUNNING_TIME_ERROR_MISSING_START_TIME = 2,
   REPORTER_RUNNING_TIME_ERROR_MISSING_END_TIME = 3,
@@ skipping 75 matching lines @@
     "SoftwareReporter.LogsUploadResultRegistryError";
 const char kExitCodeMetricName[] = "SoftwareReporter.ExitCodeFromRegistry";
 const char kEngineErrorCodeMetricName[] = "SoftwareReporter.EngineErrorCode";
 
 // The max value for histogram SoftwareReporter.LogsUploadResult, which is used
 // to send UMA information about the result of Software Reporter's attempt to
 // upload logs, when logs are enabled. This value must be consistent with the
 // SoftwareReporterLogsUploadResult enum defined in the histograms.xml file.
 const int kSwReporterLogsUploadResultMax = 30;
 
+constexpr char kChromeMojoPipeTokenSwitch[] = "chrome-mojo-pipe-token";
+
+// DO NOT SUBMIT. This is for debugging only and will be removed before landing
+// this CL.
+constexpr bool kDebugOnlyForceReporterRun = true;
+
 // Reports metrics about the software reporter via UMA (and sometimes Rappor).
 class UMAHistogramReporter {
  public:
   UMAHistogramReporter() : UMAHistogramReporter(std::string()) {}
 
   explicit UMAHistogramReporter(const std::string& suffix)
       : suffix_(suffix),
         registry_key_(suffix.empty() ? kSoftwareRemovalToolRegistryKey
                                      : base::StringPrintf(
                                            L"%ls\\%ls",
@@ skipping 368 matching lines @@
       if (error->GetBubbleView()) {
         show_bubble = false;
         break;
       }
     }
   }
   if (show_bubble)
     global_error->ShowBubbleView(browser);
 }
 
-// This function is called from a worker thread to launch the SwReporter and
-// wait for termination to collect its exit code. This task could be
-// interrupted by a shutdown at any time, so it shouldn't depend on anything
-// external that could be shut down beforehand.
-int LaunchAndWaitForExit(const SwReporterInvocation& invocation) {
+// Class responsible for launching the reporter process and waiting for its
+// completion. If feature InBrowserCleanerUI is enabled, this object will also
+// be responsible for starting the ChromePromptImpl object in the IO thread and
+// controlling its lifetime.
+//
+// Expected lifecycle of a SwReporterProcess:
+//  - created in the UI thread before the reporter process launch is posted

[grt (UTC plus 2), 2017/03/31 11:32:33]

nit: "on" a thread rather than "in" (prepositions, how do they work? :-) )

[ftirelo, 2017/03/31 16:37:37]

Done :-)

(I'm afraid that now I don't know how to properly use prepositions anymore even
in Portuguese :-) )

+//    (method ScheduleNextInvocation);
+//  - deleted in the UI thread once ReporterDone() finishes (the method is
+//    called after the reporter process exits).
+//
+// If feature InBrowserCleanerUI feature is enabled, the following tasks will
+// be posted in sequence to the IO Thread and will retain the SwReporterProcess
+// object:
+//  - creation of a ChromePromptImpl object right the reporter process is

[Joe Mason, 2017/03/31 08:06:23]

Looks like there are some missing words in this sentence.

[ftirelo, 2017/03/31 16:37:37]

Done.

+//    launched (that object will be responsible for handling IPC requests from
+//    the reporter process);
+//  - deletion of the ChromePromptImpl object on ReporterDone().
+// As a consequence, the SwReporterProcess object can outlive ReporterDone()
+// and will only be deleted after the ChromePromptImpl object is released in
+// the IO thread.
+class SwReporterProcess : public base::RefCounted<SwReporterProcess> {
+ public:
+  explicit SwReporterProcess(const SwReporterInvocation& invocation)
+      : invocation_(invocation) {}
+  ~SwReporterProcess() = default;
+
+  // This function is called from a worker thread to launch the SwReporter and
+  // wait for termination to collect its exit code. This task could be
+  // interrupted by a shutdown at any time, so it shouldn't depend on anything
+  // external that could be shut down beforehand.
+  int LaunchAndWaitForExitOnBackgroundThread();
+
+  // Schedules to delete the instance of ChromePromptImpl in the IO thread.
+  void ReleaseChromePromptImpl();
+
+  const SwReporterInvocation& invocation() const { return invocation_; }
+
+ private:
+  // Starts a new IPC service implementing the ChromePrompt interface and
+  // launches a new reporter process that can connect to the IPC.
+  base::Process LaunchConnectedReporterProcess();
+
+  // Starts a new instance of ChromePromptImpl to receive requests from the
+  // reporter. Must be run in the IO thread.
+  void CreateChromePromptImpl(mojo::ScopedMessagePipeHandle my_pipe);
+
+  // The invocation for the current reporter process.
+  SwReporterInvocation invocation_;
+
+  // Implementation of the ChromePrompt service to be used by the current
+  // reporter process.

[grt (UTC plus 2), 2017/03/31 11:32:33]

please document that this member may only be accessed on the IO thread.

[ftirelo, 2017/03/31 16:37:37]

Done.

+  std::unique_ptr<ChromePromptImpl> chrome_prompt_impl_;
+};
+
+int SwReporterProcess::LaunchAndWaitForExitOnBackgroundThread() {
   if (g_testing_delegate_)
-    return g_testing_delegate_->LaunchReporter(invocation);
+    return g_testing_delegate_->LaunchReporter(invocation_);
+
   base::Process reporter_process =
-      base::LaunchProcess(invocation.command_line, base::LaunchOptions());
+      base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature)
+          ? LaunchConnectedReporterProcess()
+          : base::LaunchProcess(invocation_.command_line,
+                                base::LaunchOptions());
+
   // 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;
-  UMAHistogramReporter uma(invocation.suffix);
+  UMAHistogramReporter uma(invocation_.suffix);
   if (reporter_process.IsValid()) {
     uma.RecordReporterStep(SW_REPORTER_START_EXECUTION);
     bool success = reporter_process.WaitForExit(&exit_code);
     DCHECK(success);
   } else {
     uma.RecordReporterStep(SW_REPORTER_FAILED_TO_START);
   }
   return exit_code;
 }
 
+base::Process SwReporterProcess::LaunchConnectedReporterProcess() {
+  DCHECK(base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature));
+
+  mojo::edk::PendingProcessConnection pending_process_connection;
+  std::string mojo_pipe_token;
+  mojo::ScopedMessagePipeHandle mojo_pipe =
+      pending_process_connection.CreateMessagePipe(&mojo_pipe_token);
+  invocation_.command_line.AppendSwitchASCII(kChromeMojoPipeTokenSwitch,
+                                             mojo_pipe_token);
+
+  mojo::edk::PlatformChannelPair channel;
+  base::HandlesToInheritVector handles_to_inherit;
+  channel.PrepareToPassClientHandleToChildProcess(&invocation_.command_line,
+                                                  &handles_to_inherit);
+
+  base::LaunchOptions launch_options;
+  launch_options.handles_to_inherit = &handles_to_inherit;
+  base::Process reporter_process =
+      base::LaunchProcess(invocation_.command_line, launch_options);
+
+  if (!reporter_process.IsValid())
+    return reporter_process;
+
+  pending_process_connection.Connect(
+      reporter_process.Handle(),
+      mojo::edk::ConnectionParams(channel.PassServerHandle()));
+
+  // ChromePromptImpl tasks will need to run in 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::Bind(&SwReporterProcess::CreateChromePromptImpl, this,
+                            base::Passed(&mojo_pipe)));
+
+  return reporter_process;
+}
+
+void SwReporterProcess::CreateChromePromptImpl(
+    mojo::ScopedMessagePipeHandle mojo_pipe) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  chrome_cleaner::mojom::ChromePromptRequest chrome_prompt_request;
+  chrome_prompt_request.Bind(std::move(mojo_pipe));
+  chrome_prompt_impl_ =
+      base::MakeUnique<ChromePromptImpl>(std::move(chrome_prompt_request));
+}
+
+void SwReporterProcess::ReleaseChromePromptImpl() {

[grt (UTC plus 2), 2017/03/31 11:32:33]

nit: method definitions should be in the same order as their in-class
declarations.

[ftirelo, 2017/03/31 16:37:37]

It's private now, so keeping it here since it was declared after
CreateChromePromptImpl on the class.

+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature));
+
+  chrome_prompt_impl_.release();
+}
+
 }  // namespace
 
 void DisplaySRTPromptForTesting(const base::FilePath& download_path) {
   DisplaySRTPrompt(download_path);
 }
 
 // Class that will attempt to download the SRT, showing the SRT notification
 // bubble when the download operation is complete. Instances of SRTFetcher own
 // themselves, they will self-delete on completion of the network request when
 // OnURLFetchComplete is called.
@@ skipping 175 matching lines @@
   void ScheduleNextInvocation() {
     DCHECK(!current_invocations_.empty());
     auto next_invocation = current_invocations_.front();
     current_invocations_.pop();
 
     AppendInvocationSpecificSwitches(&next_invocation);
 
     base::TaskRunner* task_runner =
         g_testing_delegate_ ? g_testing_delegate_->BlockingTaskRunner()
                             : blocking_task_runner_.get();
-    base::PostTaskAndReplyWithResult(
-        task_runner, FROM_HERE,
-        base::Bind(&LaunchAndWaitForExit, next_invocation),
+    scoped_refptr<SwReporterProcess> sw_reporter_process =

[grt (UTC plus 2), 2017/03/31 11:32:33]

nit: this can become "auto sw_reporter_process" since the type is obvious from
make_scoped_refptr.

[ftirelo, 2017/03/31 16:37:37]

Done.

+        make_scoped_refptr(new SwReporterProcess(next_invocation));
+    auto launch_and_wait =
+        base::Bind(&SwReporterProcess::LaunchAndWaitForExitOnBackgroundThread,
+                   sw_reporter_process);
+    auto reporter_done =
         base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this), Now(),
-                   version_, next_invocation));
+                   version_, std::move(sw_reporter_process));
+    base::PostTaskAndReplyWithResult(task_runner, FROM_HERE,
+                                     std::move(launch_and_wait),
+                                     std::move(reporter_done));
   }
 
   // This method is called on the UI thread when an invocation of the reporter
   // has completed. This is run as a task posted from an interruptible worker
   // thread so should be resilient to unexpected shutdown.
   void ReporterDone(const base::Time& reporter_start_time,
                     const base::Version& version,
-                    const SwReporterInvocation& finished_invocation,
+                    scoped_refptr<SwReporterProcess> sw_reporter_process,
                     int exit_code) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+    if (base::FeatureList::IsEnabled(kInBrowserCleanerUIFeature)) {

[grt (UTC plus 2), 2017/03/31 11:32:33]

nit: putting this conditional out here seems to be exposing what is otherwise an
implementation detail out of SwReporterProcess. how about moving this into a
method that is blindly called here (OnReporterDone or something)?

[ftirelo, 2017/03/31 16:37:37]

Done.

+      BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)
+          ->PostTask(FROM_HERE,
+                     base::Bind(&SwReporterProcess::ReleaseChromePromptImpl,
+                                sw_reporter_process));
+    }
+
     base::Time now = Now();
     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)
       current_invocations_ = SwReporterQueue();
 
     // As soon as we're not running this queue, schedule the next overall queue
     // run after the regular delay. (If there was a failure it's not worth
     // retrying earlier, risking running too often if it always fails, since
     // not many users fail here.)
     if (current_invocations_.empty()) {
       base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
           base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
           base::TimeDelta::FromDays(days_between_reporter_runs_));
     } else {
       ScheduleNextInvocation();
     }
 
     // If the reporter failed to launch, do not process the results. (The exit
     // code itself doesn't need to be logged in this case because
-    // SW_REPORTER_FAILED_TO_START is logged in |LaunchAndWaitForExit|.)
+    // SW_REPORTER_FAILED_TO_START is logged in
+    // |LaunchAndWaitForExitOnBackgroundThread|.)
     if (exit_code == kReporterFailureExitCode)
       return;
 
+    auto finished_invocation = sw_reporter_process->invocation();
     UMAHistogramReporter uma(finished_invocation.suffix);
     uma.ReportVersion(version);
     uma.ReportExitCode(exit_code);
     uma.ReportEngineErrorCode();
     uma.ReportFoundUwS();
 
     PrefService* local_state = g_browser_process->local_state();
     if (local_state) {
       if (finished_invocation.BehaviourIsSupported(
               SwReporterInvocation::BEHAVIOUR_LOG_EXIT_CODE_TO_PREFS)) {
@@ skipping 62 matching lines @@
       RecordReporterStepHistogram(SW_REPORTER_RAN_DAILY);
     } else {
       days_between_reporter_runs_ = kDaysBetweenSuccessfulSwReporterRuns;
     }
     const base::Time now = Now();
     const base::Time last_time_triggered = base::Time::FromInternalValue(
         local_state->GetInt64(prefs::kSwReporterLastTimeTriggered));
     const base::Time next_trigger(
         last_time_triggered +
         base::TimeDelta::FromDays(days_between_reporter_runs_));
-    if (!pending_invocations_.empty() &&
-        (next_trigger <= now ||
-         // Also make sure the kSwReporterLastTimeTriggered value is not set in
-         // the future.
-         last_time_triggered > now)) {
+    if (kDebugOnlyForceReporterRun ||
+        (!pending_invocations_.empty() &&
+         (next_trigger <= now ||
+          // Also make sure the kSwReporterLastTimeTriggered value is not set in
+          // the future.
+          last_time_triggered > now))) {
       const base::Time last_time_sent_logs = base::Time::FromInternalValue(
           local_state->GetInt64(prefs::kSwReporterLastTimeSentReport));
       const base::Time next_time_send_logs =
           last_time_sent_logs +
           base::TimeDelta::FromDays(kDaysBetweenReporterLogsSent);
       // Send the logs for this whole queue of invocations if the last send is
       // in the future or if logs have been sent at least
       // |kSwReporterLastTimeSentReport| days ago. The former is intended as a
       // measure for failure recovery, in case the time in local state is
       // incorrectly set to the future.
@@ skipping 73 matching lines @@
   // The queue of invocations that are currently running.
   SwReporterQueue current_invocations_;
 
   // The invocations to run next time the SwReporter is run.
   SwReporterQueue pending_invocations_;
 
   base::Version version_;
 
   scoped_refptr<base::TaskRunner> blocking_task_runner_ =
       base::CreateTaskRunnerWithTraits(
-          // LaunchAndWaitForExit() creates (MayBlock()) and joins
-          // (WithBaseSyncPrimitives()) a process.
+          // LaunchAndWaitForExitOnBackgroundThread() creates (MayBlock()) and
+          // joins (WithBaseSyncPrimitives()) a process.
           base::TaskTraits()
               .WithShutdownBehavior(
                   base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
               .WithPriority(base::TaskPriority::BACKGROUND)
               .MayBlock()
               .WithBaseSyncPrimitives());
 
   // This value is used to identify how long to wait before starting a new run
   // of the reporter queue. It's initialized with the default value and may be
   // changed to a different value when a prompt is pending and the reporter
@@ skipping 64 matching lines @@
   return srt_cleaner_key.Open(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
                               KEY_QUERY_VALUE) == ERROR_SUCCESS &&
          srt_cleaner_key.GetValueCount() > 0;
 }
 
 void SetSwReporterTestingDelegate(SwReporterTestingDelegate* delegate) {
   g_testing_delegate_ = delegate;
 }
 
 }  // namespace safe_browsing