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>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/base_paths.h"
 #include "base/bind.h"
+#include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/threading/worker_pool.h"
 #include "base/time/time.h"
 #include "base/win/registry.h"
+#include "base/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/safe_browsing/srt_fetcher_win.h"
 #include "chrome/browser/safe_browsing/srt_field_trial_win.h"
 #include "components/component_updater/component_updater_paths.h"
 #include "components/component_updater/component_updater_service.h"
 #include "components/component_updater/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/update_client/update_client.h"
 #include "components/update_client/utils.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace component_updater {
 
 namespace {
 
-// These two sets of values are used to send UMA information and are replicated
-// in the histograms.xml file, so the order MUST NOT CHANGE.
+// These values are used to send UMA information and are replicated in the
+// histograms.xml file, so the order MUST NOT CHANGE.
 enum SRTCompleted {
   SRT_COMPLETED_NOT_YET = 0,
   SRT_COMPLETED_YES = 1,
   SRT_COMPLETED_LATER = 2,
   SRT_COMPLETED_MAX,
 };
 
 // CRX hash. The extension id is: gkmgaooipdjhmangpemjhigmamcehddo. The hash was
 // generated in Python with something like this:
 // hashlib.sha256().update(open("<file>.crx").read()[16:16+294]).digest().
@@ skipping 54 matching lines @@
 void RunSwReporterAfterStartup(
     const safe_browsing::SwReporterInvocation& invocation,
     const base::Version& version) {
   content::BrowserThread::PostAfterStartupTask(
       FROM_HERE, base::ThreadTaskRunnerHandle::Get(),
       base::Bind(&safe_browsing::RunSwReporter, invocation, version,
                  base::ThreadTaskRunnerHandle::Get(),
                  base::WorkerPool::GetTaskRunner(true)));
 }
 
+// Constants and helper functions for the Experimental Engine feature.
+
+constexpr base::Feature kExperimentalEngineFeature{

[csharp, 2016/08/25 19:47:27]

The constants and enum can probably be moved up to live with the other constants
and enum, it shouldn't matter if they aren't too close to the function since
their names should be all that is needed here

[Joe Mason, 2016/08/25 22:03:20]

I see I'm outnumbered here. Done.

+    "ExperimentalSwReporterEngine", base::FEATURE_DISABLED_BY_DEFAULT};
+
+// Max length of an installer attribute (taken from the regexp in
+// ComponentInstallerTraits::InstallerAttributes).
+constexpr size_t kMaxAttributeLength = 256;
+
+// 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;
+
+// These MUST match the values for SwReporterExperimentError in histograms.xml.
+enum ExperimentError {
+  EXPERIMENT_ERROR_BAD_TAG = 0,
+  EXPERIMENT_ERROR_BAD_PARAMS = 1,
+  EXPERIMENT_ERROR_MAX,
+};
+
+void ReportExperimentError(ExperimentError error) {
+  UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.ExperimentErrors", error,
+                            EXPERIMENT_ERROR_MAX);
+}
+
+// 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 &&
+         std::all_of(str.cbegin(), str.cend(), [&extras](char c) {
+           return base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) ||
+                  extras.find(c) != std::string::npos;
+         });
+}
+
+// Reads the command-line params and an UMA histogram suffix from the manifest,
+// and launch the SwReporter with those parameters. If anything goes wrong the
+// SwReporter should not be run at all, instead of falling back to the default.
+void RunExperimentalSwReporter(const base::FilePath& exe_path,
+                               const base::Version& version,
+                               std::unique_ptr<base::DictionaryValue> manifest,
+                               const SwReporterRunner& reporter_runner) {
+  // Don't log an error if the launch_params section is entirely missing.  This
+  // can happen when the user already has an older version of the software
+  // reporter component, so |ComponentReady| is called on startup before the
+  // experimental version is downloaded.
+  //
+  // Also don't run the old version, though. We want this user to run the

[csharp, 2016/08/25 19:47:27]

nit: This comment doesn't really add much since the old version can't run here
at all. Remove?

[Joe Mason, 2016/08/25 22:03:20]

Actually the old version can run here, if we're not careful - this comment is
explaining why we return early. I reworded it to be more clear.

+  // experimental version once it's been downloaded.
+  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()) {
+    ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+    return;
+  }
+
+  // For now we only support running the reporter once, so only look at the

[csharp, 2016/08/25 19:47:28]

Nit: Instead of this comments what about just adding to the if statement above
"|| parameter_list->size() > 1", since it would be an error for this to get get
multiple args

[Joe Mason, 2016/08/25 22:03:19]

Done.

