cros: Introduce common concept for conditions that can be awaited it tests.

Bug: 912704
Change-Id: Ie1804ce8a6f05721e28a6d2c703530d8e0c53841
Reviewed-on: https://chromium-review.googlesource.com/c/1481511
Commit-Queue: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635114}

chrome/browser/chromeos/BUILD.gn

@@ -2107,8 +2107,9 @@ static_library("test_support") {
     "login/screens/mock_update_screen.h",
     "login/screens/mock_welcome_screen.cc",
     "login/screens/mock_welcome_screen.h",
-    "login/test/test_condition_waiter.cc",
     "login/test/test_condition_waiter.h",
+    "login/test/test_predicate_waiter.cc",
+    "login/test/test_predicate_waiter.h",
     "scoped_set_running_on_chromeos_for_testing.cc",
     "scoped_set_running_on_chromeos_for_testing.h",
     "settings/scoped_testing_cros_settings.cc",

chrome/browser/chromeos/login/demo_mode/demo_setup_browsertest.cc

@@ -127,13 +127,7 @@ std::string ScreenToContentQuery(OobeScreen screen) {
 
 // Waits for js condition to be fulfilled.
 void WaitForJsCondition(const std::string& js_condition) {
-  return test::TestConditionWaiter(base::BindRepeating(
-                                       [](const std::string& js_condition) {
-                                         return test::OobeJS().GetBool(
-                                             js_condition);
-                                       },
-                                       js_condition))
-      .Wait();
+  test::OobeJS().CreateWaiter(js_condition)->Wait();
 }
 
 }  // namespace

chrome/browser/chromeos/login/kiosk_browsertest.cc

@@ -316,17 +316,6 @@ class ScopedCanConfigureNetwork {
   DISALLOW_COPY_AND_ASSIGN(ScopedCanConfigureNetwork);
 };
 
-// Waits for js condition to be fulfilled.
-void WaitForJsCondition(const std::string& js_condition) {
-  return test::TestConditionWaiter(base::BindRepeating(
-                                       [](const std::string& js_condition) {
-                                         return test::OobeJS().GetBool(
-                                             js_condition);
-                                       },
-                                       js_condition))
-      .Wait();
-}
-
 class KioskFakeDiskMountManager : public file_manager::FakeDiskMountManager {
  public:
   KioskFakeDiskMountManager() {}
@@ -993,8 +982,10 @@ IN_PROC_BROWSER_TEST_F(KioskTest, LaunchInDiagnosticMode) {
   LaunchApp(kTestKioskApp, true);
 
   bool new_kiosk_ui = KioskAppMenuHandler::EnableNewKioskUI();
-  WaitForJsCondition(new_kiosk_ui ? kCheckDiagnosticModeNewAPI
-                                  : kCheckDiagnosticModeOldAPI);
+  test::OobeJS()
+      .CreateWaiter(new_kiosk_ui ? kCheckDiagnosticModeNewAPI
+                                 : kCheckDiagnosticModeOldAPI)
+      ->Wait();
 
   std::string diagnosticMode(new_kiosk_ui ?
       kCheckDiagnosticModeNewAPI : kCheckDiagnosticModeOldAPI);

chrome/browser/chromeos/login/oobe_interactive_ui_test.cc

@@ -15,7 +15,7 @@
 #include "chrome/browser/chromeos/login/screens/update_screen.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
-#include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
+#include "chrome/browser/chromeos/login/test/test_predicate_waiter.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
@@ -28,35 +28,6 @@
 namespace chromeos {
 namespace {
 
-enum class JsWaitOptions {
-  kNone,
-  kSatisfyIfOobeDestroyed,
-};
-
-// If |options| is kSatisfyIfOobeDestroyed, we are waiting for the end
-// condition, so it is automatically fullfilled if LoginDisplayHost is already
-// destroyed.
-void WaitForJsCondition(const std::string& js_condition,
-                        JsWaitOptions options) {
-  if (options == JsWaitOptions::kSatisfyIfOobeDestroyed) {
-    return test::TestConditionWaiter(
-               base::BindRepeating(
-                   [](const std::string& js_condition) {
-                     return !LoginDisplayHost::default_host() ||
-                            test::OobeJS().GetBool(js_condition);
-                   },
-                   js_condition))
-        .Wait();
-  }
-  return test::TestConditionWaiter(base::BindRepeating(
-                                       [](const std::string& js_condition) {
-                                         return test::OobeJS().GetBool(
-                                             js_condition);
-                                       },
-                                       js_condition))
-      .Wait();
-}
-
 class ScopedQuickUnlockPrivateGetAuthTokenFunctionObserver {
  public:
   explicit ScopedQuickUnlockPrivateGetAuthTokenFunctionObserver(
@@ -145,7 +116,7 @@ class OobeInteractiveUITest
 
     LOG(INFO)
         << "OobeInteractiveUITest: Waiting for LoginDisplayHost to shut down.";
-    test::TestConditionWaiter(base::BindRepeating([]() {
+    test::TestPredicateWaiter(base::BindRepeating([]() {
       return !LoginDisplayHost::default_host();
     })).Wait();
     LOG(INFO) << "OobeInteractiveUITest: LoginDisplayHost is down.";
@@ -157,8 +128,9 @@ class OobeInteractiveUITest
         content::NotificationService::AllSources());
     observer.Wait();
 
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'connect'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'connect'")
+        ->Wait();
   }
 
   void RunWelcomeScreenChecks() {
@@ -187,9 +159,10 @@ class OobeInteractiveUITest
   }
 
   void WaitForNetworkSelectionScreen() {
-    WaitForJsCondition(
-        "Oobe.getInstance().currentScreen.id == 'network-selection'",
-        JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter(
+            "Oobe.getInstance().currentScreen.id == 'network-selection'")
+        ->Wait();
     LOG(INFO)
         << "OobeInteractiveUITest: Switched to 'network-selection' screen.";
   }
@@ -207,15 +180,17 @@ class OobeInteractiveUITest
   }
 
   void WaitForEulaScreen() {
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'eula'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'eula'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Switched to 'eula' screen.";
   }
 
   void RunEulaScreenChecks() {
     // Wait for actual EULA to appear.
-    WaitForJsCondition("!$('oobe-eula-md').$.eulaDialog.hidden",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("!$('oobe-eula-md').$.eulaDialog.hidden")
+        ->Wait();
     test::OobeJS().ExpectTrue("!$('oobe-eula-md').$.acceptButton.disabled");
   }
 
@@ -224,9 +199,10 @@ class OobeInteractiveUITest
   }
 
   void WaitForUpdateScreen() {
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'update'",
-                       JsWaitOptions::kNone);
-    WaitForJsCondition("!$('update').hidden", JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'update'")
+        ->Wait();
+    test::OobeJS().CreateWaiter("!$('update').hidden")->Wait();
 
     LOG(INFO) << "OobeInteractiveUITest: Switched to 'update' screen.";
   }
@@ -241,8 +217,9 @@ class OobeInteractiveUITest
   }
 
   void WaitForGaiaSignInScreen() {
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'gaia-signin'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'gaia-signin'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Switched to 'gaia-signin' screen.";
   }
 
@@ -258,8 +235,9 @@ class OobeInteractiveUITest
 
   void WaitForSyncConsentScreen() {
     LOG(INFO) << "OobeInteractiveUITest: Waiting for 'sync-consent' screen.";
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'sync-consent'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'sync-consent'")
+        ->Wait();
   }
 
   void ExitScreenSyncConsent() {
@@ -271,27 +249,29 @@ class OobeInteractiveUITest
     screen->OnStateChanged(nullptr);
     LOG(INFO) << "OobeInteractiveUITest: Waiting for 'sync-consent' screen "
                  "to close.";
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id != 'sync-consent'",
-                       JsWaitOptions::kSatisfyIfOobeDestroyed);
+    test::CreatePredicateOrOobeDestroyedWaiter(
+        "Oobe.getInstance().currentScreen.id != 'sync-consent'")
+        ->Wait();
   }
 
   void WaitForFingerprintScreen() {
     LOG(INFO)
         << "OobeInteractiveUITest: Waiting for 'fingerprint-setup' screen.";
-    WaitForJsCondition(
-        "Oobe.getInstance().currentScreen.id == 'fingerprint-setup'",
-        JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter(
+            "Oobe.getInstance().currentScreen.id == 'fingerprint-setup'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Waiting for fingerprint setup screen "
                  "to show.";
-    WaitForJsCondition("!$('fingerprint-setup').hidden", JsWaitOptions::kNone);
+    test::OobeJS().CreateWaiter("!$('fingerprint-setup').hidden")->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Waiting for fingerprint setup screen "
                  "to initializes.";
-    WaitForJsCondition("!$('fingerprint-setup-impl').hidden",
-                       JsWaitOptions::kNone);
+    test::OobeJS().CreateWaiter("!$('fingerprint-setup-impl').hidden")->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Waiting for fingerprint setup screen "
                  "to show setupFingerprint.";
-    WaitForJsCondition("!$('fingerprint-setup-impl').$.setupFingerprint.hidden",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("!$('fingerprint-setup-impl').$.setupFingerprint.hidden")
+        ->Wait();
   }
 
   void RunFingerprintScreenChecks() {
@@ -305,8 +285,9 @@ class OobeInteractiveUITest
         "$('fingerprint-setup-impl').$.setupFingerprint.hidden");
     LOG(INFO) << "OobeInteractiveUITest: Waiting for fingerprint setup "
                  "to switch to placeFinger.";
-    WaitForJsCondition("!$('fingerprint-setup-impl').$.placeFinger.hidden",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("!$('fingerprint-setup-impl').$.placeFinger.hidden")
+        ->Wait();
   }
 
   void ExitFingerprintPinSetupScreen() {
@@ -319,16 +300,16 @@ class OobeInteractiveUITest
         "$('fingerprint-setup-impl').$.setupFingerprintLater.click()");
     LOG(INFO) << "OobeInteractiveUITest: Waiting for fingerprint setup screen "
                  "to close.";
-    WaitForJsCondition(
-        "Oobe.getInstance().currentScreen.id !="
-        "'fingerprint-setup'",
-        JsWaitOptions::kSatisfyIfOobeDestroyed);
+    test::CreatePredicateOrOobeDestroyedWaiter(
+        "Oobe.getInstance().currentScreen.id != 'fingerprint-setup'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: 'fingerprint-setup' screen done.";
   }
 
   void WaitForDiscoverScreen() {
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'discover'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'discover'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: Switched to 'discover' screen.";
   }
 
@@ -353,14 +334,16 @@ class OobeInteractiveUITest
     test::OobeJS().ExecuteAsync(
         "$('discover-impl').root.querySelector('discover-pin-setup-module')."
         "$.setupSkipButton.click()");
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id != 'discover'",
-                       JsWaitOptions::kSatisfyIfOobeDestroyed);
+    test::CreatePredicateOrOobeDestroyedWaiter(
+        "Oobe.getInstance().currentScreen.id != 'discover'")
+        ->Wait();
     LOG(INFO) << "OobeInteractiveUITest: 'discover' screen done.";
   }
 
   void WaitForUserImageScreen() {
-    WaitForJsCondition("Oobe.getInstance().currentScreen.id == 'user-image'",
-                       JsWaitOptions::kNone);
+    test::OobeJS()
+        .CreateWaiter("Oobe.getInstance().currentScreen.id == 'user-image'")
+        ->Wait();
 
     LOG(INFO) << "OobeInteractiveUITest: Switched to 'user-image' screen.";
   }

chrome/browser/chromeos/login/test/js_checker.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chromeos/login/test/test_predicate_waiter.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -18,6 +19,11 @@ std::string WrapSend(const std::string& expression) {
   return "window.domAutomationController.send(" + expression + ")";
 }
 
+bool CheckConditionIfOobeExists(const std::string& js_condition) {
+  return !chromeos::LoginDisplayHost::default_host() ||
+         chromeos::test::OobeJS().GetBool(js_condition);
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -92,6 +98,13 @@ void JSChecker::ExpectNE(const std::string& expression, bool result) {
   EXPECT_NE(GetBool(expression), result) << expression;
 }
 
+std::unique_ptr<TestConditionWaiter> JSChecker::CreateWaiter(
+    const std::string& js_condition) {
+  TestPredicateWaiter::PredicateCheck predicate = base::BindRepeating(
+      &JSChecker::GetBool, base::Unretained(this), js_condition);
+  return std::make_unique<TestPredicateWaiter>(predicate);
+}
+
 void JSChecker::GetBoolImpl(const std::string& expression, bool* result) {
   CHECK(web_contents_);
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
@@ -125,5 +138,12 @@ void ExecuteOobeJSAsync(const std::string& script) {
       LoginDisplayHost::default_host()->GetOobeWebContents(), script);
 }
 
+std::unique_ptr<TestConditionWaiter> CreatePredicateOrOobeDestroyedWaiter(
+    const std::string& js_condition) {
+  TestPredicateWaiter::PredicateCheck predicate =
+      base::BindRepeating(&CheckConditionIfOobeExists, js_condition);
+  return std::make_unique<TestPredicateWaiter>(predicate);
+}
+
 }  // namespace test
 }  // namespace chromeos

chrome/browser/chromeos/login/test/js_checker.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_TEST_JS_CHECKER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_JS_CHECKER_H_
 
+#include <memory>
 #include <string>
 
 namespace content {
@@ -14,6 +15,8 @@ class WebContents;
 namespace chromeos {
 namespace test {
 
+class TestConditionWaiter;
+
 // Utility class for tests that allows us to evalute and check JavaScript
 // expressions inside given web contents. All calls are made synchronously.
 class JSChecker {
@@ -46,6 +49,11 @@ class JSChecker {
   void ExpectEQ(const std::string& expression, bool result);
   void ExpectNE(const std::string& expression, bool result);
 
+  // Checks test waiter that would await until |js_condition| evaluates
+  // to true.
+  std::unique_ptr<TestConditionWaiter> CreateWaiter(
+      const std::string& js_condition);
+
   void set_web_contents(content::WebContents* web_contents) {
     web_contents_ = web_contents;
   }
@@ -66,6 +74,11 @@ JSChecker OobeJS();
 void ExecuteOobeJS(const std::string& script);
 void ExecuteOobeJSAsync(const std::string& script);
 
+// Helper method to create waiter over js condition that would also be satisfied
+// if oobe UI is destroyed.
+std::unique_ptr<TestConditionWaiter> CreatePredicateOrOobeDestroyedWaiter(
+    const std::string& js_expression);
+
 }  // namespace test
 }  // namespace chromeos
 

chrome/browser/chromeos/login/test/oobe_screen_waiter.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/chromeos/login/oobe_screen.h"
+#include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 
 namespace content {
@@ -16,13 +17,12 @@ class MessageLoopRunner;
 namespace chromeos {
 
 // A waiter that blocks until the expected oobe screen is reached.
-class OobeScreenWaiter : public OobeUI::Observer {
+class OobeScreenWaiter : public OobeUI::Observer,
+                         public test::TestConditionWaiter {
  public:
   explicit OobeScreenWaiter(OobeScreen expected_screen);
   ~OobeScreenWaiter() override;
 
-  // Run message loop to wait for the expected_screen to become current screen.
-  void Wait();
 
   // Run message loop to wait for the expected_screen to be fully initialized.
   void WaitForInitialization();
@@ -37,6 +37,9 @@ class OobeScreenWaiter : public OobeUI::Observer {
                               OobeScreen new_screen) override;
   void OnScreenInitialized(OobeScreen screen) override;
 
+  // TestConditionWaiter;
+  void Wait() override;
+
  private:
   OobeUI* GetOobeUI();
 

chrome/browser/chromeos/login/test/test_condition_waiter.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
+// Copyright 2019 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_TEST_TEST_CONDITION_WAITER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_TEST_CONDITION_WAITER_H_
 
-#include "base/callback_forward.h"
-#include "base/run_loop.h"
-#include "base/timer/timer.h"
+#include "base/macros.h"
 
 namespace chromeos {
 namespace test {
 
-// Waits for condition to be fulfilled.
+// Generic class for conditions that can be awaited it test.
 class TestConditionWaiter {
  public:
-  using ConditionCheck = base::RepeatingCallback<bool(void)>;
+  virtual ~TestConditionWaiter() = default;
+  virtual void Wait() = 0;
 
-  explicit TestConditionWaiter(const ConditionCheck& is_fulfilled);
-  ~TestConditionWaiter();
-
-  void Wait();
-
- private:
-  void CheckCondition();
-
-  const ConditionCheck is_fulfilled_;
-
-  base::RepeatingTimer timer_;
-  base::RunLoop run_loop_;
+ protected:
+  TestConditionWaiter() = default;
 
   DISALLOW_COPY_AND_ASSIGN(TestConditionWaiter);
 };

chrome/browser/chromeos/login/test/test_condition_waiter.cc → chrome/browser/chromeos/login/test/test_predicate_waiter.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
+#include "chrome/browser/chromeos/login/test/test_predicate_waiter.h"
 
 #include "base/callback.h"
 
@@ -10,27 +10,27 @@ namespace chromeos {
 namespace test {
 namespace {
 
-const base::TimeDelta kConditionCheckFrequency =
+const base::TimeDelta kPredicateCheckFrequency =
     base::TimeDelta::FromMilliseconds(200);
 
 }  // anonymous namespace
 
-TestConditionWaiter::TestConditionWaiter(
+TestPredicateWaiter::TestPredicateWaiter(
     const base::RepeatingCallback<bool(void)>& is_fulfilled)
     : is_fulfilled_(is_fulfilled) {}
 
-TestConditionWaiter::~TestConditionWaiter() = default;
+TestPredicateWaiter::~TestPredicateWaiter() = default;
 
-void TestConditionWaiter::Wait() {
+void TestPredicateWaiter::Wait() {
   if (is_fulfilled_.Run())
     return;
 
-  timer_.Start(FROM_HERE, kConditionCheckFrequency, this,
-               &TestConditionWaiter::CheckCondition);
+  timer_.Start(FROM_HERE, kPredicateCheckFrequency, this,
+               &TestPredicateWaiter::CheckPredicate);
   run_loop_.Run();
 }
 
-void TestConditionWaiter::CheckCondition() {
+void TestPredicateWaiter::CheckPredicate() {
   if (is_fulfilled_.Run()) {
     run_loop_.Quit();
     timer_.Stop();

chrome/browser/chromeos/login/test/test_predicate_waiter.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_TEST_TEST_PREDICATE_WAITER_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_TEST_PREDICATE_WAITER_H_
+
+#include "base/callback_forward.h"
+#include "base/run_loop.h"
+#include "base/timer/timer.h"
+#include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
+
+namespace chromeos {
+namespace test {
+
+// Waits for predicate to be fulfilled.
+class TestPredicateWaiter : public TestConditionWaiter {
+ public:
+  using PredicateCheck = base::RepeatingCallback<bool(void)>;
+
+  explicit TestPredicateWaiter(const PredicateCheck& is_fulfilled);
+  ~TestPredicateWaiter() override;
+
+  // TestConditionWaiter
+  void Wait() override;
+
+ private:
+  void CheckPredicate();
+
+  const PredicateCheck is_fulfilled_;
+
+  base::RepeatingTimer timer_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestPredicateWaiter);
+};
+
+}  // namespace test
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_TEST_PREDICATE_WAITER_H_