[Oobe WebUI] Migrate off BaseWebUIHandler::AddRawCallback

Migrated to BaseWebUIHandler::AddCallback. Also moved getPrimaryDisplayNameForTesting into OobeTestAPIHandler, with renaming to OobeTestApi.getPrimaryDisplayName Bug: 1298392, 858958, 1100910, 1309022 Change-Id: I0fe9ac5a2927a67d6a81fd8ed783241e64cc243c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3535785 Reviewed-by: Danila Kuzmin <dkuzmin@google.com> Commit-Queue: Roman Sorokin <rsorokin@chromium.org> Auto-Submit: Roman Sorokin <rsorokin@chromium.org> Cr-Commit-Position: refs/heads/main@{#984824}
chromium · Mar 24, 2022 · 9d88b18 · 9d88b18
1 parent c85dd91
commit 9d88b18
Show file tree

Hide file tree

Showing 15 changed files with 79 additions and 107 deletions.
diff --git a/chrome/browser/ash/login/screens/recommend_apps_screen_browsertest.cc b/chrome/browser/ash/login/screens/recommend_apps_screen_browsertest.cc
@@ -489,7 +489,7 @@ IN_PROC_BROWSER_TEST_F(RecommendAppsScreenTest, InstallWithNoAppsSelected) {
   // The install button is expected to be disabled at this point. Send empty app
   // list directly to test handler behavior when install is triggered with no
   // apps selected.
-  test::OobeJS().Evaluate("chrome.send('recommendAppsInstall', []);");
+  test::OobeJS().Evaluate("chrome.send('recommendAppsInstall', [[]]);");
 
   WaitForScreenExit();
   EXPECT_EQ(RecommendAppsScreen::Result::SKIPPED, screen_result_.value());

diff --git a/chrome/browser/resources/chromeos/login/cr_ui.js b/chrome/browser/resources/chromeos/login/cr_ui.js
@@ -272,17 +272,6 @@ cr.define('cr.ui', function() {
       DemoModeTestHelper.setUp('online');
     }
 
-    /**
-     * Get the primary display's name.
-     *
-     * Same as the displayInfo.name parameter returned by
-     * chrome.system.display.getInfo(), but unlike chrome.system it's available
-     * during OOBE.
-     */
-    static getPrimaryDisplayNameForTesting() {
-      return cr.sendWithPromise('getPrimaryDisplayNameForTesting');
-    }
-
     /**
      * Click on the primary action button ("Next" usually) for Gaia. On the
      * Login or Enterprise Enrollment screen.

diff --git a/chrome/browser/resources/chromeos/login/screens/common/recommend_apps.js b/chrome/browser/resources/chromeos/login/screens/common/recommend_apps.js
@@ -174,7 +174,7 @@ class RecommendAppsElement extends RecommendAppsElementBase {
       let appListView = this.$.appView;
       appListView.executeScript(
           {code: 'getSelectedPackages();'}, function(result) {
-            chrome.send('recommendAppsInstall', result[0]);
+            chrome.send('recommendAppsInstall', [result[0]]);
           });
     }
   }
@@ -204,4 +204,4 @@ class RecommendAppsElement extends RecommendAppsElementBase {
   }
 }
 
-customElements.define(RecommendAppsElement.is, RecommendAppsElement);
+customElements.define(RecommendAppsElement.is, RecommendAppsElement);
diff --git a/chrome/browser/resources/chromeos/login/screens/common/recommend_apps_old.js b/chrome/browser/resources/chromeos/login/screens/common/recommend_apps_old.js
@@ -178,7 +178,7 @@ class RecommendAppsOldElement extends RecommendAppsOldElementBase {
       let appListView = this.$.appView;
       appListView.executeScript(
           {code: 'getSelectedPackages();'}, function(result) {
-            chrome.send('recommendAppsInstall', result[0]);
+            chrome.send('recommendAppsInstall', [result[0]]);
           });
     }
   }
@@ -208,4 +208,4 @@ class RecommendAppsOldElement extends RecommendAppsOldElementBase {
   }
 }
 
-customElements.define(RecommendAppsOldElement.is, RecommendAppsOldElement);
+customElements.define(RecommendAppsOldElement.is, RecommendAppsOldElement);
diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
@@ -65,11 +65,4 @@ void BaseWebUIHandler::OnJavascriptDisallowed() {
   javascript_disallowed_ = true;
 }
 
-void BaseWebUIHandler::OnRawCallback(
-    const std::string& function_name,
-    const content::WebUI::DeprecatedMessageCallback& callback,
-    const base::ListValue* args) {
-  callback.Run(args);
-}
-
 }  // namespace chromeos
diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h
@@ -110,18 +110,6 @@ class BaseWebUIHandler : public content::WebUIMessageHandler {
            IsJavascriptAllowed();
   }
 
-  // Register WebUI callbacks. The callbacks will be recorded if recording is
-  // enabled.
-  template <typename T>
-  void AddRawCallback(const std::string& function_name,
-                      void (T::*method)(const base::ListValue* args)) {
-    content::WebUI::DeprecatedMessageCallback callback =
-        base::BindRepeating(method, base::Unretained(static_cast<T*>(this)));
-    web_ui()->RegisterDeprecatedMessageCallback(
-        function_name,
-        base::BindRepeating(&BaseWebUIHandler::OnRawCallback,
-                            base::Unretained(this), function_name, callback));
-  }
   template <typename T, typename... Args>
   void AddCallback(const std::string& function_name,
                    void (T::*method)(Args...)) {
@@ -158,11 +146,6 @@ class BaseWebUIHandler : public content::WebUIMessageHandler {
  private:
   friend class OobeUI;
 
-  // These two functions wrap Add(Raw)Callback so that the incoming JavaScript
-  // event can be recorded.
-  void OnRawCallback(const std::string& function_name,
-                     const content::WebUI::DeprecatedMessageCallback& callback,
-                     const base::ListValue* args);
   template <typename... Args>
   void OnCallback(const std::string& function_name,
                   const base::RepeatingCallback<void(Args...)>& callback,

diff --git a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
@@ -7,7 +7,6 @@
 #include <type_traits>
 
 #include "ash/constants/ash_features.h"
-#include "ash/public/ash_interfaces.h"
 #include "ash/public/cpp/event_rewriter_controller.h"
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/public/cpp/tablet_mode.h"
@@ -63,8 +62,6 @@ CoreOobeHandler::CoreOobeHandler(JSCallsContainer* js_calls_container)
 
   ash::TabletMode::Get()->AddObserver(this);
 
-  ash::BindCrosDisplayConfigController(
-      cros_display_config_.BindNewPipeAndPassReceiver());
   OobeConfiguration::Get()->AddAndFireObserver(this);
 }
 
@@ -126,11 +123,6 @@ void CoreOobeHandler::RegisterMessages() {
   AddCallback("launchHelpApp", &CoreOobeHandler::HandleLaunchHelpApp);
   AddCallback("toggleResetScreen", &CoreOobeHandler::HandleToggleResetScreen);
   AddCallback("raiseTabKeyEvent", &CoreOobeHandler::HandleRaiseTabKeyEvent);
-  // Note: Used by enterprise_RemoraRequisitionDisplayUsage.py:
-  // TODO(felixe): Use chrome.system.display or cros_display_config.mojom,
-  // https://crbug.com/858958.
-  AddRawCallback("getPrimaryDisplayNameForTesting",
-                 &CoreOobeHandler::HandleGetPrimaryDisplayNameForTesting);
   AddCallback("startDemoModeSetupForTesting",
               &CoreOobeHandler::HandleStartDemoModeSetupForTesting);
 
@@ -340,32 +332,6 @@ void CoreOobeHandler::HandleRaiseTabKeyEvent(bool reverse) {
   SendEventToSink(&event);
 }
 
-void CoreOobeHandler::HandleGetPrimaryDisplayNameForTesting(
-    const base::ListValue* args) {
-  CHECK_EQ(1U, args->GetListDeprecated().size());
-  const base::Value& callback_id = args->GetListDeprecated()[0];
-
-  cros_display_config_->GetDisplayUnitInfoList(
-      false /* single_unified */,
-      base::BindOnce(&CoreOobeHandler::GetPrimaryDisplayNameCallback,
-                     weak_ptr_factory_.GetWeakPtr(), callback_id.Clone()));
-}
-
-void CoreOobeHandler::GetPrimaryDisplayNameCallback(
-    const base::Value& callback_id,
-    std::vector<ash::mojom::DisplayUnitInfoPtr> info_list) {
-  AllowJavascript();
-  std::string display_name;
-  for (const ash::mojom::DisplayUnitInfoPtr& info : info_list) {
-    if (info->is_primary) {
-      display_name = info->name;
-      break;
-    }
-  }
-  DCHECK(!display_name.empty());
-  ResolveJavascriptCallback(callback_id, base::Value(display_name));
-}
-
 void CoreOobeHandler::HandleStartDemoModeSetupForTesting(
     const std::string& demo_config) {
   DemoSession::DemoModeConfig config;

diff --git a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h
@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "ash/public/cpp/tablet_mode_observer.h"
-#include "ash/public/mojom/cros_display_config.mojom.h"
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
@@ -23,11 +22,6 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/events/event_source.h"
 
-namespace base {
-class ListValue;
-class Value;
-}
-
 namespace ui {
 class EventSink;
 }
@@ -126,10 +120,6 @@ class CoreOobeHandler : public BaseWebUIHandler,
   void HandleSkipToLoginForTesting();
   void HandleLaunchHelpApp(double help_topic_id);
   void HandleToggleResetScreen();
-  void HandleGetPrimaryDisplayNameForTesting(const base::ListValue* args);
-  void GetPrimaryDisplayNameCallback(
-      const base::Value& callback_id,
-      std::vector<ash::mojom::DisplayUnitInfoPtr> info_list);
   // Handles demo mode setup for tests. Accepts 'online' and 'offline' as
   // `demo_config`.
   void HandleStartDemoModeSetupForTesting(const std::string& demo_config);
@@ -160,8 +150,6 @@ class CoreOobeHandler : public BaseWebUIHandler,
   // Help application used for help dialogs.
   scoped_refptr<HelpAppLauncher> help_app_;
 
-  mojo::Remote<ash::mojom::CrosDisplayConfigController> cros_display_config_;
-
   base::WeakPtrFactory<CoreOobeHandler> weak_ptr_factory_{this};
 };
 

diff --git a/chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc b/chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc
@@ -41,10 +41,10 @@ void NetworkDropdownHandler::RegisterMessages() {
               &NetworkDropdownHandler::HandleLaunchInternetDetailDialog);
   AddCallback(kJsApiLaunchAddWiFiNetworkDialog,
               &NetworkDropdownHandler::HandleLaunchAddWiFiNetworkDialog);
-  AddRawCallback(kJsApiShowNetworkDetails,
-                 &NetworkDropdownHandler::HandleShowNetworkDetails);
-  AddRawCallback(kJsApiShowNetworkConfig,
-                 &NetworkDropdownHandler::HandleShowNetworkConfig);
+  AddCallback(kJsApiShowNetworkDetails,
+              &NetworkDropdownHandler::HandleShowNetworkDetails);
+  AddCallback(kJsApiShowNetworkConfig,
+              &NetworkDropdownHandler::HandleShowNetworkConfig);
 }
 
 void NetworkDropdownHandler::HandleLaunchInternetDetailDialog() {
@@ -66,11 +66,8 @@ void NetworkDropdownHandler::HandleLaunchAddWiFiNetworkDialog() {
       LoginDisplayHost::default_host()->GetNativeWindow());
 }
 
