Index: chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc |
diff --git a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc |
index ad157be9a251e05d48bbd32021cb0a5a5e615357..60053da7e6aeaf67233d9afea955a2f5db815d7d 100644 |
--- a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc |
+++ b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc |
@@ -5,18 +5,24 @@ |
#include "chrome/browser/safe_browsing/srt_fetcher_win.h" |
#include <initializer_list> |
-#include <iterator> |
#include <set> |
+#include <tuple> |
+#include <utility> |
#include <vector> |
#include "base/bind.h" |
#include "base/bind_helpers.h" |
#include "base/callback.h" |
+#include "base/callback_helpers.h" |
#include "base/files/file_path.h" |
#include "base/memory/ref_counted.h" |
#include "base/message_loop/message_loop.h" |
+#include "base/run_loop.h" |
+#include "base/strings/string_number_conversions.h" |
+#include "base/test/multiprocess_test.h" |
#include "base/test/scoped_feature_list.h" |
#include "base/test/test_mock_time_task_runner.h" |
+#include "base/threading/sequenced_task_runner_handle.h" |
#include "base/threading/thread_task_runner_handle.h" |
#include "base/time/time.h" |
#include "base/version.h" |
@@ -30,23 +36,197 @@ |
#include "chrome/browser/ui/test/test_browser_dialog.h" |
#include "chrome/common/pref_names.h" |
#include "chrome/test/base/in_process_browser_test.h" |
+#include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h" |
#include "components/component_updater/pref_names.h" |
#include "components/prefs/pref_service.h" |
#include "components/safe_browsing_db/safe_browsing_prefs.h" |
+#include "mojo/edk/embedder/embedder.h" |
+#include "mojo/edk/embedder/scoped_ipc_support.h" |
+#include "mojo/edk/system/core.h" |
+#include "testing/multiprocess_func_list.h" |
namespace safe_browsing { |
namespace { |
-const char* const kExpectedSwitches[] = {kExtendedSafeBrowsingEnabledSwitch, |
- kChromeVersionSwitch, |
- kChromeChannelSwitch}; |
+using chrome_cleaner::mojom::PromptAcceptance; |
+ |
+// Special switches passed by the parent process (test case) to the reporter |
+// child process to indicate the behavior that should be mocked. |
+constexpr char kExitCodeToReturnSwitch[] = "exit-code-to-return"; |
+constexpr char kReportUwSFoundSwitch[] = "report-uws-found"; |
+constexpr char kReportElevationRequiredSwitch[] = "report-elevation-required"; |
+constexpr char kExpectedPromptAcceptanceSwitch[] = "expected-prompt-acceptance"; |
+ |
+// The exit code to be returned in case of failure in the child process. |
+// This should never be passed as the expected exit code to be reported by |
+// tests. |
+constexpr int kFailureExitCode = -1; |
+ |
+// Pass the |prompt_acceptance| to the mock child process in command line |
+// switch kExpectedPromptAcceptanceSwitch. |
+void AddPromptAcceptanceToCommandLine(PromptAcceptance prompt_acceptance, |
+ base::CommandLine* command_line) { |
+ command_line->AppendSwitchASCII( |
+ kExpectedPromptAcceptanceSwitch, |
+ base::IntToString(static_cast<int>(prompt_acceptance))); |
+} |
+ |
+// Parses and returns the prompt acceptance value passed by the parent process |
+// in command line switch kExpectedPromptAcceptanceSwitch. Returns |
+// PromptAcceptance::UNSPECIFIED if the switch doesn't exist or can't be |
+// parsed to a valid PromptAcceptance enumerator. |
+PromptAcceptance PromptAcceptanceFromCommandLine( |
+ const base::CommandLine& command_line) { |
+ const std::string& prompt_acceptance_str = |
+ command_line.GetSwitchValueASCII(kExpectedPromptAcceptanceSwitch); |
+ int val = -1; |
+ if (base::StringToInt(prompt_acceptance_str, &val)) { |
+ PromptAcceptance prompt_acceptance = static_cast<PromptAcceptance>(val); |
+ if (chrome_cleaner::mojom::IsKnownEnumValue(prompt_acceptance)) |
+ return prompt_acceptance; |
+ } |
+ return PromptAcceptance::UNSPECIFIED; |
+} |
+ |
+// Pointer to ChromePromptPtr object to be used by the child process. The |
+// object must be created, deleted, and accessed on the IPC thread only. |
+chrome_cleaner::mojom::ChromePromptPtr* g_chrome_prompt_ptr = nullptr; |
+ |
+// The callback function to be passed to ChromePrompt::PromptUser to check if |
+// the prompt accepted returned by the parent process is equal to |
+// |expected_prompt_acceptance|. Will set |expected_value_received| with the |
+// comparison result, so that the main thread can report failure. Will invoke |
+// |done| callback when done. |
+void PromptUserCallback(const base::Closure& done, |
+ PromptAcceptance expected_prompt_acceptance, |
+ bool* expected_value_received, |
+ PromptAcceptance prompt_acceptance) { |
+ *expected_value_received = prompt_acceptance == expected_prompt_acceptance; |
+ // It's safe to delete the ChromePromptPtr object here, since it will not be |
+ // used anymore by the child process. |
+ delete g_chrome_prompt_ptr; |
+ g_chrome_prompt_ptr = nullptr; |
+ done.Run(); |
+} |
+ |
+// Mocks the sending of scan results from the child process to the parent |
+// process. Obtains the behavior to be mocked from special switches in |
+// |command_line|. Sets |expected_value_received| as true if the parent |
+// process replies with the expected prompt acceptance value. |
+void SendScanResults(const std::string& chrome_mojo_pipe_token, |
+ const base::CommandLine& command_line, |
+ const base::Closure& done, |
+ bool* expected_value_received) { |
+ constexpr int kDefaultUwSId = 10; |
+ constexpr char kDefaultUwSName[] = "RemovedUwS"; |
+ |
+ mojo::ScopedMessagePipeHandle message_pipe_handle = |
+ mojo::edk::CreateChildMessagePipe(chrome_mojo_pipe_token); |
+ // This pointer will be deleted by PromptUserCallback. |
+ g_chrome_prompt_ptr = new chrome_cleaner::mojom::ChromePromptPtr(); |
+ g_chrome_prompt_ptr->Bind(chrome_cleaner::mojom::ChromePromptPtrInfo( |
+ std::move(message_pipe_handle), 0)); |
+ |
+ std::vector<chrome_cleaner::mojom::UwSPtr> removable_uws_found; |
+ if (command_line.HasSwitch(kReportUwSFoundSwitch)) { |
+ chrome_cleaner::mojom::UwSPtr uws = chrome_cleaner::mojom::UwS::New(); |
+ uws->id = kDefaultUwSId; |
+ uws->name = kDefaultUwSName; |
+ uws->observed_behaviours = chrome_cleaner::mojom::ObservedBehaviours::New(); |
+ removable_uws_found.push_back(std::move(uws)); |
+ } |
+ const bool elevation_required = |
+ command_line.HasSwitch(kReportElevationRequiredSwitch); |
+ const PromptAcceptance expected_prompt_acceptance = |
+ PromptAcceptanceFromCommandLine(command_line); |
+ |
+ (*g_chrome_prompt_ptr) |
+ ->PromptUser( |
+ std::move(removable_uws_found), elevation_required, |
+ base::Bind(&PromptUserCallback, done, expected_prompt_acceptance, |
+ expected_value_received)); |
+} |
+ |
+// Connects to the parent process and sends mocked scan results. Returns true |
+// if connection was successful and the prompt acceptance results sent by the |
+// parent process are the same as expected. |
+bool ConnectAndSendDataToParentProcess( |
+ const std::string& chrome_mojo_pipe_token, |
+ const base::CommandLine& command_line) { |
+ DCHECK(!chrome_mojo_pipe_token.empty()); |
+ |
+ mojo::edk::Init(); |
+ base::Thread::Options options(base::MessageLoop::TYPE_IO, 0); |
+ base::Thread io_thread("IPCThread"); |
+ if (!io_thread.StartWithOptions(options)) |
+ return false; |
+ mojo::edk::ScopedIPCSupport ipc_support( |
+ io_thread.task_runner(), |
+ mojo::edk::ScopedIPCSupport::ShutdownPolicy::CLEAN); |
+ mojo::edk::SetParentPipeHandleFromCommandLine(); |
+ base::MessageLoop message_loop; |
+ base::RunLoop run_loop; |
+ // After the response from the parent process is received, this will post a |
+ // task to unblock the child process's main thread. |
+ auto done = base::Bind( |
+ [](scoped_refptr<base::SequencedTaskRunner> main_runner, |
+ base::Closure quit_closure) { |
+ main_runner->PostTask(FROM_HERE, std::move(quit_closure)); |
+ }, |
+ base::SequencedTaskRunnerHandle::Get(), |
+ base::Passed(run_loop.QuitClosure())); |
+ |
+ bool expected_value_received = false; |
+ io_thread.task_runner()->PostTask( |
+ FROM_HERE, base::Bind(&SendScanResults, chrome_mojo_pipe_token, |
+ command_line, done, &expected_value_received)); |
+ run_loop.Run(); |
+ |
+ return expected_value_received; |
+} |
+ |
+// Mocks a Software Reporter process that returns an exit code specified by |
+// command line switch kExitCodeToReturnSwitch. If a Mojo IPC is available, |
+// this will also connect to the parent process and send mocked scan results |
+// to the parent process using data passed as command line switches. |
+MULTIPROCESS_TEST_MAIN(MockSwReporterProcess) { |
+ base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); |
+ const std::string& str = |
+ command_line->GetSwitchValueASCII(kExitCodeToReturnSwitch); |
+ const std::string& chrome_mojo_pipe_token = |
+ command_line->GetSwitchValueASCII(kChromeMojoPipeTokenSwitch); |
+ int exit_code_to_report = kFailureExitCode; |
+ bool success = base::StringToInt(str, &exit_code_to_report) && |
+ (chrome_mojo_pipe_token.empty() || |
+ ConnectAndSendDataToParentProcess(chrome_mojo_pipe_token, |
+ *command_line)); |
+ return success ? exit_code_to_report : kFailureExitCode; |
+} |
-class SRTFetcherTest : public InProcessBrowserTest, |
- public SwReporterTestingDelegate { |
+// Parameters for this test: |
+// - bool in_browser_cleaner_ui: indicates if InBrowserCleanerUI experiment |
+// is enabled; if so, the parent and the child processes will communicate |
+// via a Mojo IPC; |
+// - bool elevation_required: indicates if the scan results sent by the child |
+// process should consider that elevation will be required for cleanup. |
+class SRTFetcherTest |
+ : public InProcessBrowserTest, |
+ public SwReporterTestingDelegate, |
+ public ::testing::WithParamInterface<std::tuple<bool, bool>> { |
public: |
void SetUpInProcessBrowserTestFixture() override { |
SetSwReporterTestingDelegate(this); |
+ |
+ std::tie(in_browser_cleaner_ui_, elevation_required_) = GetParam(); |
+ // The config should only accept elevation_required_ if InBrowserCleanerUI |
+ // feature is enabled. |
+ ASSERT_TRUE(!elevation_required_ || in_browser_cleaner_ui_); |
+ |
+ if (in_browser_cleaner_ui_) |
+ scoped_feature_list_.InitAndEnableFeature(kInBrowserCleanerUIFeature); |
+ else |
+ scoped_feature_list_.InitAndDisableFeature(kInBrowserCleanerUIFeature); |
} |
void SetUpOnMainThread() override { |
@@ -80,14 +260,37 @@ class SRTFetcherTest : public InProcessBrowserTest, |
prompt_trigger_called_ = true; |
} |
- // Records that the reporter was launched with the parameters given in |
- // |invocation|. |
- int LaunchReporter(const SwReporterInvocation& invocation) override { |
+ // Spawns and returns a subprocess to mock an execution of the reporter with |
+ // the parameters given in |invocation| that will return |
+ // |exit_code_to_report_|. If IPC communication needs to be mocked, this will |
+ // also provide values that define the expected behavior of the child |
+ // process. |
+ // Records the launch and parameters used for further verification. |
+ base::Process LaunchReporter( |
+ const SwReporterInvocation& invocation, |
+ const base::LaunchOptions& launch_options) override { |
++reporter_launch_count_; |
reporter_launch_parameters_.push_back(invocation); |
if (first_launch_callback_) |
std::move(first_launch_callback_).Run(); |
- return exit_code_to_report_; |
+ |
+ base::CommandLine command_line( |
+ base::GetMultiProcessTestChildBaseCommandLine()); |
+ command_line.AppendArguments(invocation.command_line, |
+ /*include_program=*/false); |
+ command_line.AppendSwitchASCII(kExitCodeToReturnSwitch, |
+ base::IntToString(exit_code_to_report_)); |
+ if (in_browser_cleaner_ui_) { |
+ AddPromptAcceptanceToCommandLine(PromptAcceptance::DENIED, &command_line); |
+ if (exit_code_to_report_ == kSwReporterCleanupNeeded) { |
+ command_line.AppendSwitch(kReportUwSFoundSwitch); |
+ if (elevation_required_) |
+ command_line.AppendSwitch(kReportElevationRequiredSwitch); |
+ } |
+ } |
+ base::SpawnChildResult result = base::SpawnMultiProcessTestChild( |
+ "MockSwReporterProcess", command_line, launch_options); |
+ return std::move(result.process); |
} |
// Returns the test's idea of the current time. |
@@ -231,22 +434,20 @@ class SRTFetcherTest : public InProcessBrowserTest, |
} |
// Expects |invocation|'s command line to contain all the switches required |
- // for reporter logging. |
+ // for reporter logging if and only if |expect_switches| is true. |
void ExpectLoggingSwitches(const SwReporterInvocation& invocation, |
bool expect_switches) { |
+ static const std::set<std::string> logging_switches{ |
+ kExtendedSafeBrowsingEnabledSwitch, kChromeVersionSwitch, |
+ kChromeChannelSwitch}; |
+ |
const base::CommandLine::SwitchMap& invocation_switches = |
invocation.command_line.GetSwitches(); |
- std::set<std::string> expected_switches; |
- if (expect_switches) |
- expected_switches = {std::begin(kExpectedSwitches), |
- std::end(kExpectedSwitches)}; |
- EXPECT_EQ(expected_switches.size(), invocation_switches.size()); |
- // Checks if all expected switches are in the invocation switches. It's not |
- // necessary to check if all invocation switches are expected, since we |
- // checked if both sets should have the same size. |
- for (const std::string& expected_switch : expected_switches) { |
- EXPECT_NE(invocation_switches.end(), |
- invocation_switches.find(expected_switch)); |
+ // Checks if switches that enable logging on the reporter are present on |
+ // the invocation if and only if logging is allowed. |
+ for (const std::string& logging_switch : logging_switches) { |
+ EXPECT_EQ(expect_switches, invocation_switches.find(logging_switch) != |
+ invocation_switches.end()); |
} |
} |
@@ -257,6 +458,9 @@ class SRTFetcherTest : public InProcessBrowserTest, |
// The task runner that was in use before installing |mock_time_task_runner_|. |
scoped_refptr<base::SingleThreadTaskRunner> saved_task_runner_; |
+ bool in_browser_cleaner_ui_; |
+ bool elevation_required_; |
+ |
bool prompt_trigger_called_ = false; |
int reporter_launch_count_ = 0; |
std::vector<SwReporterInvocation> reporter_launch_parameters_; |
@@ -266,6 +470,8 @@ class SRTFetcherTest : public InProcessBrowserTest, |
// can be used to perform actions in the middle of a queue of reporters which |
// all launch on the same mock clock tick. |
base::OnceClosure first_launch_callback_; |
+ |
+ base::test::ScopedFeatureList scoped_feature_list_; |
}; |
class SRTFetcherPromptTest : public DialogBrowserTest { |
@@ -283,19 +489,19 @@ class SRTFetcherPromptTest : public DialogBrowserTest { |
} // namespace |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, NothingFound) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, NothingFound) { |
RunReporter(kSwReporterNothingFound); |
ExpectReporterLaunches(0, 1, false); |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, CleanupNeeded) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, CleanupNeeded) { |
RunReporter(kSwReporterCleanupNeeded); |
ExpectReporterLaunches(0, 1, true); |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RanRecently) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, RanRecently) { |
constexpr int kDaysLeft = 1; |
SetDaysSinceLastTriggered(kDaysBetweenSuccessfulSwReporterRuns - kDaysLeft); |
RunReporter(kSwReporterNothingFound); |
@@ -306,7 +512,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RanRecently) { |
} |
// Test is flaky. crbug.com/705608 |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, DISABLED_WaitForBrowser) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, DISABLED_WaitForBrowser) { |
Profile* profile = browser()->profile(); |
// Ensure that even though we're closing the last browser, we don't enter the |
@@ -346,13 +552,13 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, DISABLED_WaitForBrowser) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, Failure) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, Failure) { |
RunReporter(kReporterFailureExitCode); |
ExpectReporterLaunches(0, 1, false); |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RunDaily) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, RunDaily) { |
PrefService* local_state = g_browser_process->local_state(); |
local_state->SetBoolean(prefs::kSwReporterPendingPrompt, true); |
SetDaysSinceLastTriggered(kDaysBetweenSuccessfulSwReporterRuns - 1); |
@@ -381,7 +587,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RunDaily) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ParameterChange) { |
// If the reporter is run several times with different parameters, it should |
// only be launched once, with the last parameter set. |
const base::FilePath path1(L"path1"); |
@@ -438,7 +644,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, MultipleLaunches) { |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, MultipleLaunches) { |
const base::FilePath path1(L"path1"); |
const base::FilePath path2(L"path2"); |
const base::FilePath path3(L"path3"); |
@@ -506,8 +712,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, MultipleLaunches) { |
} |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_NoSBExtendedReporting) { |
- base::test::ScopedFeatureList scoped_feature_list; |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ReporterLogging_NoSBExtendedReporting) { |
RunReporter(kSwReporterNothingFound); |
ExpectReporterLaunches(0, 1, false); |
ExpectLoggingSwitches(reporter_launch_parameters_.front(), false); |
@@ -515,8 +720,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_NoSBExtendedReporting) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledFirstRun) { |
- base::test::ScopedFeatureList scoped_feature_list; |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ReporterLogging_EnabledFirstRun) { |
EnableSBExtendedReporting(); |
// Note: don't set last time sent logs in the local state. |
// SBER is enabled and there is no record in the local state of the last time |
@@ -528,8 +732,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledFirstRun) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledNoRecentLogging) { |
- base::test::ScopedFeatureList scoped_feature_list; |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ReporterLogging_EnabledNoRecentLogging) { |
// SBER is enabled and last time logs were sent was more than |
// |kDaysBetweenReporterLogsSent| day ago, so we should send logs in this run. |
EnableSBExtendedReporting(); |
@@ -541,8 +744,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledNoRecentLogging) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledRecentlyLogged) { |
- base::test::ScopedFeatureList scoped_feature_list; |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ReporterLogging_EnabledRecentlyLogged) { |
// SBER is enabled, but logs have been sent less than |
// |kDaysBetweenReporterLogsSent| day ago, so we shouldn't send any logs in |
// this run. |
@@ -556,8 +758,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledRecentlyLogged) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_MultipleLaunches) { |
- base::test::ScopedFeatureList scoped_feature_list; |
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ReporterLogging_MultipleLaunches) { |
EnableSBExtendedReporting(); |
SetLastTimeSentReport(kDaysBetweenReporterLogsSent + 3); |
@@ -593,6 +794,16 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_MultipleLaunches) { |
ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns); |
} |
+INSTANTIATE_TEST_CASE_P(NoInBrowserCleanerUI, |
+ SRTFetcherTest, |
+ testing::Combine(testing::Values(false), |
+ testing::Values(false))); |
+ |
+INSTANTIATE_TEST_CASE_P(InBrowserCleanerUI, |
+ SRTFetcherTest, |
+ testing::Combine(testing::Values(true), |
+ testing::Bool())); |
+ |
// This provide tests which allows explicit invocation of the SRT Prompt |
// useful for checking dialog layout or any other interactive functionality |
// tests. See docs/testing/test_browser_dialog.md for description of the |