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 e77870481184d9455df698b491d7641195865259..01b6ce5f0ced083703e2d592f4266ce08e2df4ec 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc
@@ -38,8 +38,15 @@ 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_);
+    SwReporterQueue invocations;
+    invocations.push(SwReporterInvocation::FromFilePath(exe_path));
+    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 {
@@ -71,8 +78,12 @@ class SRTFetcherTest : public InProcessBrowserTest,
               base::TimeDelta::FromDays(days) - base::TimeDelta::FromHours(1));
   }
 
-  void TestReporterLaunchCycle(int expected_launch_count,
-                               const base::FilePath& expected_launch_path) {
+  void TestReporterLaunchCycle(
+      const std::vector<base::FilePath>& expected_launch_paths,
+      bool test_if_cycle_complete = true) {

[Sorin Jianu, 2016/09/01 15:59:09]

Since the overload is called in only one case, and it seems that the true case
could be handled by a separate function, can we avoid the default parameter?

[Joe Mason, 2016/09/02 02:24:30]

Goot idea. Done.

+    reporter_launch_count_ = 0;
+    reporter_launch_parameters_ = SwReporterInvocation();
+
     // 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
@@ -107,47 +118,87 @@ 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)
+    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_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_;
+    }
+
+    // Now that all expected launches have been tested, run the cycle once more
+    // to make sure no more launches happen.
+    if (test_if_cycle_complete) {
+      ASSERT_TRUE(task_runner_->HasPendingTask());
+      ASSERT_FALSE(reporter_done_notified_);
+      ASSERT_FALSE(launch_ready_notified_);
+
+      // Call the pending TryToRun.
       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;
-
-    // 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_);
-    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(expected_launch_count, 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 another call to TryToRun should be scheduled, whether or
-    // not LaunchAndWaitForExit was called.
-    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;
+      // We expect that this scheduled another TryToRun. If it scheduled
+      // LaunchAndWaitForExit an unexpected launch is about to happen.
+      ASSERT_TRUE(task_runner_->HasPendingTask());
+      ASSERT_FALSE(launch_ready_notified_);
+      ASSERT_FALSE(reporter_done_notified_);
+      ASSERT_EQ(current_launch_count, reporter_launch_count_);
+    }
   }
 
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
@@ -155,7 +206,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;
 };
 
@@ -264,7 +319,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.
@@ -275,15 +330,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.
@@ -292,7 +348,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.
@@ -302,8 +358,82 @@ 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);
+    TestReporterLaunchCycle({path1}, false /*test_if_cycle_complete*/);
+
+    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});
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(SRTFetcherTest, EmptyQueue) {
+  exit_code_to_report_ = kSwReporterNothingFound;
+  SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
+  SwReporterQueue invocations;
+  RunReporterQueue(invocations);
+  TestReporterLaunchCycle({});
 }
 
 }  // namespace safe_browsing