Commit d99f8467 authored by Che-yu Wu's avatar Che-yu Wu Committed by Commit Bot

Chrome Cleaner: add finch switch for quarantine feature

The new quarantine feature for chrome cleanup tool needs a finch switch to control it.
It adds "--quarantine" into the command line when launching the cleaner if the switch is enabled.
It will be only enabled on the cleaner. So only adds switch to the cleaner, but not to the reporter.
Default is disabled.

Bug: crbug.com/878794
Change-Id: Ibb6f7bf76c36fbaf6106ede1ee9b6049cb2c5d2e
Reviewed-on: https://chromium-review.googlesource.com/1194404Reviewed-by: default avatarChris Sharp <csharp@chromium.org>
Commit-Queue: Che-yu Wu <cheyuw@google.com>
Cr-Commit-Position: refs/heads/master@{#587710}
parent 3d9e1d4e
...@@ -146,6 +146,10 @@ ChromeCleanerRunner::ChromeCleanerRunner( ...@@ -146,6 +146,10 @@ ChromeCleanerRunner::ChromeCleanerRunner(
std::string reboot_prompt_type = base::IntToString(GetRebootPromptType()); std::string reboot_prompt_type = base::IntToString(GetRebootPromptType());
cleaner_command_line_.AppendSwitchASCII( cleaner_command_line_.AppendSwitchASCII(
chrome_cleaner::kRebootPromptMethodSwitch, reboot_prompt_type); chrome_cleaner::kRebootPromptMethodSwitch, reboot_prompt_type);
if (base::FeatureList::IsEnabled(kChromeCleanupQuarantineFeature)) {
cleaner_command_line_.AppendSwitch(chrome_cleaner::kQuarantineSwitch);
}
} }
ChromeCleanerRunner::ProcessStatus ChromeCleanerRunner::ProcessStatus
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/test/multiprocess_test.h" #include "base/test/multiprocess_test.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread.h" #include "base/threading/thread.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/safe_browsing/chrome_cleaner/mock_chrome_cleaner_process_win.h" #include "chrome/browser/safe_browsing/chrome_cleaner/mock_chrome_cleaner_process_win.h"
...@@ -64,12 +65,14 @@ enum class ReporterEngine { ...@@ -64,12 +65,14 @@ enum class ReporterEngine {
// process running in scanning mode. // process running in scanning mode.
// - chrome_prompt (ChromePromptValue): indicates if this is a user-initiated // - chrome_prompt (ChromePromptValue): indicates if this is a user-initiated
// run or if the user was prompted. // run or if the user was prompted.
// - quarantine_enabled (bool): indicates if the quarantine feature is enabled.
class ChromeCleanerRunnerSimpleTest class ChromeCleanerRunnerSimpleTest
: public testing::TestWithParam< : public testing::TestWithParam<
std::tuple<ChromeCleanerRunner::ChromeMetricsStatus, std::tuple<ChromeCleanerRunner::ChromeMetricsStatus,
ReporterEngine, ReporterEngine,
bool, bool,
ChromePromptValue>>, ChromePromptValue,
bool>>,
public ChromeCleanerRunnerTestDelegate { public ChromeCleanerRunnerTestDelegate {
public: public:
ChromeCleanerRunnerSimpleTest() ChromeCleanerRunnerSimpleTest()
...@@ -77,7 +80,13 @@ class ChromeCleanerRunnerSimpleTest ...@@ -77,7 +80,13 @@ class ChromeCleanerRunnerSimpleTest
void SetUp() override { void SetUp() override {
std::tie(metrics_status_, reporter_engine_, cleaner_logs_enabled_, std::tie(metrics_status_, reporter_engine_, cleaner_logs_enabled_,
chrome_prompt_) = GetParam(); chrome_prompt_, quarantine_enabled_) = GetParam();
std::vector<base::Feature> enabled_features;
if (quarantine_enabled_) {
enabled_features.push_back(kChromeCleanupQuarantineFeature);
}
scoped_feature_list_.InitWithFeatures(enabled_features, {});
SetChromeCleanerRunnerTestDelegateForTesting(this); SetChromeCleanerRunnerTestDelegateForTesting(this);
} }
...@@ -149,6 +158,7 @@ class ChromeCleanerRunnerSimpleTest ...@@ -149,6 +158,7 @@ class ChromeCleanerRunnerSimpleTest
ReporterEngine reporter_engine_; ReporterEngine reporter_engine_;
bool cleaner_logs_enabled_ = false; bool cleaner_logs_enabled_ = false;
ChromePromptValue chrome_prompt_ = ChromePromptValue::kUnspecified; ChromePromptValue chrome_prompt_ = ChromePromptValue::kUnspecified;
bool quarantine_enabled_ = false;
// Set by LaunchTestProcess. // Set by LaunchTestProcess.
base::CommandLine command_line_; base::CommandLine command_line_;
...@@ -158,6 +168,9 @@ class ChromeCleanerRunnerSimpleTest ...@@ -158,6 +168,9 @@ class ChromeCleanerRunnerSimpleTest
ChromeCleanerRunner::ProcessStatus process_status_; ChromeCleanerRunner::ProcessStatus process_status_;
base::RunLoop run_loop_; base::RunLoop run_loop_;
private:
base::test::ScopedFeatureList scoped_feature_list_;
}; };
TEST_P(ChromeCleanerRunnerSimpleTest, LaunchParams) { TEST_P(ChromeCleanerRunnerSimpleTest, LaunchParams) {
...@@ -200,6 +213,9 @@ TEST_P(ChromeCleanerRunnerSimpleTest, LaunchParams) { ...@@ -200,6 +213,9 @@ TEST_P(ChromeCleanerRunnerSimpleTest, LaunchParams) {
chrome_cleaner::kRebootPromptMethodSwitch); chrome_cleaner::kRebootPromptMethodSwitch);
int reboot_prompt = -1; int reboot_prompt = -1;
EXPECT_TRUE(base::StringToInt(reboot_prompt_method, &reboot_prompt)); EXPECT_TRUE(base::StringToInt(reboot_prompt_method, &reboot_prompt));
EXPECT_EQ(quarantine_enabled_,
command_line_.HasSwitch(chrome_cleaner::kQuarantineSwitch));
} }
INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P(
...@@ -212,7 +228,8 @@ INSTANTIATE_TEST_CASE_P( ...@@ -212,7 +228,8 @@ INSTANTIATE_TEST_CASE_P(
ReporterEngine::kNewEngine), ReporterEngine::kNewEngine),
Bool(), Bool(),
Values(ChromePromptValue::kPrompted, Values(ChromePromptValue::kPrompted,
ChromePromptValue::kUserInitiated))); ChromePromptValue::kUserInitiated),
Bool()));
// Enum to be used as parameter for the ChromeCleanerRunnerTest fixture below. // Enum to be used as parameter for the ChromeCleanerRunnerTest fixture below.
enum class UwsFoundState { enum class UwsFoundState {
......
...@@ -44,6 +44,9 @@ const base::Feature kUserInitiatedChromeCleanupsFeature{ ...@@ -44,6 +44,9 @@ const base::Feature kUserInitiatedChromeCleanupsFeature{
const base::Feature kChromeCleanupDistributionFeature{ const base::Feature kChromeCleanupDistributionFeature{
"ChromeCleanupDistribution", base::FEATURE_DISABLED_BY_DEFAULT}; "ChromeCleanupDistribution", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kChromeCleanupQuarantineFeature{
"ChromeCleanupQuarantine", base::FEATURE_DISABLED_BY_DEFAULT};
bool IsInSRTPromptFieldTrialGroups() { bool IsInSRTPromptFieldTrialGroups() {
return !base::StartsWith(base::FieldTrialList::FindFullName(kSRTPromptTrial), return !base::StartsWith(base::FieldTrialList::FindFullName(kSRTPromptTrial),
kSRTPromptOffGroup, base::CompareCase::SENSITIVE); kSRTPromptOffGroup, base::CompareCase::SENSITIVE);
......
...@@ -70,6 +70,10 @@ extern const base::Feature kRebootPromptDialogFeature; ...@@ -70,6 +70,10 @@ extern const base::Feature kRebootPromptDialogFeature;
// versions will be downloaded. When not enabled, default versions will be used. // versions will be downloaded. When not enabled, default versions will be used.
extern const base::Feature kChromeCleanupDistributionFeature; extern const base::Feature kChromeCleanupDistributionFeature;
// Quarantine feature. When enabled, Chrome Cleaner will backup the removed
// files.
extern const base::Feature kChromeCleanupQuarantineFeature;
extern const char kSRTPromptTrial[]; extern const char kSRTPromptTrial[];
// Returns true if this Chrome is in a field trial group which shows the SRT // Returns true if this Chrome is in a field trial group which shows the SRT
......
...@@ -24,6 +24,7 @@ const char kSessionIdSwitch[] = "session-id"; ...@@ -24,6 +24,7 @@ const char kSessionIdSwitch[] = "session-id";
const char kSRTPromptFieldTrialGroupNameSwitch[] = "srt-field-trial-group-name"; const char kSRTPromptFieldTrialGroupNameSwitch[] = "srt-field-trial-group-name";
const char kRebootPromptMethodSwitch[] = "reboot-prompt-method"; const char kRebootPromptMethodSwitch[] = "reboot-prompt-method";
const char kUmaUserSwitch[] = "uma-user"; const char kUmaUserSwitch[] = "uma-user";
const char kQuarantineSwitch[] = "quarantine";
// Registry paths and subkeys. // Registry paths and subkeys.
const wchar_t kSoftwareRemovalToolRegistryKey[] = const wchar_t kSoftwareRemovalToolRegistryKey[] =
......
...@@ -73,6 +73,9 @@ extern const char kRebootPromptMethodSwitch[]; ...@@ -73,6 +73,9 @@ extern const char kRebootPromptMethodSwitch[];
// Indicates that metrics reporting is enabled for the current user. // Indicates that metrics reporting is enabled for the current user.
extern const char kUmaUserSwitch[]; extern const char kUmaUserSwitch[];
// Indicates that quarantine feature is enabled.
extern const char kQuarantineSwitch[];
// Registry paths where the reporter and the cleaner will write metrics data // Registry paths where the reporter and the cleaner will write metrics data
// to be reported by Chrome. // to be reported by Chrome.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment