Basic IPC communication between Chrome and the SW Reporter.

chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc

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..de4d310c20f55faff7258d4959141ea9a94a2dd6 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
@@ -7,16 +7,22 @@
 #include <initializer_list>
 #include <iterator>

[joenotcharles, 2017/04/06 17:57:15]

No longer needed (was used for std::begin / std::end)

[ftirelo, 2017/04/06 20:41:47]

Done.

 #include <set>
+#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,20 +36,180 @@
 #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 passed as the expected exit code to be report by tests.

[joenotcharles, 2017/04/06 17:57:14]

Typo: "reported"

[ftirelo, 2017/04/06 20:41:47]

Done.

+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 equals 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;
+  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

[joenotcharles, 2017/04/06 17:57:15]

I think "sets" should start a new sentence.

[ftirelo, 2017/04/06 20:41:47]

Done.

+// 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 callback |done|.

[joenotcharles, 2017/04/06 17:57:14]

That's not exactly true, it's deleted by PromptUserCallback.

[ftirelo, 2017/04/06 20:41:47]

Done.

+  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 send 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 base::CommandLine& command_line) {
+  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();
+  const std::string& chrome_mojo_pipe_token =
+      command_line.GetSwitchValueASCII(kChromeMojoPipeTokenSwitch);
+  if (chrome_mojo_pipe_token.empty())
+    return false;
+
+  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's process 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;
+}
 
-class SRTFetcherTest : public InProcessBrowserTest,
-                       public SwReporterTestingDelegate {
+// 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);
+  int exit_code_to_report = kFailureExitCode;
+  bool success = base::StringToInt(str, &exit_code_to_report) &&
+                 (!command_line->HasSwitch(kChromeMojoPipeTokenSwitch) ||

[joenotcharles, 2017/04/06 17:57:14]

Nit: I think it'd be cleaner to read chrome_mojo_pipe_token in this func, and
then pass it to ConnectAndSendDataToParentProcess. That way you're not reading
kChromeMojoPipeTokenSwitch in two separate functions.

[ftirelo, 2017/04/06 20:41:47]

Done.

+                  ConnectAndSendDataToParentProcess(*command_line));
+  return success ? exit_code_to_report : kFailureExitCode;
+}
+
+// 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>> {

[joenotcharles, 2017/04/06 17:57:14]

Please include <tuple> for this.

[ftirelo, 2017/04/06 20:41:47]

Done.

  public:
   void SetUpInProcessBrowserTestFixture() override {
     SetSwReporterTestingDelegate(this);
@@ -63,6 +229,11 @@ class SRTFetcherTest : public InProcessBrowserTest,
         base::TimeDelta::FromDays(kDaysBetweenSuccessfulSwReporterRuns * 2));
 
     ClearLastTimeSentReport();
+
+    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_);
   }
 
   void TearDownOnMainThread() override {
@@ -80,14 +251,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| and 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.
@@ -234,22 +428,31 @@ class SRTFetcherTest : public InProcessBrowserTest,
   // for reporter logging.
   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.

[joenotcharles, 2017/04/06 17:57:15]

This comment doesn't match the code anymore.

[ftirelo, 2017/04/06 20:41:47]

Done.

-    for (const std::string& expected_switch : expected_switches) {
-      EXPECT_NE(invocation_switches.end(),
-                invocation_switches.find(expected_switch));
+    for (const std::string& logging_switch : logging_switches) {
+      EXPECT_EQ(expect_switches, invocation_switches.find(logging_switch) !=
+                                     invocation_switches.end());
     }
   }
 
+  std::unique_ptr<base::test::ScopedFeatureList> GetScopedFeatureList() {
+    std::unique_ptr<base::test::ScopedFeatureList> scoped_feature_list;
+    scoped_feature_list.reset(new base::test::ScopedFeatureList());

[joenotcharles, 2017/04/06 17:57:15]

Prefer "auto scoped_feature_list = base::MakeUnique<...>"

[ftirelo, 2017/04/06 20:41:47]

Not a unique_ptr anymore.

+    if (in_browser_cleaner_ui_)
+      scoped_feature_list->InitAndEnableFeature(kInBrowserCleanerUIFeature);
+    else
+      scoped_feature_list->InitAndDisableFeature(kInBrowserCleanerUIFeature);
+    return scoped_feature_list;
+  }
+
   // A task runner with a mock clock.
   scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner_ =
       new base::TestMockTimeTaskRunner();
@@ -257,6 +460,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_;
@@ -283,19 +489,25 @@ class SRTFetcherPromptTest : public DialogBrowserTest {
 
 }  // namespace
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, NothingFound) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, NothingFound) {
+  auto scoped_feature_list = GetScopedFeatureList();

[joenotcharles, 2017/04/06 17:57:14]

Instead of putting this at the start of every test, can you make it a member var
of SRTFetcherTest and initialize it in the constructor?

[ftirelo, 2017/04/06 20:41:47]

Done.

+
   RunReporter(kSwReporterNothingFound);
   ExpectReporterLaunches(0, 1, false);
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, CleanupNeeded) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, CleanupNeeded) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   RunReporter(kSwReporterCleanupNeeded);
   ExpectReporterLaunches(0, 1, true);
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RanRecently) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, RanRecently) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   constexpr int kDaysLeft = 1;
   SetDaysSinceLastTriggered(kDaysBetweenSuccessfulSwReporterRuns - kDaysLeft);
   RunReporter(kSwReporterNothingFound);
@@ -306,7 +518,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   Profile* profile = browser()->profile();
 
   // Ensure that even though we're closing the last browser, we don't enter the
@@ -346,13 +560,17 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, DISABLED_WaitForBrowser) {
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, Failure) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, Failure) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   RunReporter(kReporterFailureExitCode);
   ExpectReporterLaunches(0, 1, false);
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RunDaily) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, RunDaily) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   PrefService* local_state = g_browser_process->local_state();
   local_state->SetBoolean(prefs::kSwReporterPendingPrompt, true);
   SetDaysSinceLastTriggered(kDaysBetweenSuccessfulSwReporterRuns - 1);
@@ -381,7 +599,9 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, RunDaily) {
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, ParameterChange) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   // 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 +658,9 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
   ExpectToRunAgain(kDaysBetweenSuccessfulSwReporterRuns);
 }
 
-IN_PROC_BROWSER_TEST_F(SRTFetcherTest, MultipleLaunches) {
+IN_PROC_BROWSER_TEST_P(SRTFetcherTest, MultipleLaunches) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   const base::FilePath path1(L"path1");
   const base::FilePath path2(L"path2");
   const base::FilePath path3(L"path3");
@@ -506,8 +728,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   RunReporter(kSwReporterNothingFound);
   ExpectReporterLaunches(0, 1, false);
   ExpectLoggingSwitches(reporter_launch_parameters_.front(), false);
@@ -515,8 +738,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   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 +752,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   // 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 +766,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   // 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 +782,9 @@ 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) {
+  auto scoped_feature_list = GetScopedFeatureList();
+
   EnableSBExtendedReporting();
   SetLastTimeSentReport(kDaysBetweenReporterLogsSent + 3);
 
@@ -593,6 +820,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