Remove RunLoop::RunWithTimeout from public API

This API is not being used much and one of the tests is flaky. Rather
than fix the test it has been decided to remove the API. The internal
working is left untouched so it would be easy to add it back should the
need arise.

Bug: 970187
Change-Id: I036d5843cd41795a84ca7f0c2be2a31acc4e9b7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1692981
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Doug Turner <dougt@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687312}

base/memory/memory_pressure_monitor_chromeos_unittest.cc

@@ -60,6 +60,11 @@ void OnMemoryPressure(
   history->push_back(level);
 }
 
+void RunLoopRunWithTimeout(TimeDelta timeout) {
+  RunLoop run_loop;
+  RunLoop::ScopedRunTimeoutForTest run_timeout(timeout, run_loop.QuitClosure());
+  run_loop.Run();
+}
 }  // namespace
 
 class TestMemoryPressureMonitor : public MemoryPressureMonitor {
@@ -180,35 +185,37 @@ TEST(ChromeOSMemoryPressureMonitorTest, CheckMemoryPressure) {
   // Moderate Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "900"));
   TriggerKernelNotification(write_end.get());
-  RunLoop().RunWithTimeout(base::TimeDelta::FromSeconds(1));
+  // TODO(bgeffon): Use RunLoop::QuitClosure() instead of relying on "spin for
+  // 1 second".
+  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
   ASSERT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
             monitor->GetCurrentPressureLevel());
 
   // Critical Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "450"));
   TriggerKernelNotification(write_end.get());
-  RunLoop().RunWithTimeout(base::TimeDelta::FromSeconds(1));
+  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
   ASSERT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL,
             monitor->GetCurrentPressureLevel());
 
   // Moderate Pressure.
   ASSERT_TRUE(SetFileContents(available_file, "550"));
   TriggerKernelNotification(write_end.get());
-  RunLoop().RunWithTimeout(base::TimeDelta::FromSeconds(1));
+  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
   ASSERT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
             monitor->GetCurrentPressureLevel());
 
   // No pressure, note: this will not cause any event.
   ASSERT_TRUE(SetFileContents(available_file, "1150"));
   TriggerKernelNotification(write_end.get());
-  RunLoop().RunWithTimeout(base::TimeDelta::FromSeconds(1));
+  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
   ASSERT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE,
             monitor->GetCurrentPressureLevel());
 
   // Back into moderate.
   ASSERT_TRUE(SetFileContents(available_file, "950"));
   TriggerKernelNotification(write_end.get());
-  RunLoop().RunWithTimeout(base::TimeDelta::FromSeconds(1));
+  RunLoopRunWithTimeout(base::TimeDelta::FromSeconds(1));
   ASSERT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE,
             monitor->GetCurrentPressureLevel());
 

base/run_loop.cc

@@ -132,10 +132,6 @@ RunLoop::~RunLoop() {
 }
 
 void RunLoop::Run() {
-  RunWithTimeout(TimeDelta::Max());
-}
-
-void RunLoop::RunWithTimeout(TimeDelta timeout) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!BeforeRun())
@@ -157,7 +153,7 @@ void RunLoop::RunWithTimeout(TimeDelta timeout) {
   const bool application_tasks_allowed =
       delegate_->active_run_loops_.size() == 1U ||
       type_ == Type::kNestableTasksAllowed;
-  delegate_->Run(application_tasks_allowed, timeout);
+  delegate_->Run(application_tasks_allowed, TimeDelta::Max());
 
   AfterRun();
 }

base/run_loop.h

