oobe: Introduce WizardControllerScreenExitWaiter

Most of the times the screen has its implementation as the OOBE screen
(html + javascript). But it could happen that there is no html part. In
that case OobeScreenExitWaiter is useless because screen on the OOBE
part might not change.
This CL create a WizardController screen exit waiter. Which should help
in such cases.

Bug: none
Change-Id: I9e48e892a4e46d2f417d92a27d59b8319b8ce965
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431046Reviewed-by: Yilkal Abe <yilkal@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811685}

chrome/browser/chromeos/BUILD.gn

@@ -3018,6 +3018,8 @@ static_library("test_support") {
     "login/test/test_condition_waiter.h",
     "login/test/test_predicate_waiter.cc",
     "login/test/test_predicate_waiter.h",
+    "login/test/wizard_controller_screen_exit_waiter.cc",
+    "login/test/wizard_controller_screen_exit_waiter.h",
     "login/version_updater/mock_version_updater_delegate.cc",
     "login/version_updater/mock_version_updater_delegate.h",
     "platform_keys/key_permissions/mock_key_permissions_service.cc",

chrome/browser/chromeos/login/screens/family_link_notice_browsertest.cc

@@ -9,9 +9,9 @@
 #include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
-#include "chrome/browser/chromeos/login/test/oobe_screen_exit_waiter.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 #include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
+#include "chrome/browser/chromeos/login/test/wizard_controller_screen_exit_waiter.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -53,7 +53,7 @@ class FamilyLinkNoticeScreenTest : public OobeBaseTest {
 
   void LoginAsRegularUser() {
     login_manager_mixin_.LoginAsNewRegularUser();
-    OobeScreenExitWaiter(UserCreationView::kScreenId).Wait();
+    WizardControllerExitWaiter(UserCreationView::kScreenId).Wait();
   }
 
   void ExpectHelpAppPrefValue(bool expected) {
@@ -145,7 +145,7 @@ class FamilyLinkNoticeScreenChildTest : public FamilyLinkNoticeScreenTest {
 
   void LoginAsChildUser() {
     login_manager_mixin_.LoginAsNewChildUser();
-    OobeScreenExitWaiter(UserCreationView::kScreenId).Wait();
+    WizardControllerExitWaiter(UserCreationView::kScreenId).Wait();
   }
 
  private:
@@ -186,7 +186,7 @@ class FamilyLinkNoticeScreenManagedTest : public FamilyLinkNoticeScreenTest {
   void LoginAsManagedUser() {
     user_policy_mixin_.RequestPolicyUpdate();
     login_manager_mixin_.LoginWithDefaultContext(test_user_);
-    OobeScreenExitWaiter(UserCreationView::kScreenId).Wait();
+    WizardControllerExitWaiter(UserCreationView::kScreenId).Wait();
   }
 
  private:

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

@@ -17,14 +17,14 @@ OobeScreenExitWaiter::OobeScreenExitWaiter(OobeScreenId target_screen)
 OobeScreenExitWaiter::~OobeScreenExitWaiter() = default;
 
 void OobeScreenExitWaiter::Wait() {
-  DCHECK_EQ(State::IDLE, state_);
+  ASSERT_EQ(State::IDLE, state_);
 
   OobeUI* oobe_ui = GetOobeUI();
   if (!oobe_ui || oobe_ui->current_screen() != target_screen_) {
     state_ = State::DONE;
     return;
   }
-  DCHECK(!run_loop_);
+  ASSERT_FALSE(run_loop_);
 
   oobe_ui_observer_.Add(GetOobeUI());
 
@@ -43,7 +43,7 @@ void OobeScreenExitWaiter::Wait() {
 
 void OobeScreenExitWaiter::OnCurrentScreenChanged(OobeScreenId current_screen,
                                                   OobeScreenId new_screen) {
-  DCHECK_NE(state_, State::IDLE);
+  ASSERT_NE(state_, State::IDLE);
   if (new_screen != target_screen_)
     EndWait();
 }

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

+// Copyright 2020 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/chromeos/login/test/wizard_controller_screen_exit_waiter.h"
+
+#include "base/logging.h"
+#include "base/run_loop.h"
+#include "chrome/browser/chromeos/login/wizard_controller.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+WizardControllerExitWaiter::WizardControllerExitWaiter(OobeScreenId screen_id)
+    : WizardControllerExitWaiter(
+          WizardController::default_controller()->GetScreen(screen_id)) {}
+
+WizardControllerExitWaiter::WizardControllerExitWaiter(
+    BaseScreen* target_screen)
+    : target_screen_(target_screen) {}
+
+WizardControllerExitWaiter::~WizardControllerExitWaiter() = default;
+
+void WizardControllerExitWaiter::Wait() {
+  ASSERT_EQ(State::IDLE, state_);
+
+  WizardController* wizard_controller = WizardController::default_controller();
+  if (!wizard_controller ||
+      wizard_controller->current_screen() != target_screen_) {
+    state_ = State::DONE;
+    return;
+  }
+  ASSERT_FALSE(run_loop_);
+
+  screen_observer_.Add(wizard_controller);
+
+  state_ = State::WAITING_FOR_SCREEN_EXIT;
+
+  LOG(INFO) << "Actually waiting for exiting screen "
+            << target_screen_->screen_id();
+
+  run_loop_ = std::make_unique<base::RunLoop>();
+  run_loop_->Run();
+  run_loop_.reset();
+
+  ASSERT_EQ(State::DONE, state_);
+
+  screen_observer_.RemoveAll();
+}
+
+void WizardControllerExitWaiter::OnCurrentScreenChanged(
+    BaseScreen* new_screen) {
+  ASSERT_NE(state_, State::IDLE);
+  if (new_screen != target_screen_)
+    EndWait();
+}
+
+void WizardControllerExitWaiter::OnShutdown() {
+  screen_observer_.RemoveAll();
+  EndWait();
+}
+
+void WizardControllerExitWaiter::EndWait() {
+  if (state_ == State::DONE)
+    return;
+
+  state_ = State::DONE;
+  run_loop_->Quit();
+}
+
+}  // namespace chromeos

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

+// Copyright 2020 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_WIZARD_CONTROLLER_SCREEN_EXIT_WAITER_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_WIZARD_CONTROLLER_SCREEN_EXIT_WAITER_H_
+
+#include "base/macros.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/chromeos/login/oobe_screen.h"
+#include "chrome/browser/chromeos/login/screens/base_screen.h"
+#include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
+#include "chrome/browser/chromeos/login/wizard_controller.h"
+
+namespace base {
+class RunLoop;
+}
+
+namespace chromeos {
+
+// A waiter that blocks until the the current WizardController screen is
+// different than the target screen, or the WizardController is destroyed.
+class WizardControllerExitWaiter : public test::TestConditionWaiter,
+                                   public WizardController::ScreenObserver {
+ public:
+  explicit WizardControllerExitWaiter(OobeScreenId screen_id);
+  explicit WizardControllerExitWaiter(BaseScreen* target_screen);
+  ~WizardControllerExitWaiter() override;
+
+  // WizardController::ScreenObserver:
+  void OnCurrentScreenChanged(BaseScreen* new_screen) override;
+  void OnShutdown() override;
+
+  // TestConditionWaiter;
+  void Wait() override;
+
+ private:
+  enum class State { IDLE, WAITING_FOR_SCREEN_EXIT, DONE };
+
+  void EndWait();
+
+  const BaseScreen* target_screen_;
+
+  State state_ = State::IDLE;
+
+  ScopedObserver<WizardController, WizardController::ScreenObserver>
+      screen_observer_{this};
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_WIZARD_CONTROLLER_SCREEN_EXIT_WAITER_H_

chrome/browser/chromeos/login/wizard_controller.cc

@@ -371,6 +371,8 @@ WizardController::WizardController()
 }
 
 WizardController::~WizardController() {
+  for (ScreenObserver& obs : screen_observers_)
+    obs.OnShutdown();
   screen_manager_.reset();
 }
 
@@ -1572,8 +1574,10 @@ void WizardController::SetCurrentScreen(BaseScreen* new_current) {
   previous_screen_ = current_screen_;
   current_screen_ = new_current;
 
-  if (!current_screen_)
+  if (!current_screen_) {
+    NotifyScreenChanged();
     return;
+  }
 
   // Record show time for UMA.
   screen_show_times_[new_current->screen_id()] = base::TimeTicks::Now();
@@ -1586,6 +1590,7 @@ void WizardController::SetCurrentScreen(BaseScreen* new_current) {
 
   UpdateStatusAreaVisibilityForScreen(current_screen_->screen_id());
   current_screen_->Show(wizard_context_.get());
+  NotifyScreenChanged();
 }
 
 void WizardController::UpdateStatusAreaVisibilityForScreen(
@@ -1874,6 +1879,14 @@ bool WizardController::IsSigninScreen(OobeScreenId screen_id) {
          screen_id == GaiaView::kScreenId;
 }
 
+void WizardController::AddObserver(ScreenObserver* obs) {
+  screen_observers_.AddObserver(obs);
+}
+
+void WizardController::RemoveObserver(ScreenObserver* obs) {
+  screen_observers_.RemoveObserver(obs);
+}
+
 void WizardController::OnLocalStateInitialized(bool /* succeeded */) {
   if (GetLocalState()->GetInitializationStatus() !=
       PrefService::INITIALIZATION_STATUS_ERROR) {
@@ -2033,6 +2046,11 @@ void WizardController::StartEnrollmentScreen(bool force_interactive) {
   SetCurrentScreen(screen);
 }
 
+void WizardController::NotifyScreenChanged() {
+  for (ScreenObserver& obs : screen_observers_)
+    obs.OnCurrentScreenChanged(current_screen_);
+}
+
 AutoEnrollmentController* WizardController::GetAutoEnrollmentController() {
   if (!auto_enrollment_controller_)
     auto_enrollment_controller_ = std::make_unique<AutoEnrollmentController>();

chrome/browser/chromeos/login/wizard_controller.h

@@ -70,6 +70,12 @@ struct TimeZoneResponseData;
 // interacts with screen controllers to move the user between screens.
 class WizardController {
  public:
+  class ScreenObserver : public base::CheckedObserver {
+   public:
+    virtual void OnCurrentScreenChanged(BaseScreen* new_screen) = 0;
+    virtual void OnShutdown() = 0;
+  };
+
   WizardController();
   ~WizardController();
 
@@ -203,6 +209,9 @@ class WizardController {
     return first_screen_for_testing_;
   }
 
+  void AddObserver(ScreenObserver* obs);
+  void RemoveObserver(ScreenObserver* obs);
+
  private:
   // Create BaseScreen instances. These are owned by |screen_manager_|.
   std::vector<std::unique_ptr<BaseScreen>> CreateScreens();
@@ -373,9 +382,7 @@ class WizardController {
   // attestation-based enrollment if appropriate.
   void StartEnrollmentScreen(bool force_interactive);
 
-  void OnConfigurationLoaded(
-      OobeScreenId first_screen,
-      std::unique_ptr<base::DictionaryValue> configuration);
+  void NotifyScreenChanged();
 
   // Returns auto enrollment controller (lazily initializes one if it doesn't
   // exist already).
@@ -462,6 +469,8 @@ class WizardController {
 
   bool is_initialized_ = false;
 
+  base::ObserverList<ScreenObserver> screen_observers_;
+
   base::WeakPtrFactory<WizardController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(WizardController);