Add support for the ExperimentalSwReporterEngine field trial.

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 d46ed04482d957058e2e2d8c14b6c25a87122959..8a1fbce0f829bffee0c763a5605f5f5786564a77 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_win.cc
@@ -119,7 +119,9 @@ const char kMemoryUsedMetricName[] = "SoftwareReporter.MemoryUsed";
 // Reports metrics about the software reporter via UMA (and sometimes Rappor).
 class UMAHistogramReporter {
  public:
-  explicit UMAHistogramReporter(const std::string& suffix = std::string())
+  UMAHistogramReporter() : UMAHistogramReporter(std::string()) {}
+
+  explicit UMAHistogramReporter(const std::string& suffix)
       : suffix_(suffix),
         registry_key_(suffix.empty() ? kSoftwareRemovalToolRegistryKey
                                      : base::StringPrintf(
@@ -588,7 +590,7 @@ void MaybeFetchSRT(Browser* browser, const base::Version& reporter_version) {
 class ReporterRunner : public chrome::BrowserListObserver {
  public:
   // Starts the sequence of attempts to run the reporter.
-  static void Run(const SwReporterInvocation& invocation,
+  static void Run(const SwReporterQueue& invocations,

[grt (UTC plus 2), 2016/09/01 21:08:55]

take as value rather than constref?

[Joe Mason, 2016/09/02 02:24:30]

Done.

                   const base::Version& version,
                   scoped_refptr<base::TaskRunner> main_thread_task_runner,
                   scoped_refptr<base::TaskRunner> blocking_task_runner) {
@@ -597,11 +599,11 @@ class ReporterRunner : public chrome::BrowserListObserver {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     // There's nothing to do if the path and version of the reporter has not
     // changed, we just keep running the tasks that are running now.
-    if (instance_->invocation_ == invocation && instance_->version_.IsValid() &&
-        instance_->version_ == version)
+    if (instance_->invocations_ == invocations &&
+        instance_->version_.IsValid() && instance_->version_ == version)
       return;
 
-    instance_->invocation_ = invocation;
+    instance_->invocations_ = invocations;

[grt (UTC plus 2), 2016/09/01 21:08:55]

std::move here?

[Joe Mason, 2016/09/02 02:24:30]

Done.

     instance_->version_ = version;
     instance_->main_thread_task_runner_ = std::move(main_thread_task_runner);
     instance_->blocking_task_runner_ = std::move(blocking_task_runner);
@@ -626,12 +628,41 @@ class ReporterRunner : public chrome::BrowserListObserver {
     BrowserList::RemoveObserver(this);
   }
 
+  // Launches the command line at the head of the queue.
+  void LaunchNextInvocation(std::unique_ptr<SwReporterQueue> invocations) {
+    DCHECK(invocations);
+    DCHECK(!invocations->empty());
+    auto next_invocation = invocations->front();
+    invocations->pop();
+
+    if (g_testing_delegate_)
+      g_testing_delegate_->NotifyLaunchReady();
+
+    // It's OK to simply |PostTaskAndReplyWithResult| so that
+    // |LaunchAndWaitForExit| doesn't need to access |main_thread_task_runner_|
+    // since the callback is not delayed and the test task runner won't need to
+    // force it.
+    base::PostTaskAndReplyWithResult(
+        blocking_task_runner_.get(), FROM_HERE,
+
+        // Send the head of the queue to the launch callback.
+        base::Bind(&LaunchAndWaitForExit, next_invocation),
+
+        // Send the tail of the queue to the next callback, which will handle
+        // the results of the first launch and then recursively launch
+        // everything in the tail.
+        base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this),
+                   base::Time::Now(), version_, next_invocation,
+                   base::Passed(std::move(invocations))));
+  }
+
   // This method is called on the UI thread when the reporter run 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,
+                    std::unique_ptr<SwReporterQueue> remaining_invocations,
                     int exit_code) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -640,24 +671,38 @@ class ReporterRunner : public chrome::BrowserListObserver {
 
     base::TimeDelta reporter_running_time =
         base::Time::Now() - reporter_start_time;
-    // Don't continue when the reporter process failed to launch, but still try
-    // again after the regular delay. It's not worth retrying earlier, risking
-    // running too often if it always fails, since not many users fail here.
-    main_thread_task_runner_->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
-        base::TimeDelta::FromDays(days_between_reporter_runs_));
+
+    // Don't continue the current queue of reporters if one failed to launch.
+    // 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 (!remaining_invocations->empty() &&
+        exit_code != kReporterFailureExitCode) {
+      LaunchNextInvocation(std::move(remaining_invocations));
+    } else {
+      main_thread_task_runner_->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
+          base::TimeDelta::FromDays(days_between_reporter_runs_));
+    }
+
+    // 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|.)
     if (exit_code == kReporterFailureExitCode)
       return;
 
     UMAHistogramReporter uma(finished_invocation.suffix);
     uma.ReportVersion(version);
     uma.ReportExitCode(exit_code);
-    uma.ReportFoundUwS(!finished_invocation.is_experimental /*use_rappor*/);
+    uma.ReportFoundUwS(finished_invocation.flags &
+                       SwReporterInvocation::FLAG_LOG_TO_RAPPOR);
 
     //  Only save results from the canonical version of the software.
     PrefService* local_state = g_browser_process->local_state();
-    if (local_state && !finished_invocation.is_experimental) {
+    if (local_state &&
+        (finished_invocation.flags & SwReporterInvocation::FLAG_LOG_TO_PREFS)) {
       local_state->SetInteger(prefs::kSwReporterLastExitCode, exit_code);
       local_state->SetInt64(prefs::kSwReporterLastTimeTriggered,
                             base::Time::Now().ToInternalValue());
@@ -666,8 +711,8 @@ class ReporterRunner : public chrome::BrowserListObserver {
     uma.ReportScanTimes();
     uma.ReportMemoryUsage();
 
-    // Only continue to launch the prompt for the canonical version.
-    if (finished_invocation.is_experimental)
+    if (!(finished_invocation.flags &
+          SwReporterInvocation::FLAG_TRIGGER_PROMPT))
       return;
 
     if (!IsInSRTPromptFieldTrialGroups()) {
@@ -721,22 +766,16 @@ class ReporterRunner : public chrome::BrowserListObserver {
         last_time_triggered +
         base::TimeDelta::FromDays(days_between_reporter_runs_) -
         base::Time::Now());
-    if (next_trigger_delay.ToInternalValue() <= 0 ||
-        // Also make sure the kSwReporterLastTimeTriggered value is not set in
-        // the future.
-        last_time_triggered > base::Time::Now()) {
-      if (g_testing_delegate_)
-        g_testing_delegate_->NotifyLaunchReady();
-
-      // It's OK to simply |PostTaskAndReplyWithResult| so that
-      // |LaunchAndWaitForExit| doesn't need to access
-      // |main_thread_task_runner_| since the callback is not delayed and the
-      // test task runner won't need to force it.
-      base::PostTaskAndReplyWithResult(
-          blocking_task_runner_.get(), FROM_HERE,
-          base::Bind(&LaunchAndWaitForExit, invocation_),
-          base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this),
-                     base::Time::Now(), version_, invocation_));
+    if (!invocations_.empty() &&
+        (next_trigger_delay.ToInternalValue() <= 0 ||
+         // Also make sure the kSwReporterLastTimeTriggered value is not set in
+         // the future.
+         last_time_triggered > base::Time::Now())) {
+      // Make a copy of the queue, since LaunchNextInvocation will pop items
+      // from it. We need to save the full original for the comparison in
+      // Run.
+      auto invocations = std::make_unique<SwReporterQueue>(invocations_);
+      LaunchNextInvocation(std::move(invocations));
     } else {
       main_thread_task_runner_->PostDelayedTask(
           FROM_HERE,
@@ -746,7 +785,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
   }
 
   bool first_run_ = true;
-  SwReporterInvocation invocation_;
+  SwReporterQueue invocations_;
   base::Version version_;
   scoped_refptr<base::TaskRunner> main_thread_task_runner_;
   scoped_refptr<base::TaskRunner> blocking_task_runner_;
@@ -786,14 +825,14 @@ SwReporterInvocation SwReporterInvocation::FromCommandLine(
 
 bool SwReporterInvocation::operator==(const SwReporterInvocation& other) const {
   return command_line.argv() == other.command_line.argv() &&
-         suffix == other.suffix && is_experimental == other.is_experimental;
+         suffix == other.suffix && flags == other.flags;
 }
 
-void RunSwReporter(const SwReporterInvocation& invocation,
-                   const base::Version& version,
-                   scoped_refptr<base::TaskRunner> main_thread_task_runner,
-                   scoped_refptr<base::TaskRunner> blocking_task_runner) {
-  ReporterRunner::Run(invocation, version, std::move(main_thread_task_runner),
+void RunSwReporters(const SwReporterQueue& invocations,
+                    const base::Version& version,
+                    scoped_refptr<base::TaskRunner> main_thread_task_runner,
+                    scoped_refptr<base::TaskRunner> blocking_task_runner) {
+  ReporterRunner::Run(invocations, version, std::move(main_thread_task_runner),
                       std::move(blocking_task_runner));
 }