[test] Provide RunLoop::RunUntilConditionForTest() helper.

Some tests need to run the message loop until a particular condition
becomes true, but have no direct signal from the code under test to
use to trigger the check & terminate the run.

RunUntilConditionForTest() runs the loop and repeatedly calls a supplied
callback until it returns true, or the Run()-timeout expires. The helper
returns true if the condition was met before the timeout.

Bug: 1021777, 976740
Change-Id: I18f887431a68cb73cc77a445f3b915284b111cb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033162
Commit-Queue: Wez <wez@chromium.org>
Auto-Submit: Wez <wez@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738222}

base/run_loop.cc

@@ -11,6 +11,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/timer/timer.h"
 #include "build/build_config.h"
 
 namespace base {
@@ -302,6 +303,25 @@ RunLoop::ScopedDisallowRunningForTesting::~ScopedDisallowRunningForTesting() =
     default;
 #endif  // DCHECK_IS_ON()
 
+void RunLoop::RunUntilConditionForTest(RepeatingCallback<bool()> condition) {
+  CHECK(ScopedRunTimeoutForTest::Current());
+  OnceClosure on_failure = ScopedRunTimeoutForTest::Current()->on_timeout();
+  ScopedRunTimeoutForTest run_timeout(
+      ScopedRunTimeoutForTest::Current()->timeout(), DoNothing());
+  auto check_condition = BindRepeating(
+      [](const RepeatingCallback<bool()>& condition, RunLoop* loop) {
+        if (condition.Run())
+          loop->Quit();
+      },
+      condition, this);
+  RepeatingTimer poll_condition;
+  poll_condition.Start(FROM_HERE, TimeDelta::FromMilliseconds(100),
+                       check_condition);
+  Run();
+  if (!condition.Run())
+    std::move(on_failure).Run();
+}
+
 bool RunLoop::BeforeRun() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 

base/run_loop.h

@@ -10,6 +10,7 @@
 
 #include "base/base_export.h"
 #include "base/callback.h"
+#include "base/compiler_specific.h"
 #include "base/containers/stack.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -326,6 +327,14 @@ class BASE_EXPORT RunLoop {
     DISALLOW_COPY_AND_ASSIGN(ScopedDisallowRunningForTesting);
   };
 
+  // Runs the loop until |condition| returns true, or the Run() timeout expires.
+  // Callers should assume that the |condition| will be polled multiple times
+  // per second. The call executes the configured on-timeout handler only if
+  // |condition| is false when the loop terminates. This API should be used only
+  // where providing an explicit signal when the desired condition becomes true,
+  // is not feasible. Use Run() and QuitClosure() where possible.
+  void RunUntilConditionForTest(RepeatingCallback<bool()> condition);
+
  private:
   FRIEND_TEST_ALL_PREFIXES(MessageLoopTypedTest, RunLoopQuitOrderAfter);
 

base/run_loop_unittest.cc

@@ -26,8 +26,10 @@
 #include "base/threading/thread.h"
 #include "base/threading/thread_checker_impl.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "build/build_config.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest-spi.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
@@ -548,47 +550,74 @@ INSTANTIATE_TEST_SUITE_P(Mock,
                          RunLoopTest,
                          testing::Values(RunLoopTestType::kTestDelegate));
 
