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 62996ec1755571a981f9d6c4ab7f605c9345e550..05af4980b12d2f192e92c930d2fd0cf8cee255d1 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_win.cc
@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <vector>
 
 #include "base/bind.h"
@@ -142,26 +143,42 @@ void DisplaySRTPrompt(const base::FilePath& download_path) {
 // 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 base::FilePath& exe_path) {
+int LaunchAndWaitForExit(const SwReporterInvocation& invocation) {
   if (!g_reporter_launcher_.is_null())
-    return g_reporter_launcher_.Run(exe_path);
+    return g_reporter_launcher_.Run(invocation.command_line.GetProgram());
   base::Process reporter_process =
-      base::LaunchProcess(exe_path.value(), base::LaunchOptions());
+      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;
   if (reporter_process.IsValid()) {
-    RecordReporterStepHistogram(SW_REPORTER_START_EXECUTION);
+    RecordReporterStepHistogram(SW_REPORTER_START_EXECUTION,
+                                invocation.suffix);
     bool success = reporter_process.WaitForExit(&exit_code);
     DCHECK(success);
   } else {
-    RecordReporterStepHistogram(SW_REPORTER_FAILED_TO_START);
+    RecordReporterStepHistogram(SW_REPORTER_FAILED_TO_START,
+                                invocation.suffix);
   }
   return exit_code;
 }
 
+std::string AddHistogramSuffix(const std::string& histogram,
+                               const std::string& suffix) {

[csharp, 2016/08/11 18:58:02]

Given the number of functions that have been modified to pass suffix around, it
seems like they should probably be turned into a class with suffix as a member.

[Joe Mason, 2016/08/15 17:24:40]

Done. Also I moved this refactor to http://crrev.com/2243303002 to make it
easier to follow, since it moves some functions around and increases the size of
this patch a lot.

+  if (suffix.empty())
+    return histogram;
+  return base::StringPrintf("%s_%s", histogram, suffix);
+}
+
+std::wstring RegistryKeyWithSuffix(const std::string& suffix) {
+  if (suffix.empty())
+    return kSoftwareRemovalToolRegistryKey;
+  return base::StringPrintf(L"%s\\%s", kSoftwareRemovalToolRegistryKey,
+                            base::UTF8ToUTF16(suffix));
+}
+
 // Reports the software reporter tool's version via UMA.
-void ReportVersion(const base::Version& version) {
+void ReportVersion(const base::Version& version, const std::string& suffix) {
   DCHECK(!version.components().empty());
   // The minor version is the 2nd last component of the version,
   // or just the first component if there is only 1.
@@ -170,7 +187,9 @@ void ReportVersion(const base::Version& version) {
     minor_version = version.components()[version.components().size() - 2];
   else
     minor_version = version.components()[0];
-  UMA_HISTOGRAM_SPARSE_SLOWLY("SoftwareReporter.MinorVersion", minor_version);
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      AddHistogramSuffix("SoftwareReporter.MinorVersion", suffix),
+      minor_version);
 
   // The major version for X.Y.Z is X*256^3+Y*256+Z. If there are additional
   // components, only the first three count, and if there are less than 3, the
@@ -185,13 +204,15 @@ void ReportVersion(const base::Version& version) {
     DCHECK_LT(version.components()[2], 0x100U);
     major_version += version.components()[2];
   }
-  UMA_HISTOGRAM_SPARSE_SLOWLY("SoftwareReporter.MajorVersion", major_version);
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      AddHistogramSuffix("SoftwareReporter.MajorVersion", suffix),
+      major_version);
 }
 
 // Reports UwS found by the software reporter tool via UMA and RAPPOR.
-void ReportFoundUwS() {
+void ReportFoundUwS(const std::string& suffix) {
   base::win::RegKey reporter_key(HKEY_CURRENT_USER,
-                                 kSoftwareRemovalToolRegistryKey,
+                                 RegistryKeyWithSuffix(suffix).c_str(),
                                  KEY_QUERY_VALUE | KEY_SET_VALUE);
   std::vector<base::string16> found_uws_strings;
   if (reporter_key.Valid() &&
@@ -204,7 +225,8 @@ void ReportFoundUwS() {
       // All UwS ids are expected to be integers.
       uint32_t uws_id = 0;
       if (base::StringToUint(uws_string, &uws_id)) {
-        UMA_HISTOGRAM_SPARSE_SLOWLY(kFoundUwsMetricName, uws_id);
+        UMA_HISTOGRAM_SPARSE_SLOWLY(AddHistogramSuffix(kFoundUwsMetricName,
+              suffix), uws_id);
         if (rappor_service) {
           rappor_service->RecordSample(kFoundUwsMetricName,
                                        rappor::COARSE_RAPPOR_TYPE,
@@ -218,21 +240,23 @@ void ReportFoundUwS() {
     // Clean up the old value.
     reporter_key.DeleteValue(kFoundUwsValueName);
 
-    UMA_HISTOGRAM_BOOLEAN(kFoundUwsReadErrorMetricName, parse_error);
+    UMA_HISTOGRAM_BOOLEAN(AddHistogramSuffix(kFoundUwsReadErrorMetricName,
+          suffix), parse_error);
   }
 }
 
 // Reports to UMA the memory usage of the software reporter tool as reported by
 // the tool itself in the Windows registry.
-void ReportSwReporterMemoryUsage() {
+void ReportSwReporterMemoryUsage(const std::string& suffix) {
   base::win::RegKey reporter_key(HKEY_CURRENT_USER,
-                                 kSoftwareRemovalToolRegistryKey,
+                                 RegistryKeyWithSuffix(suffix).c_str(),
                                  KEY_QUERY_VALUE | KEY_SET_VALUE);
   DWORD memory_used = 0;
   if (reporter_key.Valid() &&
       reporter_key.ReadValueDW(kMemoryUsedValueName, &memory_used) ==
           ERROR_SUCCESS) {
-    UMA_HISTOGRAM_MEMORY_KB(kMemoryUsedMetricName, memory_used);
+    UMA_HISTOGRAM_MEMORY_KB(AddHistogramSuffix(kMemoryUsedMetricName, suffix),
+                            memory_used);
     reporter_key.DeleteValue(kMemoryUsedValueName);
   }
 }
@@ -363,16 +387,19 @@ void MaybeFetchSRT(Browser* browser, const base::Version& reporter_version) {
 
 // Report the SwReporter run time with UMA both as reported by the tool via
 // the registry and as measured by |ReporterRunner|.
-void ReportSwReporterRuntime(const base::TimeDelta& reporter_running_time) {
-  UMA_HISTOGRAM_LONG_TIMES("SoftwareReporter.RunningTimeAccordingToChrome",
-                           reporter_running_time);
+void ReportSwReporterRuntime(const base::TimeDelta& reporter_running_time,
+                             const std::string& suffix) {
+  UMA_HISTOGRAM_LONG_TIMES(AddHistogramSuffix(
+        "SoftwareReporter.RunningTimeAccordingToChrome", suffix),
+      reporter_running_time);
 
   base::win::RegKey reporter_key(
-      HKEY_CURRENT_USER, kSoftwareRemovalToolRegistryKey, KEY_ALL_ACCESS);
+      HKEY_CURRENT_USER, RegistryKeyWithSuffix(suffix).c_str(), KEY_ALL_ACCESS);
   if (!reporter_key.Valid()) {
-    UMA_HISTOGRAM_ENUMERATION(kRunningTimeErrorMetricName,
-                              REPORTER_RUNNING_TIME_ERROR_REGISTRY_KEY_INVALID,
-                              REPORTER_RUNNING_TIME_ERROR_MAX);
+    UMA_HISTOGRAM_ENUMERATION(
+        AddHistogramSuffix(kRunningTimeErrorMetricName, suffix),
+        REPORTER_RUNNING_TIME_ERROR_REGISTRY_KEY_INVALID,
+        REPORTER_RUNNING_TIME_ERROR_MAX);
     return;
   }
 
@@ -398,30 +425,35 @@ void ReportSwReporterRuntime(const base::TimeDelta& reporter_running_time) {
     base::TimeDelta registry_run_time =
         base::Time::FromInternalValue(end_time_value) -
         base::Time::FromInternalValue(start_time_value);
-    UMA_HISTOGRAM_LONG_TIMES("SoftwareReporter.RunningTime", registry_run_time);
-    UMA_HISTOGRAM_ENUMERATION(kRunningTimeErrorMetricName,
-                              REPORTER_RUNNING_TIME_ERROR_NO_ERROR,
-                              REPORTER_RUNNING_TIME_ERROR_MAX);
+    UMA_HISTOGRAM_LONG_TIMES(AddHistogramSuffix("SoftwareReporter.RunningTime",
+          suffix), registry_run_time);
+    UMA_HISTOGRAM_ENUMERATION(
+        AddHistogramSuffix(kRunningTimeErrorMetricName, suffix),
+        REPORTER_RUNNING_TIME_ERROR_NO_ERROR,
+        REPORTER_RUNNING_TIME_ERROR_MAX);
   } else if (!has_start_time && !has_end_time) {
-    UMA_HISTOGRAM_ENUMERATION(kRunningTimeErrorMetricName,
-                              REPORTER_RUNNING_TIME_ERROR_MISSING_BOTH_TIMES,
-                              REPORTER_RUNNING_TIME_ERROR_MAX);
+    UMA_HISTOGRAM_ENUMERATION(
+        AddHistogramSuffix(kRunningTimeErrorMetricName, suffix),
+        REPORTER_RUNNING_TIME_ERROR_MISSING_BOTH_TIMES,
+        REPORTER_RUNNING_TIME_ERROR_MAX);
   } else if (!has_start_time) {
-    UMA_HISTOGRAM_ENUMERATION(kRunningTimeErrorMetricName,
-                              REPORTER_RUNNING_TIME_ERROR_MISSING_START_TIME,
-                              REPORTER_RUNNING_TIME_ERROR_MAX);
+    UMA_HISTOGRAM_ENUMERATION(
+        AddHistogramSuffix(kRunningTimeErrorMetricName, suffix),
+        REPORTER_RUNNING_TIME_ERROR_MISSING_START_TIME,
+        REPORTER_RUNNING_TIME_ERROR_MAX);
   } else {
     DCHECK(!has_end_time);
-    UMA_HISTOGRAM_ENUMERATION(kRunningTimeErrorMetricName,
-                              REPORTER_RUNNING_TIME_ERROR_MISSING_END_TIME,
-                              REPORTER_RUNNING_TIME_ERROR_MAX);
+    UMA_HISTOGRAM_ENUMERATION(
+        AddHistogramSuffix(kRunningTimeErrorMetricName, suffix),
+        REPORTER_RUNNING_TIME_ERROR_MISSING_END_TIME,
+        REPORTER_RUNNING_TIME_ERROR_MAX);
   }
 }
 
 // Report the UwS scan times of the software reporter tool via UMA.
-void ReportSwReporterScanTimes() {
+void ReportSwReporterScanTimes(const std::string& suffix) {
   base::string16 scan_times_key_path = base::StringPrintf(
-      L"%ls\\%ls", kSoftwareRemovalToolRegistryKey, kScanTimesSubKey);
+      L"%ls\\%ls", RegistryKeyWithSuffix(suffix).c_str(), kScanTimesSubKey);
   base::win::RegKey scan_times_key(HKEY_CURRENT_USER,
                                    scan_times_key_path.c_str(), KEY_ALL_ACCESS);
   if (!scan_times_key.Valid())
@@ -439,7 +471,8 @@ void ReportSwReporterScanTimes() {
       base::TimeDelta scan_time =
           base::TimeDelta::FromInternalValue(raw_scan_time);
       base::HistogramBase* histogram = base::SparseHistogram::FactoryGet(
-          kScanTimesMetricName, base::HistogramBase::kUmaTargetedHistogramFlag);
+          AddHistogramSuffix(kScanTimesMetricName, suffix),
+          base::HistogramBase::kUmaTargetedHistogramFlag);
       if (histogram) {
         // We report the number of seconds plus one because it can take less
         // than one second to scan some UwS and the count passed to |AddCount|
@@ -452,7 +485,7 @@ void ReportSwReporterScanTimes() {
   // reporter key.
   scan_times_key.Close();
   base::win::RegKey reporter_key(HKEY_CURRENT_USER,
-                                 kSoftwareRemovalToolRegistryKey,
+                                 RegistryKeyWithSuffix(suffix).c_str(),
                                  KEY_ENUMERATE_SUB_KEYS);
   if (reporter_key.Valid())
     reporter_key.DeleteKey(kScanTimesSubKey);
@@ -465,7 +498,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
  public:
   // Starts the sequence of attempts to run the reporter.
   static void Run(
-      const base::FilePath& exe_path,
+      const std::vector<SwReporterInvocation>& invocation_queue,
       const base::Version& version,
       const scoped_refptr<base::TaskRunner>& main_thread_task_runner,
       const scoped_refptr<base::TaskRunner>& blocking_task_runner) {
@@ -474,12 +507,13 @@ 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_->exe_path_ == exe_path && instance_->version_.IsValid() &&
+    if (instance_->invocation_queue_ == invocation_queue &&
+        instance_->version_.IsValid() &&
         instance_->version_ == version)
       return;
-    bool first_run = instance_->exe_path_.empty();
+    bool first_run = instance_->invocation_queue_.empty();
 
-    instance_->exe_path_ = exe_path;
+    instance_->invocation_queue_ = invocation_queue;
     instance_->version_ = version;
     instance_->main_thread_task_runner_ = main_thread_task_runner;
     instance_->blocking_task_runner_ = blocking_task_runner;
@@ -488,6 +522,27 @@ class ReporterRunner : public chrome::BrowserListObserver {
       instance_->TryToRun();
   }
 
+  // Launch the command line at the head of the queue.
+  void LaunchNextInvocation(
+      const std::vector<SwReporterInvocation>& invocation_queue) {
+  DCHECK(!invocation_queue.empty());
+  auto head = invocation_queue.begin();
+  std::vector<SwReporterInvocation> tail(
+      head + 1, invocation_queue.end());
+
+  base::PostTaskAndReplyWithResult(
+      blocking_task_runner_.get(), FROM_HERE,
+
+      // Send the head of the queue to the launch callback.
+      base::Bind(&LaunchAndWaitForExit, *head),
+
+      // 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_, head->suffix, tail));
+  }
+
  private:
   ReporterRunner() {}
   ~ReporterRunner() override {}
@@ -506,10 +561,14 @@ class ReporterRunner : public chrome::BrowserListObserver {
   // 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,
-                    int exit_code) {
+      const base::Version& version,
+      const std::string& suffix,
+      const std::vector<SwReporterInvocation>& remaining_invocations,
+      int exit_code) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+    bool is_experimental_version = !suffix.empty();
+
     base::TimeDelta reporter_running_time =
         base::Time::Now() - reporter_start_time;
     // Don't continue when the reporter process failed to launch, but still try
@@ -522,18 +581,32 @@ class ReporterRunner : public chrome::BrowserListObserver {
     if (exit_code == kReporterFailureExitCode)
       return;
 
-    ReportVersion(version);
-    UMA_HISTOGRAM_SPARSE_SLOWLY("SoftwareReporter.ExitCode", exit_code);
-    ReportFoundUwS();
+    ReportVersion(version, suffix);
+    UMA_HISTOGRAM_SPARSE_SLOWLY(AddHistogramSuffix("SoftwareReporter.ExitCode",
+          suffix), exit_code);
+    ReportFoundUwS(suffix);
+
+    // Only save results from the canonical version of the software.
     PrefService* local_state = g_browser_process->local_state();
-    if (local_state) {
+    if (local_state && !is_experimental_version) {
       local_state->SetInteger(prefs::kSwReporterLastExitCode, exit_code);
       local_state->SetInt64(prefs::kSwReporterLastTimeTriggered,
                             base::Time::Now().ToInternalValue());
     }
-    ReportSwReporterRuntime(reporter_running_time);
-    ReportSwReporterScanTimes();
-    ReportSwReporterMemoryUsage();
+
+    ReportSwReporterRuntime(reporter_running_time, suffix);
+    ReportSwReporterScanTimes(suffix);
+    ReportSwReporterMemoryUsage(suffix);
+
+    if (!remaining_invocations.empty()) {
+      // Only the experimental version should have multiple invocations.
+      DCHECK(is_experimental_version);
+      LaunchNextInvocation(remaining_invocations);
+    }
+
+    // Only continue to launch the prompt for the canonical version.
+    if (is_experimental_version)
+      return;
 
     if (!IsInSRTPromptFieldTrialGroups()) {
       // Knowing about disabled field trial is more important than reporter not
@@ -567,7 +640,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     PrefService* local_state = g_browser_process->local_state();
     if (!version_.IsValid() || !local_state) {
-      DCHECK(exe_path_.empty());
+      DCHECK(invocation_queue_.empty());
       return;
     }
 
@@ -594,11 +667,8 @@ class ReporterRunner : public chrome::BrowserListObserver {
       // |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, exe_path_),
-          base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this),
-                     base::Time::Now(), version_));
+      if (!invocation_queue_.empty())
+        LaunchNextInvocation(invocation_queue_);
     } else {
       main_thread_task_runner_->PostDelayedTask(
           FROM_HERE,
@@ -607,7 +677,7 @@ class ReporterRunner : public chrome::BrowserListObserver {
     }
   }
 
-  base::FilePath exe_path_;
+  std::vector<SwReporterInvocation> invocation_queue_;
   base::Version version_;
   scoped_refptr<base::TaskRunner> main_thread_task_runner_;
   scoped_refptr<base::TaskRunner> blocking_task_runner_;
@@ -628,12 +698,41 @@ ReporterRunner* ReporterRunner::instance_ = nullptr;
 
 }  // namespace
 
+SwReporterInvocation::SwReporterInvocation()
+    : command_line(0, nullptr) {
+}
+
+SwReporterInvocation::SwReporterInvocation(const base::FilePath& exe_path)
+    : command_line(exe_path) {
+}
+
+SwReporterInvocation::SwReporterInvocation(
+    const base::CommandLine& command_line, const std::string& suffix)
+    : command_line(command_line), suffix(suffix) {
+}
+
+bool SwReporterInvocation::operator==(const SwReporterInvocation& other) const {
+  return command_line.argv() == other.command_line.argv() &&
+      suffix == other.suffix;
+}
+
 void RunSwReporter(
     const base::FilePath& exe_path,
     const base::Version& version,
     const scoped_refptr<base::TaskRunner>& main_thread_task_runner,
     const scoped_refptr<base::TaskRunner>& blocking_task_runner) {
-  ReporterRunner::Run(exe_path, version, main_thread_task_runner,
+  std::vector<SwReporterInvocation> invocations =
+      {SwReporterInvocation(exe_path)};
+  ReporterRunner::Run(invocations, version, main_thread_task_runner,
+                      blocking_task_runner);
+}
+
+void RunExperimentalSwReporters(
+    const std::vector<SwReporterInvocation>& invocation_queue,
+    const base::Version& version,
+    const scoped_refptr<base::TaskRunner>& main_thread_task_runner,
+    const scoped_refptr<base::TaskRunner>& blocking_task_runner) {
+  ReporterRunner::Run(invocation_queue, version, main_thread_task_runner,
                       blocking_task_runner);
 }