Add support for the ExperimentalSwReporterEngine field trial.

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 e8100bf555ec1cc4b5a83912a409603936617993..943de73f4d9ce51fbe2b00f5b5ae3ad7c57d16af 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
@@ -53,8 +53,21 @@ class SRTFetcherTest : public InProcessBrowserTest,
   }
 
   void RunReporter(const base::FilePath& exe_path = base::FilePath()) {
-    RunSwReporter(SwReporterInvocation::FromFilePath(exe_path),
-                  base::Version("1.2.3"), task_runner_, task_runner_);
+    auto invocation = SwReporterInvocation::FromFilePath(exe_path);
+    invocation.flags = SwReporterInvocation::FLAG_LOG_TO_RAPPOR |
+                       SwReporterInvocation::FLAG_LOG_EXIT_CODE_TO_PREFS |
+                       SwReporterInvocation::FLAG_TRIGGER_PROMPT |
+                       SwReporterInvocation::FLAG_SEND_REPORTER_LOGS;
+
+    SwReporterQueue invocations;
+    invocations.push(invocation);
+    RunSwReporters(invocations, base::Version("1.2.3"), task_runner_,
+                   task_runner_);
+  }
+
+  void RunReporterQueue(const SwReporterQueue& invocations) {
+    RunSwReporters(invocations, base::Version("1.2.3"), task_runner_,
+                   task_runner_);
   }
 
   void TriggerPrompt(Browser* browser, const std::string& version) override {
@@ -135,15 +148,26 @@ class SRTFetcherTest : public InProcessBrowserTest,
     EXPECT_LT(last_time_sent_logs, now);
   }
 
