Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(848)

Unified Diff: chrome/browser/component_updater/sw_reporter_installer_win.cc

Issue 2226133005: Add support for the ExperimentalSwReporterEngine field trial. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Comment on usage of TestPartialLaunchCycle. Always save the time of last run. Misc cleanups. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..fb6cc2a82152a1df8e84073576cf8109a0d11d37 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,83 @@ 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;
- }
+ 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;
+ }
- // 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;
- }
+ // 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;
- // 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;
- }
+ // The suffix must be an alphanumeric string. (Empty is fine as long as the
+ // "suffix" key is present.)
+ std::string suffix;
+ if (!invocation_params->GetString("suffix", &suffix) ||
+ !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.)
+ const base::ListValue* arguments = nullptr;
+ if (!invocation_params->GetList("arguments", &arguments)) {
ReportExperimentError(SW_REPORTER_EXPERIMENT_ERROR_BAD_PARAMS);
return;
}
- if (!argument.empty())
- argv.push_back(argument);
- }
- base::CommandLine command_line(argv);
+ std::vector<base::string16> argv = {exe_path.value()};
+ 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);
}
} // namespace
@@ -268,8 +278,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 +417,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 =

Powered by Google App Engine
This is Rietveld 408576698