Add support for the ExperimentalSwReporterEngine field trial.

chrome/browser/safe_browsing/srt_fetcher_browsertest_win.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/srt_fetcher_win.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 20 matching lines @@
     task_runner_ = new base::TestSimpleTaskRunner;
 
     SetSwReporterTestingDelegate(this);
   }
 
   void TearDownInProcessBrowserTestFixture() override {
     SetSwReporterTestingDelegate(nullptr);
   }
 
   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 {
     prompt_trigger_called_ = true;
   }
 
   int LaunchReporter(const SwReporterInvocation& invocation) override {
     ++reporter_launch_count_;
     reporter_launch_parameters_ = invocation;
     return exit_code_to_report_;
@@ skipping 11 matching lines @@
   }
 
   void ExpectToRunAgain(int days) {
     ASSERT_TRUE(task_runner_->HasPendingTask());
     EXPECT_LE(task_runner_->NextPendingTaskDelay(),
               base::TimeDelta::FromDays(days));
     EXPECT_GT(task_runner_->NextPendingTaskDelay(),
               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
     // 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
@@ skipping 14 matching lines @@
     // it in a loop to make sure we're past all pending TryToRun calls before
     // LaunchAndWaitForExit will be called.
     //
     // Once a call to LaunchAndWaitForExit has been posted, TryToRun won't be
     // called again until we pump the UI message loop in order to run
     // ReporterDone.
 
     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_GE(max_expected_launch_count, reporter_launch_count_);
+      ASSERT_FALSE(launch_ready_notified_);
+      ASSERT_FALSE(reporter_done_notified_);
 
-    // Reset launch_ready_notified_ so that we can tell if TryToRun gets called
-    // again unexpectedly.
-    launch_ready_notified_ = false;
+      // 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());
 
-    // 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_);
+      // 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 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());
+      // 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());
 
-    // 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();
+      // 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);
 
-    // At this point another call to TryToRun should be scheduled, whether or
-    // not LaunchAndWaitForExit was called.
-    ASSERT_TRUE(task_runner_->HasPendingTask());
+      reporter_done_notified_ = false;
+      current_launch_count = reporter_launch_count_;
+    }
 
-    // Make sure the flags are false for the next launch cycle test.
-    ASSERT_FALSE(launch_ready_notified_);
-    reporter_done_notified_ = false;
+    // 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();
+
+      // 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_;
   bool prompt_trigger_called_ = false;
   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;
 };
 
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(SRTFetcherTest, NothingFound) {
   exit_code_to_report_ = kSwReporterNothingFound;
   RunReporter();
   task_runner_->RunPendingTasks();
   EXPECT_EQ(1, reporter_launch_count_);
@@ skipping 88 matching lines @@
   const base::FilePath path2(L"path2");
   const base::FilePath path3(L"path3");
 
   // Schedule path1 with a day left in the reporting period.
   // The reporter should not launch.
   constexpr int kDaysLeft = 1;
   {
     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.
   // Now the reporter should launch, just once, using path2.
   {
     SCOPED_TRACE("Reporter runs now");
     SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
     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.
   // Now the reporter should launch again using path3. (Tests that the
   // parameters from the first launch aren't reused.)
   {
     SCOPED_TRACE("Previous run still scheduled");
     SetDaysSinceLastReport(kDaysBetweenSuccessfulSwReporterRuns);
-    TestReporterLaunchCycle(2, path3);
+    TestReporterLaunchCycle({path3});
   }
 
   // Schedule path3 again in the next reporting period.
   // The reporter should launch again using path3, since enough time has
   // passed, even though the parameters haven't changed.
   {
     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