-  void TestReporterLaunchCycle(int expected_launch_count,
-                               const base::FilePath& expected_launch_path) {
+  // Run through the steps needed to launch the reporter, as many times as
+  // needed to launch all the reporters given in |expected_launch_paths|. Test
+  // that each of those launches succeeded. But do not test that ONLY those
+  // launches succeeded.
+  //
+  // After this, if more launches are expected you can call
+  // |TestPartialLaunchCycle| again with another list of paths, to test that
+  // the launch cycle will continue with those paths.
+  //
+  // To test that a list of paths are launched AND NO OTHERS, use
+  // |TestReporterLaunchCycle|.
+  void TestPartialLaunchCycle(
+      const std::vector<base::FilePath>& expected_launch_paths) {
     // This test has an unfortunate amount of knowledge of the internals of
     // ReporterRunner, because it needs to pump the right message loops at the
     // right time so that all its internal messages are delivered. This
     // function might need to be updated if the internals change.
     //
     // The basic sequence is:
-
+    //
     // 1. TryToRun kicks the whole thing off. If the reporter should not be
     // launched now (eg. DaysSinceLastReport is too low) it posts a call to
     // itself again. (In a regular task runner this will be scheduled with a
@@ -171,47 +195,100 @@ class SRTFetcherTest : public InProcessBrowserTest,
     ASSERT_TRUE(task_runner_->HasPendingTask());
     ASSERT_FALSE(reporter_done_notified_);
 
-    // Clear out any pending TryToRun calls. Use a bounded loop so that if the
-    // reporter will never be called, we'll eventually continue. (If
-    // LaunchAndWaitForExit was pending but TryToRun wasn't, this will call
-    // it.)
-    const int max_expected_launch_count = reporter_launch_count_ + 1;
-    for (int i = 0; !launch_ready_notified_ && i < 100; ++i)
+    reporter_launch_count_ = 0;
+    reporter_launch_parameters_ = SwReporterInvocation();
+
+    int current_launch_count = reporter_launch_count_;
+    for (const auto& expected_launch_path : expected_launch_paths) {
+      // If RunReporter was called with no pending messages, and it was already
+      // time to launch the reporter, then |launch_ready_notified_| will
+      // already be true. Otherwise there will be a TryToRun message pending,
+      // which must be processed first.
+      if (!launch_ready_notified_) {
+        task_runner_->RunPendingTasks();
+        // Since we're expecting a launch here, we expect it to schedule
+        // LaunchAndWaitForExit. So NOW |launch_ready_notified_| should be
+        // true.
+        ASSERT_TRUE(task_runner_->HasPendingTask());
+      }
+      ASSERT_TRUE(launch_ready_notified_);
+      ASSERT_EQ(current_launch_count, reporter_launch_count_);
+
+      // Reset |launch_ready_notified_| so that we can tell if TryToRun gets
+      // called again unexpectedly.
+      launch_ready_notified_ = false;
+
+      // Call the pending LaunchAndWaitForExit.
       task_runner_->RunPendingTasks();
-    ASSERT_GE(max_expected_launch_count, reporter_launch_count_);
-
-    // Reset launch_ready_notified_ so that we can tell if TryToRun gets called
-    // again unexpectedly.
-    launch_ready_notified_ = false;
+      ASSERT_FALSE(launch_ready_notified_);
+      ASSERT_FALSE(reporter_done_notified_);
+
+      // At this point LaunchAndWaitForExit has definitely been called if
+      // it's going to be called at all. (If not, TryToRun will have been
+      // scheduled again.)
+      EXPECT_EQ(current_launch_count + 1, reporter_launch_count_);
+      EXPECT_EQ(expected_launch_path,
+                reporter_launch_parameters_.command_line.GetProgram());
+
+      // Pump the UI message loop to process the ReporterDone call (which
+      // will schedule the next TryToRun.) If LaunchAndWaitForExit wasn't
+      // called, this does nothing.
+      base::RunLoop().RunUntilIdle();
+
+      // At this point there are three things that could have happened:
+      //
+      // 1. LaunchAndWaitForExit was not called. There should be a TryToRun
+      // scheduled.
+      //
+      // 2. ReporterDone was called and there was nothing left in the queue
+      // of SwReporterInvocation's. There should be a TryToRun scheduled.
+      //
+      // 3. ReporterDone was called and there were more
+      // SwReporterInvocation's in the queue to run immediately. There should
+      // be a LaunchAndWaitForExit scheduled.
+      //
+      // So in all cases there should be a pending task, and if we are expecting
+      // more launches in this loop, |launch_ready_notified_| will already be
+      // true.
+      ASSERT_TRUE(task_runner_->HasPendingTask());
+
+      // The test task runner does not actually advance the clock. Pretend that
+      // one day has passed. (Otherwise, when we launch the last
+      // SwReporterInvocation in the queue, the next call to TryToRun will
+      // start a whole new launch cycle.)
+      SetDaysSinceLastReport(1);
+
+      reporter_done_notified_ = false;
+      current_launch_count = reporter_launch_count_;
+    }
+  }
 
-    // This will trigger LaunchAndWaitForExit if it's on the queue and hasn't
-    // been called already. If it was called already, this will do nothing.
-    // (There definitely isn't anything queued up yet after
-    // LaunchAndWaitForExit because ReporterDone wasn't called yet.)
-    task_runner_->RunPendingTasks();
-    ASSERT_GE(max_expected_launch_count, reporter_launch_count_);
-    ASSERT_FALSE(launch_ready_notified_);
+  // Run through the steps needed to launch the reporter, as many times as
+  // needed to launch all the reporters given in |expected_launch_paths|. Test
+  // that each of those launches succeeded. Then, run through the steps needed
+  // to launch the reporter again, to test that the launch cycle is complete
+  // (no more reporters will be launched).
+  void TestReporterLaunchCycle(
+      const std::vector<base::FilePath>& expected_launch_paths) {
+    TestPartialLaunchCycle(expected_launch_paths);
+
+    // Now that all expected launches have been tested, run the cycle once more
+    // to make sure no more launches happen.
+    ASSERT_TRUE(task_runner_->HasPendingTask());
     ASSERT_FALSE(reporter_done_notified_);
+    ASSERT_FALSE(launch_ready_notified_);
 
-    // At this point LaunchAndWaitForExit has definitely been called if it's
-    // going to be called at all. (If not, TryToRun will have been scheduled
-    // again.)
-    EXPECT_EQ(expected_launch_count, reporter_launch_count_);
-    EXPECT_EQ(expected_launch_path,
-              reporter_launch_parameters_.command_line.GetProgram());
+    int current_launch_count = reporter_launch_count_;
 
-    // Pump the UI message loop to process the ReporterDone call (which will
-    // schedule the next TryToRun.) If LaunchAndWaitForExit wasn't called, this
-    // does nothing.
-    base::RunLoop().RunUntilIdle();
+    // Call the pending TryToRun.
+    task_runner_->RunPendingTasks();
 
-    // At this point another call to TryToRun should be scheduled, whether or
-    // not LaunchAndWaitForExit was called.
+    // We expect that this scheduled another TryToRun. If it scheduled
+    // LaunchAndWaitForExit an unexpected launch is about to happen.
     ASSERT_TRUE(task_runner_->HasPendingTask());
-
-    // Make sure the flags are false for the next launch cycle test.
     ASSERT_FALSE(launch_ready_notified_);
-    reporter_done_notified_ = false;
+    ASSERT_FALSE(reporter_done_notified_);
+    ASSERT_EQ(current_launch_count, reporter_launch_count_);
   }
 
   // Expects |reporter_launch_parameters_| to contain exactly the command line
@@ -243,7 +320,11 @@ class SRTFetcherTest : public InProcessBrowserTest,
   int reporter_launch_count_ = 0;
   SwReporterInvocation reporter_launch_parameters_;
   int exit_code_to_report_ = kReporterFailureExitCode;
+
+  // This will be set to true when a call to |LaunchAndWaitForExit| is next in
+  // the task queue.
   bool launch_ready_notified_ = false;
+
   bool reporter_done_notified_ = false;
 };
 
