[Oobe WebUI] Make the screen switch go through CoreOobeHandler

* Moved BaseWebUIHandler::ShowScreen(WithData) into CoreOobeHandler * Introduce BaseScreenHandler::ShowScreen with optional data dict This is done to isolate the handlers from each other. Each handler would have their own javascript queue independent from each other. This would allow us to initialize handlers independently and also get rid of the show_on_init logic everywhere. Bug: 1309022, 1298392, 1310191, 1310195 Change-Id: I68247da78d126481d5db4ba0d57c46840ab01fa9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3548876 Commit-Queue: Roman Sorokin <rsorokin@chromium.org> Auto-Submit: Roman Sorokin <rsorokin@chromium.org> Reviewed-by: Osama Fathy <osamafathy@google.com> Commit-Queue: Osama Fathy <osamafathy@google.com> Cr-Commit-Position: refs/heads/main@{#985292}
chromium · Mar 25, 2022 · 9693685 · 9693685
1 parent 4b17bc1
commit 9693685
Show file tree

Hide file tree

Showing 57 changed files with 172 additions and 155 deletions.
diff --git a/chrome/browser/ui/webui/chromeos/login/active_directory_login_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/active_directory_login_screen_handler.cc
@@ -59,18 +59,18 @@ void ActiveDirectoryLoginScreenHandler::Show() {
     return;
   }
 
-  base::DictionaryValue screen_data;
-  screen_data.SetStringKey("realm", g_browser_process->platform_part()
-                                        ->browser_policy_connector_ash()
-                                        ->GetRealm());
+  base::Value::Dict screen_data;
+  screen_data.Set("realm", g_browser_process->platform_part()
+                               ->browser_policy_connector_ash()
+                               ->GetRealm());
   std::string email_domain;
   if (CrosSettings::Get()->GetString(kAccountsPrefLoginScreenDomainAutoComplete,
                                      &email_domain) &&
       !email_domain.empty()) {
-    screen_data.SetStringKey("emailDomain", email_domain);
+    screen_data.Set("emailDomain", email_domain);
   }
 
-  ShowScreenWithData(kScreenId, &screen_data);
+  ShowInWebUI(std::move(screen_data));
 }
 
 void ActiveDirectoryLoginScreenHandler::Bind(

diff --git a/chrome/browser/ui/webui/chromeos/login/active_directory_password_change_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/active_directory_password_change_screen_handler.cc
@@ -47,10 +47,10 @@ void ActiveDirectoryPasswordChangeScreenHandler::RegisterMessages() {
 void ActiveDirectoryPasswordChangeScreenHandler::Show(
     const std::string& username,
     int error) {
-  base::DictionaryValue data;
-  data.SetStringKey(kUsernameKey, username);
-  data.SetIntKey(kErrorKey, error);
-  ShowScreenWithData(kScreenId, &data);
+  base::Value::Dict data;
+  data.Set(kUsernameKey, username);
+  data.Set(kErrorKey, error);
+  ShowInWebUI(std::move(data));
 }
 
 void ActiveDirectoryPasswordChangeScreenHandler::Bind(

diff --git a/chrome/browser/ui/webui/chromeos/login/app_downloading_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/app_downloading_screen_handler.cc
@@ -63,9 +63,9 @@ void AppDownloadingScreenHandler::Bind(AppDownloadingScreen* screen) {
 }
 
 void AppDownloadingScreenHandler::Show() {
-  base::DictionaryValue data;
-  data.SetKey("numOfApps", base::Value(GetNumberOfUserSelectedApps()));
-  ShowScreenWithData(kScreenId, &data);
+  base::Value::Dict data;
+  data.Set("numOfApps", GetNumberOfUserSelectedApps());
+  ShowInWebUI(std::move(data));
 }
 
 void AppDownloadingScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc
@@ -85,16 +85,16 @@ void AppLaunchSplashScreenHandler::Show() {
 
   is_shown_ = true;
 
-  base::DictionaryValue data;
-  data.SetBoolKey("shortcutEnabled",
-                  !KioskAppManager::Get()->GetDisableBailoutShortcut());
+  base::Value::Dict data;
+  data.Set("shortcutEnabled",
+           !KioskAppManager::Get()->GetDisableBailoutShortcut());
 
   base::DictionaryValue app_info;
   PopulateAppInfo(&app_info);
-  data.SetKey("appInfo", std::move(app_info));
+  data.Set("appInfo", std::move(app_info));
 
   SetLaunchText(l10n_util::GetStringUTF8(GetProgressMessageFromState(state_)));
-  ShowScreenWithData(kScreenId, &data);
+  ShowInWebUI(std::move(data));
   if (toggle_network_config_on_show_.has_value()) {
     DoToggleNetworkConfig(toggle_network_config_on_show_.value());
     toggle_network_config_on_show_.reset();

diff --git a/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
@@ -341,7 +341,7 @@ void ArcTermsOfServiceScreenHandler::DoShow() {
 
   action_taken_ = false;
 
-  ShowScreen(kScreenId);
+  ShowInWebUI();
   arc_managed_ = arc::IsArcPlayStoreEnabledPreferenceManagedForProfile(profile);
   is_child_account_ = user_manager::UserManager::Get()->IsLoggedInAsChildUser();
   CallJS("login.ArcTermsOfServiceScreen.setArcManaged", arc_managed_,
@@ -360,7 +360,7 @@ void ArcTermsOfServiceScreenHandler::DoShowForDemoModeSetup() {
 
   CallJS("login.ArcTermsOfServiceScreen.setupForDemoMode");
   action_taken_ = false;
-  ShowScreen(kScreenId);
+  ShowInWebUI();
   MaybeLoadPlayStoreToS(true);
   StartNetworkAndTimeZoneObserving();
 }

diff --git a/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.cc
@@ -223,7 +223,7 @@ void AssistantOptInFlowScreenHandler::Show() {
 
   SetupAssistantConnection();
 
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void AssistantOptInFlowScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/auto_enrollment_check_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/auto_enrollment_check_screen_handler.cc
@@ -31,7 +31,7 @@ void AutoEnrollmentCheckScreenHandler::Show() {
     show_on_init_ = true;
     return;
   }
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void AutoEnrollmentCheckScreenHandler::SetDelegate(Delegate* delegate) {

diff --git a/chrome/browser/ui/webui/chromeos/login/base_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/base_screen_handler.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h"
 
+#include "chrome/browser/ash/login/oobe_screen.h"
 #include "chrome/browser/ash/login/screens/base_screen.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_webui_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
@@ -12,7 +13,9 @@ namespace chromeos {
 
 BaseScreenHandler::BaseScreenHandler(OobeScreenId oobe_screen,
                                      JSCallsContainer* js_calls_container)
-    : BaseWebUIHandler(js_calls_container), oobe_screen_(oobe_screen) {}
+    : BaseWebUIHandler(js_calls_container), oobe_screen_(oobe_screen) {
+  DCHECK_NE(oobe_screen_.name, OobeScreen::SCREEN_UNKNOWN.name);
+}
 
 BaseScreenHandler::~BaseScreenHandler() = default;
 
@@ -22,6 +25,13 @@ void BaseScreenHandler::SetBaseScreen(BaseScreen* base_screen) {
   base_screen_ = base_screen;
 }
 
+void BaseScreenHandler::ShowInWebUI(absl::optional<base::Value::Dict> data) {
+  if (!GetOobeUI())
+    return;
+  GetOobeUI()->GetCoreOobeView()->ShowScreenWithData(oobe_screen_,
+                                                     std::move(data));
+}
+
 void BaseScreenHandler::RegisterMessages() {
   if (!user_acted_method_path_.empty()) {
     AddCallback(user_acted_method_path_, &BaseScreenHandler::HandleUserAction);

diff --git a/chrome/browser/ui/webui/chromeos/login/base_screen_handler.h b/chrome/browser/ui/webui/chromeos/login/base_screen_handler.h
@@ -5,10 +5,12 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_BASE_SCREEN_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_BASE_SCREEN_HANDLER_H_
 
+#include "base/values.h"
 #include "chrome/browser/ash/login/oobe_screen.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_webui_handler.h"
 // TODO(https://crbug.com/1164001): move to forward declaration
 #include "chrome/browser/ash/login/screens/base_screen.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace chromeos {
 
@@ -32,6 +34,10 @@ class BaseScreenHandler : public BaseWebUIHandler {
   void RegisterMessages() override;
 
  protected:
+  // Advances to the `oobe_screen_` in the WebUI. Optional `data` will be passed
+  // to the `onBeforeShow` on the javascript side.
+  void ShowInWebUI(absl::optional<base::Value::Dict> data = absl::nullopt);
+
   // Set the method identifier for a userActed callback. The actual callback
   // will be registered in RegisterMessages so this should be called in the
   // constructor. This takes the full method path, ie,
@@ -51,7 +57,7 @@ class BaseScreenHandler : public BaseWebUIHandler {
   std::string user_acted_method_path_;
 
   // OobeScreen that this handler corresponds to.
-  OobeScreenId oobe_screen_ = OobeScreen::SCREEN_UNKNOWN;
+  const OobeScreenId oobe_screen_;
 
   BaseScreen* base_screen_ = nullptr;
 };

diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "components/login/localized_values_builder.h"
 #include "content/public/browser/web_ui.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace chromeos {
 
@@ -36,18 +37,10 @@ void BaseWebUIHandler::RegisterMessages() {
 
 void BaseWebUIHandler::GetAdditionalParameters(base::DictionaryValue* dict) {}
 
-void BaseWebUIHandler::ShowScreen(OobeScreenId screen) {
-  ShowScreenWithData(screen, nullptr);
-}
-
-void BaseWebUIHandler::ShowScreenWithData(OobeScreenId screen,
-                                          const base::DictionaryValue* data) {
-  base::DictionaryValue screen_params;
-  screen_params.SetStringKey("id", screen.name);
-  if (data) {
-    screen_params.SetKey("data", data->Clone());
-  }
-  CallJS("cr.ui.Oobe.showScreen", std::move(screen_params));
+void BaseWebUIHandler::ShowScreenDeprecated(OobeScreenId screen) {
+  if (!GetOobeUI())
+    return;
+  GetOobeUI()->GetCoreOobeView()->ShowScreenWithData(screen, absl::nullopt);
 }
 
 OobeUI* BaseWebUIHandler::GetOobeUI() {

diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h
@@ -121,11 +121,7 @@ class BaseWebUIHandler : public content::WebUIMessageHandler {
   virtual void Initialize() = 0;
 
   // Show selected WebUI `screen`.
-  void ShowScreen(OobeScreenId screen);
-  // Show selected WebUI `screen`. Pass screen initialization using the `data`
-  // parameter.
-  void ShowScreenWithData(OobeScreenId screen,
-                          const base::DictionaryValue* data);
+  void ShowScreenDeprecated(OobeScreenId screen);
 
   // Returns the OobeUI instance.
   OobeUI* GetOobeUI();

diff --git a/chrome/browser/ui/webui/chromeos/login/consolidated_consent_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/consolidated_consent_screen_handler.cc
@@ -143,26 +143,25 @@ void ConsolidatedConsentScreenHandler::DeclareLocalizedValues(
 void ConsolidatedConsentScreenHandler::Initialize() {}
 
 void ConsolidatedConsentScreenHandler::Show(const ScreenConfig& config) {
-  base::DictionaryValue data;
+  base::Value::Dict data;
   // If ARC is enabled, show the ARC ToS and the related opt-ins.
-  data.SetBoolKey("isArcEnabled", config.is_arc_enabled);
+  data.Set("isArcEnabled", config.is_arc_enabled);
   // In demo mode, don't show any opt-ins related to ARC and allow showing the
   // offline ARC ToS if the online version failed to load.
-  data.SetBoolKey("isDemo", config.is_demo);
+  data.Set("isDemo", config.is_demo);
   // Child accounts have alternative strings for the opt-ins.
-  data.SetBoolKey("isChildAccount", config.is_child_account);
+  data.Set("isChildAccount", config.is_child_account);
   // Managed account will not be shown any terms of service, and the title
   // string will be updated.
-  data.SetBoolKey("isEnterpriseManagedAccount",
-                  config.is_enterprise_managed_account);
+  data.Set("isEnterpriseManagedAccount", config.is_enterprise_managed_account);
   // Country code is needed to load the ARC ToS.
-  data.SetStringKey("countryCode", config.country_code);
+  data.Set("countryCode", config.country_code);
   // URL for EULA, the URL should include the locale.
-  data.SetStringKey("googleEulaUrl", config.google_eula_url);
+  data.Set("googleEulaUrl", config.google_eula_url);
   // URL for Chrome and ChromeOS additional terms of service, the URL should
   // include the locale.
-  data.SetStringKey("crosEulaUrl", config.cros_eula_url);
-  ShowScreenWithData(kScreenId, &data);
+  data.Set("crosEulaUrl", config.cros_eula_url);
+  ShowInWebUI(std::move(data));
 }
 
 void ConsolidatedConsentScreenHandler::Bind(ConsolidatedConsentScreen* screen) {

diff --git a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
@@ -134,6 +134,17 @@ void CoreOobeHandler::FocusReturned(bool reverse) {
   CallJS("cr.ui.Oobe.focusReturned", reverse);
 }
 
+void CoreOobeHandler::ShowScreenWithData(
+    const ash::OobeScreenId& screen,
+    absl::optional<base::Value::Dict> data) {
+  base::Value::Dict screen_params;
+  screen_params.Set("id", screen.name);
+  if (data.has_value()) {
+    screen_params.Set("data", std::move(data.value()));
+  }
+  CallJS("cr.ui.Oobe.showScreen", std::move(screen_params));
+}
+
 void CoreOobeHandler::ReloadContent(base::DictionaryValue dictionary) {
   CallJS("cr.ui.Oobe.reloadContent", std::move(dictionary));
 }

diff --git a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h
@@ -15,6 +15,7 @@
 #include "base/values.h"
 #include "chrome/browser/ash/login/help_app_launcher.h"
 #include "chrome/browser/ash/login/oobe_configuration.h"
+#include "chrome/browser/ash/login/oobe_screen.h"
 #include "chrome/browser/ash/login/version_info_updater.h"
 #include "chrome/browser/ash/tpm_firmware_update.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_webui_handler.h"
@@ -32,6 +33,8 @@ class CoreOobeView {
  public:
   virtual ~CoreOobeView() = default;
 
+  virtual void ShowScreenWithData(const ash::OobeScreenId& screen,
+                                  absl::optional<base::Value::Dict> data) = 0;
   virtual void ReloadContent(base::DictionaryValue dictionary) = 0;
   virtual void SetVirtualKeyboardShown(bool shown) = 0;
   virtual void SetShelfHeight(int height) = 0;
@@ -95,6 +98,8 @@ class CoreOobeHandler : public BaseWebUIHandler,
 
  private:
   // CoreOobeView implementation:
+  void ShowScreenWithData(const ash::OobeScreenId& screen,
+                          absl::optional<base::Value::Dict> data) override;
   void ReloadContent(base::DictionaryValue dictionary) override;
   void SetVirtualKeyboardShown(bool displayed) override;
   void SetShelfHeight(int height) override;

diff --git a/chrome/browser/ui/webui/chromeos/login/demo_preferences_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/demo_preferences_screen_handler.cc
@@ -27,7 +27,7 @@ DemoPreferencesScreenHandler::~DemoPreferencesScreenHandler() {
 }
 
 void DemoPreferencesScreenHandler::Show() {
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void DemoPreferencesScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/demo_setup_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/demo_setup_screen_handler.cc
@@ -31,7 +31,7 @@ DemoSetupScreenHandler::~DemoSetupScreenHandler() {
 }
 
 void DemoSetupScreenHandler::Show() {
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void DemoSetupScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/device_disabled_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/device_disabled_screen_handler.cc
@@ -27,11 +27,11 @@ DeviceDisabledScreenHandler::~DeviceDisabledScreenHandler() {
 void DeviceDisabledScreenHandler::Show(const std::string& serial,
                                        const std::string& domain,
                                        const std::string& message) {
-  base::DictionaryValue screen_data;
-  screen_data.SetStringPath("serial", serial);
-  screen_data.SetStringPath("domain", domain);
-  screen_data.SetStringPath("message", message);
-  ShowScreenWithData(kScreenId, &screen_data);
+  base::Value::Dict screen_data;
+  screen_data.Set("serial", serial);
+  screen_data.Set("domain", domain);
+  screen_data.Set("message", message);
+  ShowInWebUI(std::move(screen_data));
 }
 
 void DeviceDisabledScreenHandler::Hide() {

diff --git a/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.cc
@@ -34,7 +34,7 @@ void EnableAdbSideloadingScreenHandler::Show() {
     show_on_init_ = true;
     return;
   }
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void EnableAdbSideloadingScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/enable_debugging_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/enable_debugging_screen_handler.cc
@@ -39,7 +39,7 @@ void EnableDebuggingScreenHandler::Show() {
   }
 
   DVLOG(1) << "Showing enable debugging screen.";
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void EnableDebuggingScreenHandler::Hide() {}

diff --git a/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/encryption_migration_screen_handler.cc
@@ -31,7 +31,7 @@ void EncryptionMigrationScreenHandler::Show() {
     show_on_init_ = true;
     return;
   }
-  ShowScreen(kScreenId);
+  ShowInWebUI();
 }
 
 void EncryptionMigrationScreenHandler::Hide() {