-TEST(ScopedRunTimeoutForTestTest, TimesOut) {
-  test::TaskEnvironment task_environment;
-  RunLoop run_loop;
+TEST(RunLoopUntilConditionTest, FailsTestOnTimeout) {
+  test::SingleThreadTaskEnvironment task_environment;
 
-  static constexpr auto kArbitraryTimeout =
-      base::TimeDelta::FromMilliseconds(10);
-  RunLoop::ScopedRunTimeoutForTest run_timeout(
-      kArbitraryTimeout, MakeExpectedRunAtLeastOnceClosure(FROM_HERE));
+  // Expect the Run() timeout to be run when |condition| is false and the loop
+  // times out.
+  const RunLoop::ScopedRunTimeoutForTest short_timeout(
+      TimeDelta::FromMilliseconds(10),
+      MakeExpectedRunAtLeastOnceClosure(FROM_HERE));
+  RunLoop().RunUntilConditionForTest(BindRepeating([]() { return false; }));
+}
 
-  // Since the delayed task will be posted only after the message pump starts
-  // running, the ScopedRunTimeoutForTest will already have started to elapse,
-  // so if Run() exits at the correct time then our delayed task will not run.
-  SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(base::IgnoreResult(&SequencedTaskRunner::PostDelayedTask),
-                     SequencedTaskRunnerHandle::Get(), FROM_HERE,
-                     MakeExpectedNotRunClosure(FROM_HERE), kArbitraryTimeout));
+TEST(RunLoopUntilConditionTest, FailsTestIfConditionNotMetOnQuit) {
+  test::SingleThreadTaskEnvironment task_environment;
+  RunLoop loop;
+
+  // Expect the Run() timeout to be run when |condition| is false and the loop
+  // Quit()s prematurely.
+  const RunLoop::ScopedRunTimeoutForTest short_timeout(
+      TimeDelta::FromMilliseconds(10),
+      MakeExpectedRunAtLeastOnceClosure(FROM_HERE));
 
-  // This task should get to run before Run() times-out.
-  SequencedTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, MakeExpectedRunClosure(FROM_HERE), kArbitraryTimeout);
+  // Running with a never-true condition will fire the on-timeout callback.
+  RunLoop().RunUntilConditionForTest(BindRepeating([]() { return false; }));
+}
+
+TEST(RunLoopUntilConditionTest, NoEffectIfConditionMetOnQuit) {
+  test::SingleThreadTaskEnvironment task_environment;
+  RunLoop loop;
 
-  run_loop.Run();
+  // Verify that the call does not trigger the Run() timeout.
+  const RunLoop::ScopedRunTimeoutForTest short_timeout(
+      TimeDelta::FromMilliseconds(10), MakeExpectedNotRunClosure(FROM_HERE));
+  SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE, loop.QuitClosure());
+  RunLoop().RunUntilConditionForTest(BindRepeating([]() { return true; }));
 }
 
-TEST(ScopedRunTimeoutForTestTest, RunTasksUntilTimeout) {
-  test::TaskEnvironment task_environment;
-  RunLoop run_loop;
+TEST(RunLoopUntilConditionTest, NoEffectIfConditionMetOnTimeout) {
+  test::SingleThreadTaskEnvironment task_environment;
+  RunLoop loop;
 
-  static constexpr auto kArbitraryTimeout =
-      base::TimeDelta::FromMilliseconds(10);
-  RunLoop::ScopedRunTimeoutForTest run_timeout(
-      kArbitraryTimeout, MakeExpectedRunAtLeastOnceClosure(FROM_HERE));
+  // Verify that the call does not trigger the Run() timeout.
+  // Note that |short_timeout| must be shorter than the RunUntilConditionForTest
+  // polling frequency.
+  const RunLoop::ScopedRunTimeoutForTest short_timeout(
+      TimeDelta::FromMilliseconds(10), MakeExpectedNotRunClosure(FROM_HERE));
+  RunLoop().RunUntilConditionForTest(BindRepeating([]() { return true; }));
+}
 
-  // Posting a task with the same delay as our timeout, immediately before
-  // calling Run(), means it should get to run. Since this uses QuitWhenIdle(),
-  // the Run() timeout callback should also get to run.
-  SequencedTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, MakeExpectedRunClosure(FROM_HERE), kArbitraryTimeout);
+TEST(RunLoopUntilConditionTest, QuitsLoopIfConditionMetOnPoll) {
+  test::SingleThreadTaskEnvironment task_environment;
+  RunLoop loop;
+
+  // Configure a long timeout so it won't fire before we poll.
+  const RunLoop::ScopedRunTimeoutForTest long_timeout(
+      TestTimeouts::action_timeout(), MakeExpectedNotRunClosure(FROM_HERE));
+
+  // Arrange to post a task to the loop after the Run()-timeout has been
+  // started, set to run after the |condition| is polled and before the Run()
+  // timeout expires. This task should not get to run.
+  ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, BindOnce([]() {
+        ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+            FROM_HERE, MakeExpectedNotRunClosure(FROM_HERE),
+            TimeDelta::FromSeconds(1));
+      }));
 