@@ -352,7 +433,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
     SCOPED_TRACE("N days left until next reporter run");
     SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns - kDaysLeft);
     RunReporter(path1);
-    TestReporterLaunchCycle(0, base::FilePath());
+    TestReporterLaunchCycle({});
   }
 
   // Schedule path2 just as we enter the next reporting period.
@@ -363,15 +444,16 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
     RunReporter(path2);
     // Schedule it twice; it should only actually run once.
     RunReporter(path2);
-    TestReporterLaunchCycle(1, path2);
+    TestReporterLaunchCycle({path2});
   }
 
   // Schedule path3 before any more time has passed.
   // The reporter should not launch.
   {
     SCOPED_TRACE("No more time passed");
+    SetDaysSinceLastReport(0);
     RunReporter(path3);
-    TestReporterLaunchCycle(1, path2);
+    TestReporterLaunchCycle({});
   }
 
   // Enter the next reporting period as path3 is still scheduled.
@@ -380,7 +462,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
   {
     SCOPED_TRACE("Previous run still scheduled");
     SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
-    TestReporterLaunchCycle(2, path3);
+    TestReporterLaunchCycle({path3});
   }
 
   // Schedule path3 again in the next reporting period.
@@ -390,7 +472,75 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ParameterChange) {
     SCOPED_TRACE("Run with same parameters");
     SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
     RunReporter(path3);
-    TestReporterLaunchCycle(3, path3);
+    TestReporterLaunchCycle({path3});
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, MultipleLaunches) {
+  exit_code_to_report_ = kSwReporterNothingFound;
+
+  const base::FilePath path1(L"path1");
+  const base::FilePath path2(L"path2");
+  const base::FilePath path3(L"path3");
+
+  SwReporterQueue invocations;
+  invocations.push(SwReporterInvocation::FromFilePath(path1));
+  invocations.push(SwReporterInvocation::FromFilePath(path2));
+
+  {
+    SCOPED_TRACE("Launch 2 times");
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    RunReporterQueue(invocations);
+    TestReporterLaunchCycle({path1, path2});
+  }
+
+  // Schedule a launch with 2 elements, then another with the same 2. It should
+  // just run 2 times, not 4.
+  {
+    SCOPED_TRACE("Launch 2 times with retry");
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    RunReporterQueue(invocations);
+    RunReporterQueue(invocations);
+    TestReporterLaunchCycle({path1, path2});
+  }
+
+  // Schedule a launch with 2 elements, then add a third while the queue is
+  // running.
+  {
+    SCOPED_TRACE("Add third launch while running");
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    RunReporterQueue(invocations);
+
+    // Only test the cycle once, to process the first element in queue.
+    TestPartialLaunchCycle({path1});
+
+    invocations.push(SwReporterInvocation::FromFilePath(path3));
+    RunReporterQueue(invocations);
+
+    // There is still a 2nd element on the queue - that should execute, and
+    // nothing more.
+    TestReporterLaunchCycle({path2});
+
+    // Time passes... Now the 3-element queue should run.
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    TestReporterLaunchCycle({path1, path2, path3});
+  }
+
+  // Second launch should not occur after a failure.
+  {
+    SCOPED_TRACE("Launch multiple times with failure");
+    exit_code_to_report_ = kReporterFailureExitCode;
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    RunReporterQueue(invocations);
+    TestReporterLaunchCycle({path1});
+
+    // If we try again before the reporting period is up, it should not do
+    // anything.
+    TestReporterLaunchCycle({});
+
+    // After enough time has passed, should try the queue again.
+    SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+    TestReporterLaunchCycle({path1});
   }
 }
 