-void NetworkDropdownHandler::HandleShowNetworkDetails(
-    const base::ListValue* args) {
-  DCHECK_GE(args->GetListDeprecated().size(), 2U);
-  std::string type = args->GetListDeprecated()[0].GetString();
-  std::string guid = args->GetListDeprecated()[1].GetString();
+void NetworkDropdownHandler::HandleShowNetworkDetails(const std::string& type,
+                                                      const std::string& guid) {
   if (type == ::onc::network_type::kCellular) {
     // Make sure Cellular is enabled.
     NetworkStateHandler* handler =
@@ -85,10 +82,7 @@ void NetworkDropdownHandler::HandleShowNetworkDetails(
       guid, LoginDisplayHost::default_host()->GetNativeWindow());
 }
 
-void NetworkDropdownHandler::HandleShowNetworkConfig(
-    const base::ListValue* args) {
-  DCHECK(!args->GetListDeprecated().empty());
-  std::string guid = args->GetListDeprecated()[0].GetString();
+void NetworkDropdownHandler::HandleShowNetworkConfig(const std::string& guid) {
   chromeos::InternetConfigDialog::ShowDialogForNetworkId(
       guid, LoginDisplayHost::default_host()->GetNativeWindow());
 }

diff --git a/chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.h b/chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.h
@@ -30,8 +30,9 @@ class NetworkDropdownHandler : public BaseWebUIHandler {
  private:
   void HandleLaunchInternetDetailDialog();
   void HandleLaunchAddWiFiNetworkDialog();
-  void HandleShowNetworkDetails(const base::ListValue* args);
-  void HandleShowNetworkConfig(const base::ListValue* args);
+  void HandleShowNetworkDetails(const std::string& type,
+                                const std::string& guid);
+  void HandleShowNetworkConfig(const std::string& guid);
 };
 
 }  // namespace chromeos

diff --git a/chrome/browser/ui/webui/chromeos/login/recommend_apps_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/recommend_apps_screen_handler.cc
@@ -7,6 +7,7 @@
 #include "ash/components/arc/arc_prefs.h"
 #include "ash/constants/ash_features.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/values.h"
 #include "chrome/browser/ash/arc/session/arc_session_manager.h"
 #include "chrome/browser/ash/login/screens/recommend_apps_screen.h"
 #include "chrome/browser/profiles/profile.h"
@@ -98,8 +99,7 @@ void RecommendAppsScreenHandler::DeclareLocalizedValues(
 void RecommendAppsScreenHandler::RegisterMessages() {
   BaseScreenHandler::RegisterMessages();
   AddCallback(kUserActionSkip, &RecommendAppsScreenHandler::OnUserSkip);
-  AddRawCallback(kUserActionInstall,
-                 &RecommendAppsScreenHandler::HandleInstall);
+  AddCallback(kUserActionInstall, &RecommendAppsScreenHandler::HandleInstall);
 }
 
 void RecommendAppsScreenHandler::Bind(RecommendAppsScreen* screen) {
@@ -167,9 +167,11 @@ void RecommendAppsScreenHandler::HandleSkip() {
     screen_->OnSkip();
 }
 
-void RecommendAppsScreenHandler::HandleInstall(const base::ListValue* args) {
+void RecommendAppsScreenHandler::HandleInstall(
+    const base::ListValue* apps_arg) {
+  base::Value::List apps = apps_arg->GetList().Clone();
   if (recommended_app_count_ != 0) {
-    int selected_app_count = static_cast<int>(args->GetListDeprecated().size());
+    int selected_app_count = static_cast<int>(apps.size());
     int selected_recommended_percentage =
         100 * selected_app_count / recommended_app_count_;
     RecordUmaUserSelectionAppCount(selected_app_count);
@@ -178,14 +180,15 @@ void RecommendAppsScreenHandler::HandleInstall(const base::ListValue* args) {
 
   // If the user does not select any apps, we should skip the app downloading
   // screen.
-  if (args->GetListDeprecated().empty()) {
+  if (apps.empty()) {
     RecordUmaScreenAction(RecommendAppsScreenAction::SELECTED_NONE);
     HandleSkip();
     return;
   }
 
   RecordUmaScreenAction(RecommendAppsScreenAction::APP_SELECTED);
-  pref_service_->Set(arc::prefs::kArcFastAppReinstallPackages, *args);
+  pref_service_->Set(arc::prefs::kArcFastAppReinstallPackages,
+                     base::Value(std::move(apps)));
 
   arc::ArcFastAppReinstallStarter* fast_app_reinstall_starter =
       arc::ArcSessionManager::Get()->fast_app_resintall_starter();

diff --git a/chrome/browser/ui/webui/chromeos/login/testapi/oobe_test_api_handler.cc b/chrome/browser/ui/webui/chromeos/login/testapi/oobe_test_api_handler.cc
@@ -6,9 +6,12 @@
 
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_switches.h"
+#include "ash/public/ash_interfaces.h"
+#include "ash/public/mojom/cros_display_config.mojom.h"
 #include "base/bind.h"
 #include "base/check.h"
 #include "base/logging.h"
+#include "base/values.h"
 #include "build/branding_buildflags.h"
 #include "chrome/browser/ash/login/existing_user_controller.h"
 #include "chrome/browser/ash/login/helper.h"
@@ -42,6 +45,12 @@ void OobeTestAPIHandler::DeclareJSCallbacks() {
   AddCallback("OobeTestApi.loginAsGuest", &OobeTestAPIHandler::LoginAsGuest);
   AddCallback("OobeTestApi.showGaiaDialog",
               &OobeTestAPIHandler::ShowGaiaDialog);
+
+  // Keeping the code in case the test using this will be ported to tast. The
+  // function used to be called getPrimaryDisplayNameForTesting. In order to use
+  // this one you need to add a function into login/test_api/test_api.js.
+  AddCallback("OobeTestApi.getPrimaryDisplayName",
+              &OobeTestAPIHandler::HandleGetPrimaryDisplayName);
 }
 
 void OobeTestAPIHandler::Initialize() {}
@@ -95,5 +104,35 @@ void OobeTestAPIHandler::LoginAsGuest() {
 void OobeTestAPIHandler::ShowGaiaDialog() {
   LoginDisplayHost::default_host()->ShowGaiaDialog(EmptyAccountId());
 }
+void OobeTestAPIHandler::HandleGetPrimaryDisplayName(
+    const std::string& callback_id) {
+  mojo::Remote<ash::mojom::CrosDisplayConfigController> cros_display_config;
+  ash::BindCrosDisplayConfigController(
+      cros_display_config.BindNewPipeAndPassReceiver());
+
+  cros_display_config->GetDisplayUnitInfoList(
+      false /* single_unified */,
+      base::BindOnce(&OobeTestAPIHandler::OnGetDisplayUnitInfoList,
+                     base::Unretained(this), callback_id));
+}
+
+void OobeTestAPIHandler::OnGetDisplayUnitInfoList(
+    const std::string& callback_id,
+    std::vector<ash::mojom::DisplayUnitInfoPtr> info_list) {
+  std::string display_name;
+  for (const ash::mojom::DisplayUnitInfoPtr& info : info_list) {
+    if (info->is_primary) {
+      display_name = info->name;
+      break;
+    }
+  }
+  if (display_name.empty()) {
+    RejectJavascriptCallback(base::Value(callback_id),
+                             base::Value(display_name));
+    return;
+  }
+  ResolveJavascriptCallback(base::Value(callback_id),
+                            base::Value(display_name));
+}
 
 }  // namespace chromeos
diff --git a/chrome/browser/ui/webui/chromeos/login/testapi/oobe_test_api_handler.h b/chrome/browser/ui/webui/chromeos/login/testapi/oobe_test_api_handler.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_TESTAPI_OOBE_TEST_API_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_TESTAPI_OOBE_TEST_API_HANDLER_H_
 
+#include "ash/public/mojom/cros_display_config.mojom.h"
 #include "base/values.h"
 #include "chrome/browser/ui/webui/chromeos/login/base_webui_handler.h"
 
@@ -30,6 +31,10 @@ class OobeTestAPIHandler : public BaseWebUIHandler {
   void SkipPostLoginScreens();
   void LoginAsGuest();
   void ShowGaiaDialog();
+  void HandleGetPrimaryDisplayName(const std::string& callback_id);
+  void OnGetDisplayUnitInfoList(
+      const std::string& callback_id,
+      std::vector<ash::mojom::DisplayUnitInfoPtr> info_list);
 };
 
 }  // namespace chromeos