Add support for the ExperimentalSwReporterEngine field trial.

chrome/browser/component_updater/sw_reporter_installer_win.cc

Index: chrome/browser/component_updater/sw_reporter_installer_win.cc
diff --git a/chrome/browser/component_updater/sw_reporter_installer_win.cc b/chrome/browser/component_updater/sw_reporter_installer_win.cc
index fc0e1e684911926cf2a98e197c867fe599591bae..5db58046ffe2b395fad9c6ea223054be56c4677b 100644
--- a/chrome/browser/component_updater/sw_reporter_installer_win.cc
+++ b/chrome/browser/component_updater/sw_reporter_installer_win.cc
@@ -121,12 +121,12 @@ void ReportExperimentError(SwReporterExperimentError error) {
 // (This is the default |reporter_runner| function passed to the
 // |SwReporterInstallerTraits| constructor in |RegisterSwReporterComponent|
 // below.)
-void RunSwReporterAfterStartup(
-    const safe_browsing::SwReporterInvocation& invocation,
+void RunSwReportersAfterStartup(
+    const safe_browsing::SwReporterQueue& invocations,
     const base::Version& version) {
   content::BrowserThread::PostAfterStartupTask(
       FROM_HERE, base::ThreadTaskRunnerHandle::Get(),
-      base::Bind(&safe_browsing::RunSwReporter, invocation, version,
+      base::Bind(&safe_browsing::RunSwReporters, invocations, version,
                  base::ThreadTaskRunnerHandle::Get(),
                  base::WorkerPool::GetTaskRunner(true)));
 }
@@ -158,73 +158,88 @@ void RunExperimentalSwReporter(const base::FilePath& exe_path,
     return;
 
   const base::ListValue* parameter_list = nullptr;
-  if (!launch_params->GetAsList(&parameter_list) || parameter_list->empty() ||
-      // For future expansion, the manifest takes a list of invocation
-      // parameters, but currently we only support a single invocation.
-      parameter_list->GetSize() > 1) {
+  if (!launch_params->GetAsList(&parameter_list) || parameter_list->empty()) {
     ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
     return;
   }
 
-  const base::DictionaryValue* invocation_params = nullptr;
-  if (!parameter_list->GetDictionary(0, &invocation_params)) {
-    ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
-    return;
-  }
-
-  // Max length of the registry and histogram suffix. Fairly arbitrary: the
-  // Windows registry accepts much longer keys, but we need to display this
-  // string in histograms as well.
-  constexpr size_t kMaxSuffixLength = 80;
-
-  // The suffix must be an alphanumeric string. (Empty is fine as long as the
-  // "suffix" key is present.)
-  std::string suffix;
-  const base::Value* suffix_value = nullptr;
-  if (!invocation_params->Get("suffix", &suffix_value) ||
-      !suffix_value->GetAsString(&suffix) ||
-      (!suffix.empty() &&
-       !ValidateString(suffix, std::string(), kMaxSuffixLength))) {
-    ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
-    return;
-  }
+  safe_browsing::SwReporterQueue invocations;
+  for (const auto& iter : *parameter_list) {
+    const base::DictionaryValue* invocation_params = nullptr;
+    if (!iter->GetAsDictionary(&invocation_params)) {
+      ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
+      return;
+    }
 
-  // Build a command line for the reporter out of the executable path and the
-  // arguments from the manifest. (The "arguments" key must be present, but
-  // it's ok if it's an empty list or a list of empty strings.)
-  std::vector<base::string16> argv = {exe_path.value()};
-  const base::Value* arguments_value = nullptr;
-  const base::ListValue* arguments = nullptr;
-  if (!invocation_params->Get("arguments", &arguments_value) ||
-      !arguments_value->GetAsList(&arguments)) {
-    ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
-    return;
-  }
+    // Max length of the registry and histogram suffix. Fairly arbitrary: the
+    // Windows registry accepts much longer keys, but we need to display this
+    // string in histograms as well.
+    constexpr size_t kMaxSuffixLength = 80;
+
+    // The suffix must be an alphanumeric string. (Empty is fine as long as the
+    // "suffix" key is present.)
+    std::string suffix;
+    const base::Value* suffix_value = nullptr;
+    if (!invocation_params->Get("suffix", &suffix_value) ||

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

    if (!invocation_params->GetString("suffix", &suffix)) ||

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

Done.

+        !suffix_value->GetAsString(&suffix) ||
+        (!suffix.empty() &&

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

this check doesn't add anything beyond the comment above, yet it makes the code
harder to read. additionally, it introduces an an extra check to short-circuit
the following function call that will only happen in the exceptional error
condition. in the majority of cases, it's a few more wasted cycles (and bytes in
the binary).

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

Done.

+         !ValidateString(suffix, std::string(), kMaxSuffixLength))) {
+      ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
+      return;
+    }
 
-  for (const auto& value : *arguments) {
-    base::string16 argument;
-    if (!value->GetAsString(&argument)) {
+    // Build a command line for the reporter out of the executable path and the
+    // arguments from the manifest. (The "arguments" key must be present, but
+    // it's ok if it's an empty list or a list of empty strings.)
+    std::vector<base::string16> argv = {exe_path.value()};

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

nit: move down just before line 203

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

Done.

+    const base::Value* arguments_value = nullptr;
+    const base::ListValue* arguments = nullptr;
+    if (!invocation_params->Get("arguments", &arguments_value) ||

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

    if (!invocation_params->GetList("arguments", &arguments)) {

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

Done.

+        !arguments_value->GetAsList(&arguments)) {
       ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
       return;
     }
-    if (!argument.empty())
-      argv.push_back(argument);
-  }
 
-  base::CommandLine command_line(argv);
+    for (const auto& value : *arguments) {
+      base::string16 argument;
+      if (!value->GetAsString(&argument)) {
+        ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
+        return;
+      }
+      if (!argument.empty())
+        argv.push_back(argument);
+    }
 
-  // Add the histogram suffix to the command-line as well, so that the
-  // reporter will add the same suffix to registry keys where it writes
-  // metrics.
-  if (!suffix.empty())
-    command_line.AppendSwitchASCII("registry-suffix", suffix);
+    base::CommandLine command_line(argv);
+
+    // Add the histogram suffix to the command-line as well, so that the
+    // reporter will add the same suffix to registry keys where it writes
+    // metrics.
+    if (!suffix.empty())
+      command_line.AppendSwitchASCII("registry-suffix", suffix);
+
+    // "prompt" is optional, but if present must be a boolean.
+    safe_browsing::SwReporterInvocation::Flags flags = 0;
+    const base::Value* prompt_value = nullptr;
+    if (invocation_params->Get("prompt", &prompt_value)) {
+      bool prompt = false;
+      if (!prompt_value->GetAsBoolean(&prompt)) {
+        ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
+        return;
+      }
+      if (prompt)
+        flags |= safe_browsing::SwReporterInvocation::FLAG_TRIGGER_PROMPT;
+    }
 
-  auto invocation =
-      safe_browsing::SwReporterInvocation::FromCommandLine(command_line);
-  invocation.suffix = suffix;
-  invocation.is_experimental = true;
+    auto invocation =
+        safe_browsing::SwReporterInvocation::FromCommandLine(command_line);
+    invocation.suffix = suffix;
+    invocation.flags = flags;
+    invocations.push(invocation);
+  }
 
-  reporter_runner.Run(invocation, version);
+  DCHECK(!invocations.empty());
+  reporter_runner.Run(invocations, version);

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

how about std::move the collection into Run (and change the callback type
accordingly)?

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

Done.

 }
 
 }  // namespace
@@ -268,8 +283,10 @@ void SwReporterInstallerTraits::ComponentReady(
     RunExperimentalSwReporter(exe_path, version, std::move(manifest),
                               reporter_runner_);
   } else {
-    reporter_runner_.Run(
-        safe_browsing::SwReporterInvocation::FromFilePath(exe_path), version);
+    safe_browsing::SwReporterQueue invocations;
+    invocations.push(
+        safe_browsing::SwReporterInvocation::FromFilePath(exe_path));
+    reporter_runner_.Run(invocations, version);
   }
 }
 
@@ -405,7 +422,7 @@ void RegisterSwReporterComponent(ComponentUpdateService* cus) {
 
   // Install the component.
   std::unique_ptr<ComponentInstallerTraits> traits(
-      new SwReporterInstallerTraits(base::Bind(&RunSwReporterAfterStartup),
+      new SwReporterInstallerTraits(base::Bind(&RunSwReportersAfterStartup),
                                     is_experimental_engine_supported));
   // |cus| will take ownership of |installer| during installer->Register(cus).
   DefaultComponentInstaller* installer =