-  run_loop.Run();
+  // Run the loop, which should be Quit() the first time |condition| is polled.
+  RunLoop().RunUntilConditionForTest(BindRepeating([]() { return true; }));
 }
 
 TEST(RunLoopDeathTest, MustRegisterBeforeInstantiating) {

base/util/memory_pressure/system_memory_pressure_evaluator_chromeos_unittest.cc

@@ -15,10 +15,12 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/system/sys_info.h"
 #include "base/task/post_task.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/scoped_blocking_call.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace util {
@@ -59,12 +61,6 @@ void OnMemoryPressure(
   history->push_back(level);
 }
 
-void RunLoopRunWithTimeout(base::TimeDelta timeout) {
-  base::RunLoop run_loop;
-  base::RunLoop::ScopedRunTimeoutForTest run_timeout(timeout,
-                                                     run_loop.QuitClosure());
-  run_loop.Run();
-}
 }  // namespace
 
 class TestSystemMemoryPressureEvaluator : public SystemMemoryPressureEvaluator {
@@ -196,39 +192,49 @@ TEST(ChromeOSSystemMemoryPressureEvaluatorTest, CheckMemoryPressure) {
   // Moderate Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "900"));
   TriggerKernelNotification(write_end.get());
-  // TODO(bgeffon): Use RunLoop::QuitClosure() instead of relying on "spin for
-  // 1 second".
-  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
-  ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
-            evaluator->current_vote());
+  // TODO(bgeffon): Use RunLoop::QuitClosure() instead of relying on "run until
+  // condition is true".
+  base::RunLoop().RunUntilConditionForTest(
+      base::BindLambdaForTesting([&evaluator]() {
+        return base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE ==
+               evaluator->current_vote();
+      }));
 
   // Critical Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "450"));
   TriggerKernelNotification(write_end.get());
-  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
-  ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL,
-            evaluator->current_vote());
+  base::RunLoop().RunUntilConditionForTest(
+      base::BindLambdaForTesting([&evaluator]() {
+        return base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL ==
+               evaluator->current_vote();
+      }));
 
   // Moderate Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "550"));
   TriggerKernelNotification(write_end.get());
-  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
-  ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
-            evaluator->current_vote());
+  base::RunLoop().RunUntilConditionForTest(
+      base::BindLambdaForTesting([&evaluator]() {
+        return base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE ==
+               evaluator->current_vote();
+      }));
 
   // No pressure, note: this will not cause any event.
   ASSERT_TRUE(SetFileContents(available_file, "1150"));
   TriggerKernelNotification(write_end.get());
-  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
-  ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE,
-            evaluator->current_vote());
+  base::RunLoop().RunUntilConditionForTest(
+      base::BindLambdaForTesting([&evaluator]() {
+        return base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE ==
+               evaluator->current_vote();
+      }));
 
   // Back into moderate.
   ASSERT_TRUE(SetFileContents(available_file, "950"));
   TriggerKernelNotification(write_end.get());
-  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
-  ASSERT_EQ(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
-            evaluator->current_vote());
+  base::RunLoop().RunUntilConditionForTest(
+      base::BindLambdaForTesting([&evaluator]() {
+        return base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE ==
+               evaluator->current_vote();
+      }));
 
   // Now our events should be MODERATE, CRITICAL, MODERATE.
   ASSERT_EQ(4u, pressure_events.size());