@@ -400,7 +550,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_FeatureDisabled) {
   scoped_feature_list.InitAndDisableFeature(
       kSwReporterExtendedSafeBrowsingFeature);
   RunReporter();
-  TestReporterLaunchCycle(1, base::FilePath());
+  TestReporterLaunchCycle({base::FilePath()});
   ExpectLoggingSwitches({/*expect no switches*/});
   ExpectLastTimeSentReportNotSet();
 }
@@ -411,7 +561,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_NoSBExtendedReporting) {
   scoped_feature_list.InitAndEnableFeature(
       kSwReporterExtendedSafeBrowsingFeature);
   RunReporter();
-  TestReporterLaunchCycle(1, base::FilePath());
+  TestReporterLaunchCycle({base::FilePath()});
   ExpectLoggingSwitches({/*expect no switches*/});
   ExpectLastTimeSentReportNotSet();
 }
@@ -426,7 +576,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledFirstRun) {
   // SBER is enabled and there is no record in the local state of the last time
   // logs have been sent, so we should send logs in this run.
   RunReporter();
-  TestReporterLaunchCycle(1, base::FilePath());
+  TestReporterLaunchCycle({base::FilePath()});
   ExpectLoggingSwitches(std::set<std::string>(std::begin(kExpectedSwitches),
                                               std::end(kExpectedSwitches)));
   ExpectLastReportSentInTheLastHour();
@@ -442,7 +592,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledNoRecentLogging) {
   EnableSBExtendedReporting();
   SetLastTimeSentReport(kDaysBetweenReporterLogsSent + 3);
   RunReporter();
-  TestReporterLaunchCycle(1, base::FilePath());
+  TestReporterLaunchCycle({base::FilePath()});
   ExpectLoggingSwitches(std::set<std::string>(std::begin(kExpectedSwitches),
                                               std::end(kExpectedSwitches)));
   ExpectLastReportSentInTheLastHour();
@@ -460,7 +610,7 @@ IN_PROC_BROWSER_TEST_F(SRTFetcherTest, ReporterLogging_EnabledRecentlyLogged) {
   SetLastTimeSentReport(kDaysBetweenReporterLogsSent - 1);
   int64_t last_time_sent_logs = GetLastTimeSentReport();
   RunReporter();
-  TestReporterLaunchCycle(1, base::FilePath());
+  TestReporterLaunchCycle({base::FilePath()});
   ExpectLoggingSwitches(std::set<std::string>{/*expect no switches*/});
   EXPECT_EQ(last_time_sent_logs, GetLastTimeSentReport());
 }