Chromium Code Reviews| 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 b97077186b5118bc7d4580b31460de6bd652120d..87de9f788ebb8114ec84f101a8db8f7fc2923a37 100644 |
| --- a/chrome/browser/component_updater/sw_reporter_installer_win.cc |
| +++ b/chrome/browser/component_updater/sw_reporter_installer_win.cc |
| @@ -6,6 +6,7 @@ |
| #include <stdint.h> |
| +#include <algorithm> |
| #include <map> |
| #include <memory> |
| #include <string> |
| @@ -13,6 +14,8 @@ |
| #include <vector> |
| #include "base/base_paths.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" |
| @@ -20,6 +23,9 @@ |
| #include "base/metrics/sparse_histogram.h" |
| #include "base/path_service.h" |
| #include "base/strings/string_tokenizer.h" |
| +#include "base/strings/stringprintf.h" |
| +#include "base/strings/utf_string_conversions.h" |
| +#include "base/sys_info.h" |
| #include "base/threading/thread_task_runner_handle.h" |
| #include "base/threading/worker_pool.h" |
| #include "base/time/time.h" |
| @@ -35,7 +41,9 @@ |
| #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" |
| +#include "third_party/re2/src/re2/re2.h" |
| namespace component_updater { |
| @@ -67,6 +75,14 @@ const wchar_t kExitCodeValueName[] = L"ExitCode"; |
| const wchar_t kUploadResultsValueName[] = L"UploadResults"; |
| const wchar_t kVersionValueName[] = L"Version"; |
| +// Experiment parameters for the new engine. |
| +const base::Feature kExperimentalEngineFeature{ |
| + "ExperimentalSwReporterEngine", base::FEATURE_DISABLED_BY_DEFAULT}; |
| + |
| +const char kExperimentalEngineTagParam[] = "tag"; |
| +const re2::RE2 kExperimentalEngineArgsParam = "args(\\d+)"; |
| +const re2::RE2 kExperimentalEngineSuffixParam = "suffix(\\d+)"; |
| + |
| void SRTHasCompleted(SRTCompleted value) { |
| UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.Cleaner.HasCompleted", value, |
| SRT_COMPLETED_MAX); |
| @@ -103,6 +119,15 @@ void ReportUploadsWithUma(const base::string16& upload_results) { |
| UMA_HISTOGRAM_BOOLEAN("SoftwareReporter.LastUploadResult", last_result); |
| } |
| +bool IsExperimentalEngineEnabled() { |
| + // The experiment is only enabled on x86. Finch doesn't have a way to check |
| + // this so we'll hard-code it. |
| + if (base::SysInfo::OperatingSystemArchitecture() != "x86") |
| + return false; |
| + |
| + return base::FeatureList::IsEnabled(kExperimentalEngineFeature); |
| +} |
| + |
| class SwReporterInstallerTraits : public ComponentInstallerTraits { |
| public: |
| SwReporterInstallerTraits() {} |
| @@ -130,8 +155,83 @@ class SwReporterInstallerTraits : public ComponentInstallerTraits { |
| const base::FilePath& install_dir, |
| std::unique_ptr<base::DictionaryValue> manifest) override { |
| DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
| - safe_browsing::RunSwReporter(install_dir.Append(kSwReporterExeName), |
| - version, base::ThreadTaskRunnerHandle::Get(), |
| + |
| + base::FilePath exe_path(install_dir.Append(kSwReporterExeName)); |
| + |
| + // If this user is getting the experimental engine, read the command-line |
| + // params and an UMA histogram suffix from the experiment parameters. |
| + if (IsExperimentalEngineEnabled()) { |
| + // TODO(joenotcharles): check the version or manifest to make sure we |
| + // actually got an experimental version (unless in the control group). |
| + std::vector<safe_browsing::SwReporterInvocation> invocations; |
| + |
| + // Command-line arguments are passed as args0, args1, ..., argsN. |
| + // Suffixes are passed as suffix0, suffix1, ..., suffixN. |
| + // |
| + // Read all the parameters whose names match these patterns into maps, |
| + // then make sure that all indices 0 through N are stored in both maps. |
| + // If any are missing, the experiment config is badly formatted. If it's |
| + // ok, add each pair (command line, suffix) to the invocations vector. |
| + std::map<std::string, std::string> experiment_params; |
| + if (variations::GetVariationParamsByFeature(kExperimentalEngineFeature, |
| + &experiment_params)) { |
| + std::map<int, std::string> arg_strings; |
| + std::map<int, std::string> suffixes; |
| + unsigned int max_index = 0; |
| + for (const auto& param : experiment_params) { |
| + unsigned int index = 0; |
| + if (re2::RE2::FullMatch(param.first, kExperimentalEngineArgsParam, |
| + &index)) { |
| + arg_strings[index] = param.second; |
| + max_index = std::max(index, max_index); |
| + } else if (re2::RE2::FullMatch(param.first, |
| + kExperimentalEngineSuffixParam, &index)) { |
| + suffixes[index] = param.second; |
| + max_index = std::max(index, max_index); |
| + } |
| + } |
| + |
| + invocations.reserve(max_index + 1); |
| + for (unsigned int i = 0; i < max_index; ++i) { |
| + auto args = arg_strings.find(i); |
| + auto suffix = suffixes.find(i); |
| + if (args == arg_strings.end() || |
| + suffix == suffixes.end()) { |
| + // TODO(joenotcharles): Report bad finch config. |
| + invocations.clear(); |
| + break; |
| + } |
| + |
| + // Construct the command-line using a placeholder which is guaranteed |
| + // not to need quoting for arg 0. Then replace it with the real path, |
| + // which may contain spaces and other weirdness. |
| + auto command_line = base::CommandLine::FromString(base::UTF8ToUTF16( |
| + base::StringPrintf("%s %s", kSwReporterExeName, args->second))); |
| + command_line.SetProgram(exe_path); |
| + |
| + // 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. |
| + command_line.AppendSwitchNative("registry-suffix", |
| + base::UTF8ToUTF16(suffix->second)); |
|
Joe Mason
2016/08/09 15:24:26
pmbureau: This is what I was talking about in the
|
| + |
| + invocations[i] = safe_browsing::SwReporterInvocation( |
| + command_line, suffix->second); |
| + } |
| + } |
| + |
| + // If there were no explicit command-lines found, fall through to the |
| + // default invocation which is the binary name without arguments. |
| + if (!invocations.empty()) { |
| + safe_browsing::RunExperimentalSwReporters(invocations, version, |
| + base::ThreadTaskRunnerHandle::Get(), |
| + base::WorkerPool::GetTaskRunner(true)); |
| + return; |
| + } |
| + } |
| + |
| + safe_browsing::RunSwReporter(exe_path, version, |
| + base::ThreadTaskRunnerHandle::Get(), |
| base::WorkerPool::GetTaskRunner(true)); |
| } |
| @@ -144,7 +244,16 @@ class SwReporterInstallerTraits : public ComponentInstallerTraits { |
| std::string GetName() const override { return "Software Reporter Tool"; } |
| update_client::InstallerAttributes GetInstallerAttributes() const override { |
| - 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. |
| + std::string tag = variations::GetVariationParamValueByFeature( |
| + kExperimentalEngineFeature, kExperimentalEngineTagParam); |
| + if (!tag.empty()) |
| + attributes[kExperimentalEngineTagParam] = tag; |
| + } |
| + return attributes; |
| } |
| std::vector<std::string> GetMimeTypes() const override { |
| return std::vector<std::string>(); |