Add support for the ExperimentalSwReporterEngine field trial.

chrome/browser/component_updater/sw_reporter_installer_win.cc

 // Copyright (c) 2014 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/component_updater/sw_reporter_installer_win.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 103 matching lines @@
 
 void ReportExperimentError(SwReporterExperimentError error) {
   UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.ExperimentErrors", error,
                             SW_REPORTER_EXPERIMENT_ERROR_MAX);
 }
 
 // Run the software reporter on the next Chrome startup after it's downloaded.
 // (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)));
 }
 
 // Ensures |str| contains only alphanumeric characters and characters from
 // |extras|, and is not longer than |max_length|.
 bool ValidateString(const std::string& str,
                     const std::string& extras,
                     size_t max_length) {
   return str.size() <= max_length &&
@@ skipping 11 matching lines @@
                                std::unique_ptr<base::DictionaryValue> manifest,
                                const SwReporterRunner& reporter_runner) {
   // The experiment requires launch_params so if they aren't present just
   // return. This isn't an error because the user could get into the experiment
   // group before they've downloaded the experiment component.
   base::Value* launch_params = nullptr;
   if (!manifest->Get("launch_params", &launch_params))
     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;
-  }
-
-  // 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;
-  }
-
-  for (const auto& value : *arguments) {
-    base::string16 argument;
-    if (!value->GetAsString(&argument)) {
+  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;
     }
-    if (!argument.empty())
-      argv.push_back(argument);
+
+    // 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;
+    }
+
+    // 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;
+    }
+
+    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);
+    }
+
+    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.flags = flags;
+    invocations.push(invocation);
   }
 
-  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);
-
-  auto invocation =
-      safe_browsing::SwReporterInvocation::FromCommandLine(command_line);
-  invocation.suffix = suffix;
-  invocation.is_experimental = true;
-
-  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
 
 SwReporterInstallerTraits::SwReporterInstallerTraits(
     const SwReporterRunner& reporter_runner,
     bool is_experimental_engine_supported)
     : reporter_runner_(reporter_runner),
       is_experimental_engine_supported_(is_experimental_engine_supported) {}
 
@@ skipping 23 matching lines @@
 void SwReporterInstallerTraits::ComponentReady(
     const base::Version& version,
     const base::FilePath& install_dir,
     std::unique_ptr<base::DictionaryValue> manifest) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   const base::FilePath exe_path(install_dir.Append(kSwReporterExeName));
   if (IsExperimentalEngineEnabled()) {
     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);
   }
 }
 
 base::FilePath SwReporterInstallerTraits::GetRelativeInstallDir() const {
   return base::FilePath(FILE_PATH_LITERAL("SwReporter"));
 }
 
 void SwReporterInstallerTraits::GetHash(std::vector<uint8_t>* hash) const {
   DCHECK(hash);
   hash->assign(kSha256Hash, kSha256Hash + sizeof(kSha256Hash));
@@ skipping 115 matching lines @@
   }
 
   // The experiment is only enabled on x86. There's no way to check this in the
   // variations config so we'll hard-code it.
   const bool is_experimental_engine_supported =
       base::win::OSInfo::GetInstance()->architecture() ==
       base::win::OSInfo::X86_ARCHITECTURE;
 
   // 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 =
       new DefaultComponentInstaller(std::move(traits));
   installer->Register(cus, base::Closure());
 }
 
 void RegisterPrefsForSwReporter(PrefRegistrySimple* registry) {
   registry->RegisterInt64Pref(prefs::kSwReporterLastTimeTriggered, 0);
   registry->RegisterIntegerPref(prefs::kSwReporterLastExitCode, -1);
   registry->RegisterBooleanPref(prefs::kSwReporterPendingPrompt, false);
 }
 
 void RegisterProfilePrefsForSwReporter(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterStringPref(prefs::kSwReporterPromptVersion, "");
 
   registry->RegisterStringPref(prefs::kSwReporterPromptSeed, "");
 }
 
 }  // namespace component_updater