Revert "[test] Clean up some incorrect use of ScopedRunTimeout in //chrome."

This reverts commit 77cffbd1.

Reason for revert: This causes compile error in linux-chromeos-chrome.

Original change's description:
> [test] Clean up some incorrect use of ScopedRunTimeout in //chrome.
> 
> ScopedRunTimeout is intended to be used to fail tests if a loop is Run()
> for too long, rather than to Run() the loop for a specific length of
> time.
> 
> Clean up some tests which were using ScopedRunTimeout inappropriately.
> 
> Bug: 1021777
> Change-Id: If6b2ee5f3eab498698d09fdee103e759fff51ead
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033445
> Auto-Submit: Wez <wez@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#738180}

TBR=xiyuan@chromium.org,wez@chromium.org,gab@chromium.org,eroman@chromium.org,mmenke@chromium.org

Change-Id: I919d3ce3bdf4430b05664849f8f57b31a21cc3df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1021777
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037315Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738194}

base/util/memory_pressure/system_memory_pressure_evaluator_chromeos_unittest.cc

@@ -19,7 +19,6 @@
 #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 {
@@ -62,8 +61,8 @@ void OnMemoryPressure(
 
 void RunLoopRunWithTimeout(base::TimeDelta timeout) {
   base::RunLoop run_loop;
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, run_loop.QuitClosure(), timeout);
+  base::RunLoop::ScopedRunTimeoutForTest run_timeout(timeout,
+                                                     run_loop.QuitClosure());
   run_loop.Run();
 }
 }  // namespace

chrome/browser/chromeos/login/oobe_localization_browsertest.cc

