diff --git a/chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc b/chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc index d1029f88bedda..534381c1b06b5 100644 --- a/chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc +++ b/chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc @@ -41,8 +41,8 @@ class EnrollmentScreenTest : public WizardInProcessBrowserTest { IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, TestCancel) { ASSERT_TRUE(WizardController::default_controller()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; @@ -69,8 +69,8 @@ IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, DISABLED_TestSuccess) { ASSERT_TRUE(WizardController::default_controller()); EXPECT_FALSE(StartupUtils::IsOobeCompleted()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; @@ -105,8 +105,8 @@ class AttestationAuthEnrollmentScreenTest : public EnrollmentScreenTest { IN_PROC_BROWSER_TEST_F(AttestationAuthEnrollmentScreenTest, TestCancel) { ASSERT_TRUE(WizardController::default_controller()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; @@ -133,7 +133,8 @@ IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, EnrollmentSpinner) { WizardController* wcontroller = WizardController::default_controller(); ASSERT_TRUE(wcontroller); - EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get(wcontroller); + EnrollmentScreen* enrollment_screen = + EnrollmentScreen::Get(wcontroller->screen_manager()); ASSERT_TRUE(enrollment_screen); EnrollmentScreenView* view = enrollment_screen->GetView(); @@ -177,8 +178,8 @@ class ForcedAttestationAuthEnrollmentScreenTest : public EnrollmentScreenTest { IN_PROC_BROWSER_TEST_F(ForcedAttestationAuthEnrollmentScreenTest, TestCancel) { ASSERT_TRUE(WizardController::default_controller()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; @@ -224,8 +225,8 @@ class MultiAuthEnrollmentScreenTest : public EnrollmentScreenTest { IN_PROC_BROWSER_TEST_F(MultiAuthEnrollmentScreenTest, TestCancel) { ASSERT_TRUE(WizardController::default_controller()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; @@ -269,8 +270,8 @@ class ProvisionedEnrollmentScreenTest : public EnrollmentScreenTest { IN_PROC_BROWSER_TEST_F(ProvisionedEnrollmentScreenTest, TestBackButton) { ASSERT_TRUE(WizardController::default_controller()); - EnrollmentScreen* enrollment_screen = - EnrollmentScreen::Get(WizardController::default_controller()); + EnrollmentScreen* enrollment_screen = EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(enrollment_screen); base::RunLoop run_loop; diff --git a/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc b/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc index 41f22f9031111..7b602e29762fd 100644 --- a/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc +++ b/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc @@ -201,7 +201,8 @@ class EnterpriseEnrollmentTest : public LoginManagerTest { // Helper method to return the current EnrollmentScreen instance. EnrollmentScreen* enrollment_screen() { - return EnrollmentScreen::Get(WizardController::default_controller()); + return EnrollmentScreen::Get( + WizardController::default_controller()->screen_manager()); } private: diff --git a/chrome/browser/chromeos/login/oobe_localization_browsertest.cc b/chrome/browser/chromeos/login/oobe_localization_browsertest.cc index 8dd34734a4ab4..b389b27f47210 100644 --- a/chrome/browser/chromeos/login/oobe_localization_browsertest.cc +++ b/chrome/browser/chromeos/login/oobe_localization_browsertest.cc @@ -94,8 +94,8 @@ class TimedRunLoop { class LanguageListWaiter : public NetworkScreen::Observer { public: LanguageListWaiter() - : network_screen_( - NetworkScreen::Get(WizardController::default_controller())), + : network_screen_(NetworkScreen::Get( + WizardController::default_controller()->screen_manager())), loop_(base::TimeDelta::FromSeconds(kTimeoutSeconds), "LanguageList") { network_screen_->AddObserver(this); CheckLanguageList(); diff --git a/chrome/browser/chromeos/login/screen_manager.cc b/chrome/browser/chromeos/login/screen_manager.cc index 8457c5a667114..23ee4fa69a51c 100644 --- a/chrome/browser/chromeos/login/screen_manager.cc +++ b/chrome/browser/chromeos/login/screen_manager.cc @@ -4,22 +4,24 @@ #include "chrome/browser/chromeos/login/screen_manager.h" +#include "base/memory/ptr_util.h" +#include "chrome/browser/chromeos/login/wizard_controller.h" + namespace chromeos { -ScreenManager::ScreenManager() { -} +ScreenManager::ScreenManager(WizardController* wizard_controller) + : wizard_controller_(wizard_controller) {} -ScreenManager::~ScreenManager() { -} +ScreenManager::~ScreenManager() {} BaseScreen* ScreenManager::GetScreen(OobeScreen screen) { auto iter = screens_.find(screen); if (iter != screens_.end()) return iter->second.get(); - BaseScreen* result = CreateScreen(screen); + BaseScreen* result = wizard_controller_->CreateScreen(screen); DCHECK(result) << "Can not create screen named " << GetOobeScreenName(screen); - screens_[screen] = make_linked_ptr(result); + screens_[screen] = base::WrapUnique(result); return result; } diff --git a/chrome/browser/chromeos/login/screen_manager.h b/chrome/browser/chromeos/login/screen_manager.h index 360d2b28e96c5..e0b1383e26da8 100644 --- a/chrome/browser/chromeos/login/screen_manager.h +++ b/chrome/browser/chromeos/login/screen_manager.h @@ -6,26 +6,26 @@ #define CHROME_BROWSER_CHROMEOS_LOGIN_SCREEN_MANAGER_H_ #include +#include #include #include "base/gtest_prod_util.h" #include "base/macros.h" -#include "base/memory/linked_ptr.h" #include "chrome/browser/chromeos/login/screens/base_screen.h" namespace chromeos { +class WizardController; + // Class that manages creation and ownership of screens. class ScreenManager { public: - ScreenManager(); - virtual ~ScreenManager(); + // |wizard_controller| is not owned by this class. + explicit ScreenManager(WizardController* wizard_controller); + ~ScreenManager(); // Getter for screen with lazy initialization. - virtual BaseScreen* GetScreen(OobeScreen screen); - - // Factory for screen instances. - virtual BaseScreen* CreateScreen(OobeScreen screen) = 0; + BaseScreen* GetScreen(OobeScreen screen); bool HasScreen(OobeScreen screen); @@ -37,8 +37,11 @@ class ScreenManager { friend class WizardInProcessBrowserTest; friend class WizardControllerBrokenLocalStateTest; - // Screens. - std::map> screens_; + // Created screens. + std::map> screens_; + + // Used to allocate BaseScreen instances. Unowned. + WizardController* wizard_controller_; DISALLOW_COPY_AND_ASSIGN(ScreenManager); }; diff --git a/chrome/browser/chromeos/login/screens/network_screen_browsertest.cc b/chrome/browser/chromeos/login/screens/network_screen_browsertest.cc index 55da5ebc64dd4..3337dee077b0b 100644 --- a/chrome/browser/chromeos/login/screens/network_screen_browsertest.cc +++ b/chrome/browser/chromeos/login/screens/network_screen_browsertest.cc @@ -71,8 +71,8 @@ class NetworkScreenTest : public WizardInProcessBrowserTest { WizardInProcessBrowserTest::SetUpOnMainThread(); mock_base_screen_delegate_.reset(new MockBaseScreenDelegate()); ASSERT_TRUE(WizardController::default_controller() != nullptr); - network_screen_ = - NetworkScreen::Get(WizardController::default_controller()); + network_screen_ = NetworkScreen::Get( + WizardController::default_controller()->screen_manager()); ASSERT_TRUE(network_screen_ != nullptr); ASSERT_EQ(WizardController::default_controller()->current_screen(), network_screen_); diff --git a/chrome/browser/chromeos/login/screens/update_screen_browsertest.cc b/chrome/browser/chromeos/login/screens/update_screen_browsertest.cc index 0eff665908a3f..84dbbc83db982 100644 --- a/chrome/browser/chromeos/login/screens/update_screen_browsertest.cc +++ b/chrome/browser/chromeos/login/screens/update_screen_browsertest.cc @@ -83,9 +83,10 @@ class UpdateScreenTest : public WizardInProcessBrowserTest { WizardInProcessBrowserTest::SetUpOnMainThread(); - ASSERT_TRUE(WizardController::default_controller() != NULL); - update_screen_ = UpdateScreen::Get(WizardController::default_controller()); - ASSERT_TRUE(update_screen_ != NULL); + ASSERT_TRUE(WizardController::default_controller() != nullptr); + update_screen_ = UpdateScreen::Get( + WizardController::default_controller()->screen_manager()); + ASSERT_TRUE(update_screen_ != nullptr); ASSERT_EQ(WizardController::default_controller()->current_screen(), update_screen_); update_screen_->base_screen_delegate_ = mock_base_screen_delegate_.get(); diff --git a/chrome/browser/chromeos/login/supervised/supervised_user_creation_flow.cc b/chrome/browser/chromeos/login/supervised/supervised_user_creation_flow.cc index 394046d6e5e62..ff70fbbd33876 100644 --- a/chrome/browser/chromeos/login/supervised/supervised_user_creation_flow.cc +++ b/chrome/browser/chromeos/login/supervised/supervised_user_creation_flow.cc @@ -18,8 +18,8 @@ namespace { SupervisedUserCreationScreen* GetScreen(LoginDisplayHost* host) { DCHECK(host); DCHECK(host->GetWizardController()); - SupervisedUserCreationScreen* result = - SupervisedUserCreationScreen::Get(host->GetWizardController()); + SupervisedUserCreationScreen* result = SupervisedUserCreationScreen::Get( + host->GetWizardController()->screen_manager()); DCHECK(result); return result; } diff --git a/chrome/browser/chromeos/login/users/avatar/user_image_sync_observer.cc b/chrome/browser/chromeos/login/users/avatar/user_image_sync_observer.cc index 9ec5471cc52d2..747c037f3ca07 100644 --- a/chrome/browser/chromeos/login/users/avatar/user_image_sync_observer.cc +++ b/chrome/browser/chromeos/login/users/avatar/user_image_sync_observer.cc @@ -189,7 +189,8 @@ bool UserImageSyncObserver::GetSyncedImageIndex(int* index) { bool UserImageSyncObserver::CanUpdateLocalImageNow() { if (WizardController* wizard_controller = WizardController::default_controller()) { - UserImageScreen* screen = UserImageScreen::Get(wizard_controller); + UserImageScreen* screen = + UserImageScreen::Get(wizard_controller->screen_manager()); if (wizard_controller->current_screen() == screen) { if (screen->user_selected_image()) return false; diff --git a/chrome/browser/chromeos/login/wizard_controller.cc b/chrome/browser/chromeos/login/wizard_controller.cc index eeb7042c2ddbe..b06e37f0a7071 100644 --- a/chrome/browser/chromeos/login/wizard_controller.cc +++ b/chrome/browser/chromeos/login/wizard_controller.cc @@ -251,7 +251,10 @@ bool WizardController::zero_delay_enabled_ = false; PrefService* WizardController::local_state_for_testing_ = nullptr; WizardController::WizardController(LoginDisplayHost* host, OobeUI* oobe_ui) - : host_(host), oobe_ui_(oobe_ui), weak_factory_(this) { + : screen_manager_(this), + host_(host), + oobe_ui_(oobe_ui), + weak_factory_(this) { DCHECK(default_controller_ == nullptr); default_controller_ = this; if (!ash_util::IsRunningInMash()) { @@ -358,7 +361,7 @@ ErrorScreen* WizardController::GetErrorScreen() { BaseScreen* WizardController::GetScreen(OobeScreen screen) { if (screen == OobeScreen::SCREEN_ERROR_MESSAGE) return GetErrorScreen(); - return ScreenManager::GetScreen(screen); + return screen_manager_.GetScreen(screen); } BaseScreen* WizardController::CreateScreen(OobeScreen screen) { @@ -430,7 +433,8 @@ void WizardController::ShowNetworkScreen() { VLOG(1) << "Showing network screen."; // Hide the status area initially; it only appears after OOBE first animates // in. Keep it visible if the user goes back to the existing network screen. - SetStatusAreaVisible(HasScreen(OobeScreen::SCREEN_OOBE_NETWORK)); + SetStatusAreaVisible( + screen_manager_.HasScreen(OobeScreen::SCREEN_OOBE_NETWORK)); SetCurrentScreen(GetScreen(OobeScreen::SCREEN_OOBE_NETWORK)); // There are two possible screens where we listen to the incoming Bluetooth @@ -582,7 +586,8 @@ void WizardController::ShowWrongHWIDScreen() { void WizardController::ShowAutoEnrollmentCheckScreen() { VLOG(1) << "Showing Auto-enrollment check screen."; SetStatusAreaVisible(true); - AutoEnrollmentCheckScreen* screen = AutoEnrollmentCheckScreen::Get(this); + AutoEnrollmentCheckScreen* screen = + AutoEnrollmentCheckScreen::Get(screen_manager()); if (retry_auto_enrollment_check_) screen->ClearState(); screen->set_auto_enrollment_controller(host_->GetAutoEnrollmentController()); @@ -893,7 +898,7 @@ void WizardController::InitiateOOBEUpdate() { void WizardController::StartOOBEUpdate() { VLOG(1) << "StartOOBEUpdate"; SetCurrentScreenSmooth(GetScreen(OobeScreen::SCREEN_OOBE_UPDATE), true); - UpdateScreen::Get(this)->StartNetworkCheck(); + UpdateScreen::Get(screen_manager())->StartNetworkCheck(); } void WizardController::StartTimezoneResolve() { @@ -1195,7 +1200,7 @@ bool WizardController::GetUsageStatisticsReporting() const { void WizardController::SetHostNetwork() { if (!shark_controller_) return; - NetworkScreen* network_screen = NetworkScreen::Get(this); + NetworkScreen* network_screen = NetworkScreen::Get(screen_manager()); std::string onc_spec; network_screen->GetConnectedWifiNetwork(&onc_spec); if (!onc_spec.empty()) @@ -1205,7 +1210,7 @@ void WizardController::SetHostNetwork() { void WizardController::SetHostConfiguration() { if (!shark_controller_) return; - NetworkScreen* network_screen = NetworkScreen::Get(this); + NetworkScreen* network_screen = NetworkScreen::Get(screen_manager()); shark_controller_->SetHostConfiguration( true, // Eula must be accepted before we get this far. network_screen->GetApplicationLocale(), network_screen->GetTimezone(), @@ -1224,7 +1229,7 @@ void WizardController::ConfigureHostRequested( StartupUtils::MarkEulaAccepted(); SetUsageStatisticsReporting(send_reports); - NetworkScreen* network_screen = NetworkScreen::Get(this); + NetworkScreen* network_screen = NetworkScreen::Get(screen_manager()); network_screen->SetApplicationLocaleAndInputMethod(lang, keyboard_layout); network_screen->SetTimezone(timezone); @@ -1241,7 +1246,7 @@ void WizardController::AddNetworkRequested(const std::string& onc_spec) { remora_controller_->OnNetworkConnectivityChanged( pairing_chromeos::HostPairingController::CONNECTIVITY_CONNECTING); - NetworkScreen* network_screen = NetworkScreen::Get(this); + NetworkScreen* network_screen = NetworkScreen::Get(screen_manager()); const chromeos::NetworkState* network_state = chromeos::NetworkHandler::Get() ->network_state_handler() ->DefaultNetwork(); @@ -1506,7 +1511,7 @@ void WizardController::StartEnrollmentScreen(bool force_interactive) { : policy::EnrollmentConfig::MODE_MANUAL_REENROLLMENT; } - EnrollmentScreen* screen = EnrollmentScreen::Get(this); + EnrollmentScreen* screen = EnrollmentScreen::Get(screen_manager()); screen->SetParameters(effective_config, shark_controller_.get()); SetStatusAreaVisible(true); SetCurrentScreen(screen); diff --git a/chrome/browser/chromeos/login/wizard_controller.h b/chrome/browser/chromeos/login/wizard_controller.h index d11dffc256f3c..8e9d150043a41 100644 --- a/chrome/browser/chromeos/login/wizard_controller.h +++ b/chrome/browser/chromeos/login/wizard_controller.h @@ -51,7 +51,6 @@ struct TimeZoneResponseData; // Class that manages control flow between wizard screens. Wizard controller // interacts with screen controllers to move the user between screens. class WizardController : public BaseScreenDelegate, - public ScreenManager, public EulaScreen::Delegate, public ControllerPairingScreen::Delegate, public HostPairingScreen::Delegate, @@ -113,13 +112,19 @@ class WizardController : public BaseScreenDelegate, // Returns true if the current wizard instance has reached the login screen. bool login_screen_started() const { return login_screen_started_; } - // ScreenManager implementation. - BaseScreen* GetScreen(OobeScreen screen) override; - BaseScreen* CreateScreen(OobeScreen screen) override; + // Returns a given screen. Creates it lazily. + BaseScreen* GetScreen(OobeScreen screen); + + // Returns the current ScreenManager instance. + ScreenManager* screen_manager() { return &screen_manager_; } // Volume percent at which spoken feedback is still audible. static const int kMinAudibleOutputVolumePercent; + // Allocate a given BaseScreen for the given |Screen|. Used by + // |screen_manager_|. + BaseScreen* CreateScreen(OobeScreen screen); + private: // Show specific screen. void ShowNetworkScreen(); @@ -300,6 +305,8 @@ class WizardController : public BaseScreenDelegate, // attestation-based enrollment if appropriate. void StartEnrollmentScreen(bool force_interactive); + ScreenManager screen_manager_; + // Whether to skip any screens that may normally be shown after login // (registration, Terms of Service, user image selection). static bool skip_post_login_screens_; diff --git a/chrome/browser/chromeos/login/wizard_controller_browsertest.cc b/chrome/browser/chromeos/login/wizard_controller_browsertest.cc index bb21659416511..fc2123da0b0bd 100644 --- a/chrome/browser/chromeos/login/wizard_controller_browsertest.cc +++ b/chrome/browser/chromeos/login/wizard_controller_browsertest.cc @@ -222,20 +222,22 @@ class MockOutShowHide : public T { std::unique_ptr view_; }; -#define MOCK(mock_var, screen_name, mocked_class, view_class) \ - mock_var = new MockOutShowHide( \ - WizardController::default_controller(), new view_class); \ - WizardController::default_controller()->screens_[screen_name] = \ - make_linked_ptr(mock_var); \ - EXPECT_CALL(*mock_var, Show()).Times(0); \ +#define MOCK(mock_var, screen_name, mocked_class, view_class) \ + mock_var = new MockOutShowHide( \ + WizardController::default_controller(), new view_class); \ + WizardController::default_controller() \ + ->screen_manager() \ + ->screens_[screen_name] = base::WrapUnique(mock_var); \ + EXPECT_CALL(*mock_var, Show()).Times(0); \ EXPECT_CALL(*mock_var, Hide()).Times(0); #define MOCK_WITH_DELEGATE(mock_var, screen_name, mocked_class, view_class) \ mock_var = new MockOutShowHide( \ WizardController::default_controller(), \ WizardController::default_controller(), new view_class); \ - WizardController::default_controller()->screens_[screen_name] = \ - make_linked_ptr(mock_var); \ + WizardController::default_controller() \ + ->screen_manager() \ + ->screens_[screen_name] = base::WrapUnique(mock_var); \ EXPECT_CALL(*mock_var, Show()).Times(0); \ EXPECT_CALL(*mock_var, Hide()).Times(0); @@ -419,11 +421,13 @@ class WizardControllerFlowTest : public WizardControllerTest { NetworkHandler::Get()->network_state_handler()->SetCheckPortalList(""); // Set up the mocks for all screens. - mock_network_screen_.reset(new MockNetworkScreen( + mock_network_screen_ = new MockNetworkScreen( WizardController::default_controller(), - WizardController::default_controller(), GetOobeUI()->GetNetworkView())); + WizardController::default_controller(), GetOobeUI()->GetNetworkView()); WizardController::default_controller() - ->screens_[OobeScreen::SCREEN_OOBE_NETWORK] = mock_network_screen_; + ->screen_manager() + ->screens_[OobeScreen::SCREEN_OOBE_NETWORK] + .reset(mock_network_screen_); EXPECT_CALL(*mock_network_screen_, Show()).Times(0); EXPECT_CALL(*mock_network_screen_, Hide()).Times(0); @@ -442,9 +446,10 @@ class WizardControllerFlowTest : public WizardControllerTest { OobeScreen::SCREEN_OOBE_ENABLE_DEBUGGING, MockEnableDebuggingScreen, MockEnableDebuggingScreenView); device_disabled_screen_view_.reset(new MockDeviceDisabledScreenView); - wizard_controller->screens_[OobeScreen::SCREEN_DEVICE_DISABLED] = - make_linked_ptr(new DeviceDisabledScreen( - wizard_controller, device_disabled_screen_view_.get())); + wizard_controller->screen_manager() + ->screens_[OobeScreen::SCREEN_DEVICE_DISABLED] = + base::MakeUnique( + wizard_controller, device_disabled_screen_view_.get()); EXPECT_CALL(*device_disabled_screen_view_, Show()).Times(0); // Switch to the initial screen. @@ -454,7 +459,7 @@ class WizardControllerFlowTest : public WizardControllerTest { } void TearDownOnMainThread() override { - mock_network_screen_.reset(); + mock_network_screen_ = nullptr; device_disabled_screen_view_.reset(); WizardControllerTest::TearDownOnMainThread(); } @@ -512,7 +517,7 @@ class WizardControllerFlowTest : public WizardControllerTest { } void ResetAutoEnrollmentCheckScreen() { - WizardController::default_controller()->screens_.erase( + WizardController::default_controller()->screen_manager()->screens_.erase( OobeScreen::SCREEN_AUTO_ENROLLMENT_CHECK); } @@ -567,7 +572,7 @@ class WizardControllerFlowTest : public WizardControllerTest { ->GetCurrentTimezoneID())); } - linked_ptr mock_network_screen_; + MockNetworkScreen* mock_network_screen_; // Unowned ptr. MockOutShowHide* mock_update_screen_; MockOutShowHide* mock_eula_screen_; MockOutShowHide*