+  // first set of arguments in the list.
+  const base::DictionaryValue* invocation_params = nullptr;
+  if (!parameter_list->GetDictionary(0, &invocation_params)) {
+    ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+    return;
+  }
+
+  // The suffix is optional. If present, it must be a short alphanumeric string.
+  std::string suffix;
+  const base::Value* suffix_value = nullptr;
+  if (invocation_params->Get("suffix", &suffix_value)) {
+    if (!suffix_value->GetAsString(&suffix)) {
+      ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+      return;
+    }
+    if (!suffix.empty() &&

[csharp, 2016/08/25 19:47:28]

nit: I wonder if it'd be clear to merge this with the above statement"
if (!(invocation_params->Get("suffix", &suffix_value) && !suffix.empty() &&
ValidateString(suffix, std::string(), kMaxSuffixLength))"

[Joe Mason, 2016/08/25 22:03:20]

Done.

+        !ValidateString(suffix, std::string(), kMaxSuffixLength)) {
+      ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+      return;
+    }
+  }
+
+  // Build a command line for the reporter out of the executable path and the
+  // (optional) arguments from the manifest.
+  std::vector<base::string16> argv = {exe_path.value()};
+  const base::Value* arguments_value = nullptr;
+  if (invocation_params->Get("arguments", &arguments_value)) {
+    const base::ListValue* arguments = nullptr;
+    if (!arguments_value->GetAsList(&arguments)) {
+      ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+      return;
+    }
+    for (const std::unique_ptr<base::Value>& value : *arguments) {
+      base::string16 argument;
+      if (!value->GetAsString(&argument)) {
+        ReportExperimentError(EXPERIMENT_ERROR_BAD_PARAMS);
+        return;
+      }
+      if (!argument.empty())

[csharp, 2016/08/25 19:47:27]

An empty argument seems like an error, move into the above if statement?

[Joe Mason, 2016/08/25 22:03:19]

I don't see any reason to ban an explicit empty string. It's a natural way to
write "no arguments". Look at the EmptyArguments and EmptyArguments2 unit tests
- it'd be confusing for one to work and the other to fail.

+        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);
+
+  auto invocation =
+      safe_browsing::SwReporterInvocation::FromCommandLine(command_line);
+  invocation.suffix = suffix;
+  invocation.is_experimental = true;
+
+  reporter_runner.Run(invocation, version);
+}
+
 }  // namespace
 
 SwReporterInstallerTraits::SwReporterInstallerTraits(
-    const SwReporterRunner& reporter_runner)
-    : reporter_runner_(reporter_runner) {}
+    const SwReporterRunner& reporter_runner,
+    bool is_experimental_engine_supported)
+    : reporter_runner_(reporter_runner),
+      is_experimental_engine_supported_(is_experimental_engine_supported) {}
 
 SwReporterInstallerTraits::~SwReporterInstallerTraits() {}
 
 bool SwReporterInstallerTraits::VerifyInstallation(
     const base::DictionaryValue& manifest,
     const base::FilePath& dir) const {
   return base::PathExists(dir.Append(kSwReporterExeName));
 }
 
 bool SwReporterInstallerTraits::SupportsGroupPolicyEnabledComponentUpdates()
@@ skipping 10 matching lines @@
     const base::FilePath& install_dir) {
   return true;
 }
 
 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_);
+    return;

[csharp, 2016/08/25 19:47:27]

nit: Instead of a return here what about just adding an else for the next run
command

[Joe Mason, 2016/08/25 22:03:19]

Done.

+  }
   reporter_runner_.Run(
       safe_browsing::SwReporterInvocation::FromFilePath(exe_path), 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));
 }
 
 std::string SwReporterInstallerTraits::GetName() const {
   return "Software Reporter Tool";
 }
 
 update_client::InstallerAttributes
 SwReporterInstallerTraits::GetInstallerAttributes() const {
-  return update_client::InstallerAttributes();
+  update_client::InstallerAttributes attributes;
+  if (IsExperimentalEngineEnabled()) {
+    // Pass the "tag" parameter to the installer; it will be used to choose
+    // which binary is downloaded.
+    constexpr char kTagParam[] = "tag";
+    const std::string tag = variations::GetVariationParamValueByFeature(
+        kExperimentalEngineFeature, kTagParam);
+
+    // If the tag is not a valid attribute (see the regexp in
+    // ComponentInstallerTraits::InstallerAttributes), set it to a valid but
+    // unrecognized value so that nothing will be downloaded.
+    if (tag.empty() || !ValidateString(tag, "_.,;+_=", kMaxAttributeLength)) {

[csharp, 2016/08/25 19:47:28]

nit: Please put "_.,;+_=" into a named constant

[Joe Mason, 2016/08/25 22:03:20]

Done.

+      ReportExperimentError(EXPERIMENT_ERROR_BAD_TAG);
+      attributes[kTagParam] = "missing_tag";
+    } else {
+      attributes[kTagParam] = tag;
+    }
+  }
+  return attributes;
 }
 
 std::vector<std::string> SwReporterInstallerTraits::GetMimeTypes() const {
   return std::vector<std::string>();
 }
 
+bool SwReporterInstallerTraits::IsExperimentalEngineEnabled() const {
+  return is_experimental_engine_supported_ &&
+         base::FeatureList::IsEnabled(kExperimentalEngineFeature);
+}
+
 void RegisterSwReporterComponent(ComponentUpdateService* cus) {
   if (!safe_browsing::IsSwReporterEnabled())
     return;
 
   // Check if we have information from Cleaner and record UMA statistics.
   base::string16 cleaner_key_name(
       safe_browsing::kSoftwareRemovalToolRegistryKey);
   cleaner_key_name.append(1, L'\\').append(kCleanerSuffixRegistryKey);
   base::win::RegKey cleaner_key(
       HKEY_CURRENT_USER, cleaner_key_name.c_str(), KEY_ALL_ACCESS);
@@ skipping 56 matching lines @@
         ReportUploadsWithUma(upload_results);
       }
     } else {
       if (cleaner_key.HasValue(safe_browsing::kEndTimeValueName)) {
         SRTHasCompleted(SRT_COMPLETED_LATER);
         cleaner_key.DeleteValue(safe_browsing::kEndTimeValueName);
       }
     }
   }
 
+  // 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(&RunSwReporterAfterStartup),
+                                    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