@@ -44,6 +44,9 @@ namespace chromeos {
 
 namespace {
 
+// Timeout for RunLoop::Run() in this test.
+const int kTimeoutSeconds = 2;
+
 // OOBE constants.
 const char kLanguageSelect[] = "languageSelect";
 const char kKeyboardSelect[] = "keyboardSelect";
@@ -53,12 +56,52 @@ std::string GetGetSelectStatement(const std::string& selectId) {
 }
 
 const char kUSLayout[] = "xkb:us::eng";
+class TimedRunLoop {
+ public:
+  TimedRunLoop(const base::TimeDelta& timeout,
+               const std::string& failure_message)
+      : timeout_(timeout), message_(failure_message) {}
+
+  // Returns true if Run() successfully finished,
+  // Returns false on timeout.
+  bool Run() {
+    base::OneShotTimer timer;
+    timer.Start(FROM_HERE, timeout_,
+                base::Bind(&TimedRunLoop::OnTimeout, base::Unretained(this)));
+    loop_.Run();
+    return result_;
+  }
+
+  void Quit() {
+    result_ = true;
+    loop_.Quit();
+  }
+
+  base::Closure QuitClosure() {
+    return base::Bind(&TimedRunLoop::Quit, base::Unretained(this));
+  }
+
+ private:
+  void OnTimeout() {
+    LOG(ERROR) << "Timeout waiting for: " << message_;
+    result_ = false;
+    loop_.Quit();
+  }
+
+  bool result_ = false;
+  const base::TimeDelta timeout_;
+  const std::string message_;
+  base::RunLoop loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(TimedRunLoop);
+};
 
 class LanguageListWaiter : public WelcomeScreen::Observer {
  public:
   LanguageListWaiter()
       : welcome_screen_(WelcomeScreen::Get(
-            WizardController::default_controller()->screen_manager())) {
+            WizardController::default_controller()->screen_manager())),
+        loop_(base::TimeDelta::FromSeconds(kTimeoutSeconds), "LanguageList") {
     welcome_screen_->AddObserver(this);
     CheckLanguageList();
   }
@@ -68,8 +111,13 @@ class LanguageListWaiter : public WelcomeScreen::Observer {
   // WelcomeScreen::Observer implementation:
   void OnLanguageListReloaded() override { CheckLanguageList(); }
 
-  // Run the loop until the list is ready or the default Run() timeout expires.
-  void RunUntilLanguageListReady() { loop_.Run(); }
+  // Returns true on success, false on timeout.
+  bool Wait() {
+    if (LanguageListReady())
+      return true;
+
+    return loop_.Run();
+  }
 
  private:
   bool LanguageListReady() const { return welcome_screen_->language_list(); }
@@ -80,7 +128,7 @@ class LanguageListWaiter : public WelcomeScreen::Observer {
   }
 
   WelcomeScreen* welcome_screen_;
-  base::RunLoop loop_;
+  TimedRunLoop loop_;
 };
 
 }  // namespace
@@ -207,17 +255,23 @@ class OobeLocalizationTest
   // Runs the test for the given locale and keyboard layout.
   void RunLocalizationTest();
 
-  // Runs until the OOBE UI JS is ready, or the default Run() timeout expires.
-  void RunUntilJSIsReady() {
+  // Returns true on success, false on error.
+  bool WaitUntilJSIsReady() {
     LoginDisplayHost* host = LoginDisplayHost::default_host();
-    ASSERT_TRUE(host);
+    if (!host)
+      return false;
 
     OobeUI* oobe_ui = host->GetOobeUI();
-    ASSERT_TRUE(oobe_ui);
+    if (!oobe_ui)
+      return false;
+
+    TimedRunLoop run_loop(base::TimeDelta::FromSeconds(kTimeoutSeconds),
+                          "WaitUntilJSIsReady()");
+    const bool oobe_ui_ready = oobe_ui->IsJSReady(run_loop.QuitClosure());
+    if (oobe_ui_ready)
+      return true;
 
-    base::RunLoop run_loop;
-    if (!oobe_ui->IsJSReady(run_loop.QuitClosure()))
-      run_loop.Run();
+    return run_loop.Run();
   }
 
  private:
@@ -348,9 +402,9 @@ void OobeLocalizationTest::RunLocalizationTest() {
   const std::string expected_keyboard_select =
       TranslateXKB2Extension(expected_keyboard_select_control);
 
-  ASSERT_NO_FATAL_FAILURE(LanguageListWaiter().RunUntilLanguageListReady());
+  ASSERT_TRUE(LanguageListWaiter().Wait());
 
-  ASSERT_NO_FATAL_FAILURE(RunUntilJSIsReady());
+  ASSERT_TRUE(WaitUntilJSIsReady());
 
   const std::string first_language =
       expected_locale.substr(0, expected_locale.find(','));

chrome/browser/net/network_context_configuration_browsertest.cc

@@ -22,7 +22,6 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
-#include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_content_browser_client.h"
@@ -2011,11 +2010,12 @@ class NetworkContextConfigurationProxySettingsBrowserTest
     expected_connections_run_loop.Run();
 
     // Then wait for any remaining connections that we should NOT get.
-    base::RunLoop ugly_100ms_wait;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, ugly_100ms_wait.QuitClosure(),
-        base::TimeDelta::FromMilliseconds(100));
-    ugly_100ms_wait.Run();
+    base::RunLoop unexpected_connections_run_loop;
+    base::RunLoop::ScopedRunTimeoutForTest run_timeout(
+        base::TimeDelta::FromMilliseconds(100),
+        base::BindLambdaForTesting(
+            [&]() { unexpected_connections_run_loop.Quit(); }));
+    unexpected_connections_run_loop.Run();
 
     // Stop the server.
     ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());

chrome/browser/ui/ash/assistant/assistant_test_mixin.cc

@@ -11,7 +11,6 @@
 #include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/cpp/test/assistant_test_api.h"
 #include "ash/public/mojom/assistant_state_controller.mojom-shared.h"
-#include "base/auto_reset.h"
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/time/time.h"
@@ -46,6 +45,23 @@ bool Equals(const char* left, const char* right) {
   return strcmp(left, right) == 0;
 }
 
+// Run the loop until the timeout expires, or until it is quit through other
+// mechanisms.
+// Returns whether the loop finished successfully, i.e. |false| if the timeout
+// expired.
+bool RunWithTimeout(base::RunLoop* run_loop, base::TimeDelta timeout) {
+  bool success = true;
+
+  base::RunLoop::ScopedRunTimeoutForTest scoped_timeout(
+      timeout, base::BindLambdaForTesting([&success, run_loop] {
+        success = false;
+        run_loop->QuitClosure();
+      }));
+  run_loop->Run();
+
+  return success;
+}
+
 // Waiter that blocks in the |Wait| method until a given |mojom::AssistantState|
 // is reached, or until a timeout is hit.
 // On timeout this will abort the test with a useful error message.
@@ -59,30 +75,31 @@ class AssistantStatusWaiter : private ash::AssistantStateObserver {
 
   ~AssistantStatusWaiter() override { state_->RemoveObserver(this); }
 
-  void RunUntilExpectedStatus() {
+  void Wait(base::TimeDelta wait_timeout) {
     if (state_->assistant_state() == expected_status_)
       return;
 
     // Wait until we're ready or we hit the timeout.
-    base::RunLoop run_loop;
-    base::AutoReset<base::OnceClosure> quit_loop(&quit_loop_,
-                                                 run_loop.QuitClosure());
-    EXPECT_NO_FATAL_FAILURE(run_loop.Run())
-        << "Failed waiting for AssistantStatus |" << expected_status_ << "|. "
+    run_loop_ = std::make_unique<base::RunLoop>();
+    bool success = RunWithTimeout(run_loop_.get(), wait_timeout);
+    run_loop_.reset();
+
+    EXPECT_TRUE(success)
+        << "Timeout waiting for AssistantStatus |" << expected_status_ << "|. "
         << "Current status is |" << state_->assistant_state() << "|. "
         << "One possible cause is that you're using an expired access token.";
   }
 
  private:
   void OnAssistantStatusChanged(ash::mojom::AssistantState status) override {
-    if (status == expected_status_ && quit_loop_)
-      std::move(quit_loop_).Run();
+    if (status == expected_status_ && run_loop_ != nullptr)
+      run_loop_->Quit();
   }
 
   ash::AssistantState* const state_;
   ash::mojom::AssistantState const expected_status_;
 
-  base::OnceClosure quit_loop_;
+  std::unique_ptr<base::RunLoop> run_loop_;
 };
 
 // Base class that observes all new responses being displayed under the
@@ -103,18 +120,19 @@ class ResponseWaiter : private views::ViewObserver {
       parent_view_->RemoveObserver(this);
   }
 
-  void RunUntilResponseReceived() {
+  void Wait(base::TimeDelta wait_timeout) {
     if (HasExpectedResponse())
       return;
 
     // Wait until we're ready or we hit the timeout.
-    base::RunLoop run_loop;
-    base::AutoReset<base::OnceClosure> quit_loop(&quit_loop_,
-                                                 run_loop.QuitClosure());
-    EXPECT_NO_FATAL_FAILURE(run_loop.Run())
-        << "Failed waiting for Assistant response.\n"
-        << "Expected any of " << FormatExpectedResponses() << ".\n"
-        << "Got \"" << GetResponseText() << "\"";
+    run_loop_ = std::make_unique<base::RunLoop>();
+    bool success = RunWithTimeout(run_loop_.get(), wait_timeout);
+    run_loop_.reset();
+
+    EXPECT_TRUE(success) << "Timeout waiting for Assistant response.\n"
+                         << "Expected any of " << FormatExpectedResponses()
+                         << ".\n"
+                         << "Got \"" << GetResponseText() << "\"";
   }
 
  private:
@@ -122,19 +140,19 @@ class ResponseWaiter : private views::ViewObserver {
   void OnViewHierarchyChanged(
       views::View* observed_view,
       const views::ViewHierarchyChangedDetails& details) override {
-    if (quit_loop_ && HasExpectedResponse())
-      std::move(quit_loop_).Run();
+    if (run_loop_ && HasExpectedResponse())
+      run_loop_->Quit();
   }
 
   void OnViewIsDeleting(views::View* observed_view) override {
     DCHECK(observed_view == parent_view_);
 
-    if (quit_loop_) {
+    if (run_loop_) {
       FAIL() << parent_view_->GetClassName() << " is deleted "
              << "before receiving the Assistant response.\n"
              << "Expected any of " << FormatExpectedResponses() << ".\n"
              << "Got \"" << GetResponseText() << "\"";
-      std::move(quit_loop_).Run();
+      run_loop_->Quit();
     }
 
     parent_view_ = nullptr;
@@ -180,7 +198,7 @@ class ResponseWaiter : private views::ViewObserver {
   views::View* parent_view_;
   std::vector<std::string> expected_responses_;
 
-  base::OnceClosure quit_loop_;
+  std::unique_ptr<base::RunLoop> run_loop_;
 };
 
 class TextResponseWaiter : public ResponseWaiter {
@@ -321,8 +339,6 @@ void AssistantTestMixin::TearDownOnMainThread() {
 
 void AssistantTestMixin::StartAssistantAndWaitForReady(
     base::TimeDelta wait_timeout) {
-  const base::RunLoop::ScopedRunTimeoutForTest run_timeout(
-      wait_timeout, base::MakeExpectedNotRunClosure(FROM_HERE));
   // Note: You might be tempted to call this function from SetUpOnMainThread(),
   // but that will not work as the Assistant service can not start until
   // |BrowserTestBase| calls InitializeNetworkProcess(), which it only does
@@ -333,7 +349,7 @@ void AssistantTestMixin::StartAssistantAndWaitForReady(
 
   AssistantStatusWaiter waiter(test_api_->GetAssistantState(),
                                ash::mojom::AssistantState::NEW_READY);
-  waiter.RunUntilExpectedStatus();
+  waiter.Wait(wait_timeout);
 
   // With the warmer welcome enabled the Assistant service will start an
   // interaction that will never complete (as our tests finish too soon).
@@ -354,11 +370,9 @@ void AssistantTestMixin::SendTextQuery(const std::string& query) {
 void AssistantTestMixin::ExpectCardResponse(
     const std::string& expected_response,
     base::TimeDelta wait_timeout) {
-  const base::RunLoop::ScopedRunTimeoutForTest run_timeout(
-      wait_timeout, base::MakeExpectedNotRunClosure(FROM_HERE));
   CardResponseWaiter waiter(test_api_->ui_element_container(),
                             {expected_response});
-  waiter.RunUntilResponseReceived();
+  waiter.Wait(wait_timeout);
 }
 
 void AssistantTestMixin::ExpectTextResponse(
@@ -370,11 +384,9 @@ void AssistantTestMixin::ExpectTextResponse(
 void AssistantTestMixin::ExpectAnyOfTheseTextResponses(
     const std::vector<std::string>& expected_responses,
     base::TimeDelta wait_timeout) {
-  const base::RunLoop::ScopedRunTimeoutForTest run_timeout(
-      wait_timeout, base::MakeExpectedNotRunClosure(FROM_HERE));
   TextResponseWaiter waiter(test_api_->ui_element_container(),
                             expected_responses);
-  waiter.RunUntilResponseReceived();
+  waiter.Wait(wait_timeout);
 }
 
 void AssistantTestMixin::PressAssistantKey() {