@@ -75,12 +75,6 @@ class BASE_EXPORT RunLoop {
   // RunLoop::Delegate asynchronously.
   void Run();
 
-  // Run the current RunLoop::Delegate. This blocks until either |timeout| has
-  // elapsed or Quit is called. Nesting multiple runloops with and without
-  // timeouts is supported. If an inner loop has a longer timeout than the outer
-  // loop, the outer loop will immediately exit when the inner one does.
-  void RunWithTimeout(TimeDelta timeout);
-
   // Run the current RunLoop::Delegate until it doesn't find any tasks or
   // messages in its queue (it goes idle).
   // WARNING #1: This may run long (flakily timeout) and even never return! Do

base/run_loop_unittest.cc

@@ -283,123 +283,6 @@ TEST_P(RunLoopTest, QuitWhenIdleClosure) {
   run_loop_.Run();
 }
 
-TEST_P(RunLoopTest, RunWithTimeout) {
-  // SimpleSingleThreadTaskRunner doesn't support delayed tasks.
-  if (GetParam() == RunLoopTestType::kTestDelegate)
-    return;
-
-  bool task1_run = false;
-  bool task2_run = false;
-  bool task3_run = false;
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task1_run = true; }),
-      TimeDelta::FromMilliseconds(10));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task2_run = true; }),
-      TimeDelta::FromMilliseconds(20));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task3_run = true; }),
-      TimeDelta::FromSeconds(10));
-
-  run_loop_.RunWithTimeout(TimeDelta::FromMilliseconds(20));
-  EXPECT_TRUE(task1_run);
-  EXPECT_TRUE(task2_run);
-  EXPECT_FALSE(task3_run);
-}
-
-// TODO(https://crbug.com/970187): This test is inherently flaky.
-TEST_P(RunLoopTest, DISABLED_NestedRunWithTimeout) {
-  // SimpleSingleThreadTaskRunner doesn't support delayed tasks.
-  if (GetParam() == RunLoopTestType::kTestDelegate)
-    return;
-
-  bool task1_run = false;
-  bool task2_run = false;
-  bool task3_run = false;
-  bool task4_run = false;
-  bool task5_run = false;
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task1_run = true; }),
-      TimeDelta::FromMilliseconds(10));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() {
-        task2_run = true;
-        EXPECT_FALSE(task3_run);
-        RunLoop nested_run_loop(RunLoop::Type::kNestableTasksAllowed);
-        nested_run_loop.RunWithTimeout(TimeDelta::FromMilliseconds(20));
-        EXPECT_TRUE(task3_run);
-        EXPECT_TRUE(task4_run);
-      }),
-      TimeDelta::FromMilliseconds(20));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task3_run = true; }),
-      TimeDelta::FromMilliseconds(30));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task4_run = true; }),
-      TimeDelta::FromMilliseconds(40));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task5_run = true; }),
-      TimeDelta::FromSeconds(10));
-
-  run_loop_.RunWithTimeout(TimeDelta::FromMilliseconds(40));
-  EXPECT_TRUE(task1_run);
-  EXPECT_TRUE(task2_run);
-  EXPECT_TRUE(task3_run);
-  EXPECT_TRUE(task4_run);
-  EXPECT_FALSE(task5_run);
-}
-
-TEST_P(RunLoopTest, NestedRunWithTimeoutWhereInnerLoopHasALongerTimeout) {
-  // SimpleSingleThreadTaskRunner doesn't support delayed tasks.
-  if (GetParam() == RunLoopTestType::kTestDelegate)
-    return;
-
-  bool task1_run = false;
-  bool task2_run = false;
-  bool task3_run = false;
-  bool task4_run = false;
-  bool task5_run = false;
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task1_run = true; }),
-      TimeDelta::FromMilliseconds(10));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() {
-        task2_run = true;
-        EXPECT_FALSE(task3_run);
-        RunLoop nested_run_loop(RunLoop::Type::kNestableTasksAllowed);
-        nested_run_loop.RunWithTimeout(TimeDelta::FromMilliseconds(50));
-        EXPECT_TRUE(task3_run);
-        EXPECT_TRUE(task4_run);
-      }),
-      TimeDelta::FromMilliseconds(20));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task3_run = true; }),
-      TimeDelta::FromMilliseconds(30));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task4_run = true; }),
-      TimeDelta::FromMilliseconds(40));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task5_run = true; }),
-      TimeDelta::FromMilliseconds(50));
-
-  run_loop_.RunWithTimeout(TimeDelta::FromMilliseconds(40));
-  EXPECT_TRUE(task1_run);
-  EXPECT_TRUE(task2_run);
-  EXPECT_TRUE(task3_run);
-  EXPECT_TRUE(task4_run);
-  EXPECT_TRUE(task5_run);
-}
-
 // Verify that the QuitWhenIdleClosure() can run after the RunLoop has been
 // deleted. It should have no effect.
 TEST_P(RunLoopTest, QuitWhenIdleClosureAfterRunLoopScope) {

base/test/test_mock_time_task_runner_unittest.cc

@@ -190,31 +190,6 @@ TEST(TestMockTimeTaskRunnerTest, RunLoopDriveableWhenBound) {
   EXPECT_EQ(expected_value, counter);
 }
 
-TEST(TestMockTimeTaskRunnerTest, RunLoopRunWithTimeout) {
-  auto bound_mock_time_task_runner = MakeRefCounted<TestMockTimeTaskRunner>(
-      TestMockTimeTaskRunner::Type::kBoundToThread);
-  bool task1_ran = false;
-  bool task2_ran = false;
-  bool task3_ran = false;
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task1_ran = true; }),
-      TimeDelta::FromSeconds(3));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task2_ran = true; }),
-      TimeDelta::FromSeconds(33));
-
-  ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, BindLambdaForTesting([&]() { task3_ran = true; }),
-      TimeDelta::FromSeconds(333));
-
-  RunLoop().RunWithTimeout(TimeDelta::FromSeconds(33));
-  EXPECT_TRUE(task1_ran);
-  EXPECT_TRUE(task2_ran);
-  EXPECT_FALSE(task3_ran);
-}
-
 TEST(TestMockTimeTaskRunnerTest, AvoidCaptureWhenBound) {
   // Make sure that capturing the active task runner --- which sometimes happens
   // unknowingly due to ThreadsafeObserverList deep within some singleton ---

chrome/browser/ui/views/accessibility/uia_accessibility_event_waiter.cc

@@ -11,6 +11,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_variant.h"
@@ -38,7 +39,9 @@ void UiaAccessibilityEventWaiter::Wait() {
 
 void UiaAccessibilityEventWaiter::WaitWithTimeout(base::TimeDelta timeout) {
   // Pump messages via |shutdown_loop_| until the thread is complete.
-  shutdown_loop_.RunWithTimeout(timeout);
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, shutdown_loop_.QuitClosure(), timeout);
+  shutdown_loop_.Run();
   base::PlatformThread::Join(thread_handle_);
 }
 

content/browser/renderer_host/render_widget_host_view_mac_browsertest.mm

@@ -48,7 +48,10 @@
 }
 
 - (void)waitWithTimeout:(NSTimeInterval)timeout {
-  run_loop_->RunWithTimeout(base::TimeDelta::FromSecondsD(timeout));
+  base::RunLoop::ScopedRunTimeoutForTest run_timeout(
+      base::TimeDelta::FromSecondsD(timeout), run_loop_->QuitClosure());
+  run_loop_->Run();
+
   [self reset];
 }