Replace ash::Shell display_manager calls in core_oobe_handler.cc

This is used by getPrimaryDisplayNameForTesting which is called from
the CrOS autotest enterprise_RemoraRequisitionDisplayUsage.py.

Includes a TODO to update the autotest to call the equivalent
extension API.

If this breaks enterprise_RemoraRequisitionDisplayUsage, please revert.

Bug: 855251
Change-Id: I73007ba2a4d4a20c46da940f59fe98353cabf294
Reviewed-on: https://chromium-review.googlesource.com/1114295
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571923}

chrome/browser/ui/ash/ash_util.cc

@@ -10,6 +10,7 @@
 #include "ash/public/interfaces/event_properties.mojom.h"
 #include "ash/shell.h"
 #include "base/macros.h"
+#include "content/public/common/service_manager_connection.h"
 #include "mojo/public/cpp/bindings/type_converter.h"
 #include "services/ui/public/cpp/property_type_converters.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
@@ -56,4 +57,12 @@ void SetupWidgetInitParamsForContainer(views::Widget::InitParams* params,
   }
 }
 
+service_manager::Connector* GetServiceManagerConnector() {
+  content::ServiceManagerConnection* manager_connection =
+      content::ServiceManagerConnection::GetForProcess();
+  if (!manager_connection)
+    return nullptr;
+  return manager_connection->GetConnector();
+}
+
 }  // namespace ash_util

chrome/browser/ui/ash/ash_util.h

@@ -9,6 +9,10 @@
 
 #include "ui/views/widget/widget.h"
 
+namespace service_manager {
+class Connector;
+}
+
 namespace ui {
 class Accelerator;
 class KeyEvent;
@@ -33,6 +37,10 @@ bool WillAshProcessAcceleratorForEvent(const ui::KeyEvent& key_event);
 void SetupWidgetInitParamsForContainer(views::Widget::InitParams* params,
                                        int container_id);
 
+// Returns the connector from ServiceManagerConnection::GetForProcess().
+// May be null in unit tests.
+service_manager::Connector* GetServiceManagerConnector();
+
 }  // namespace ash_util
 
 #endif  // CHROME_BROWSER_UI_ASH_ASH_UTIL_H_

chrome/browser/ui/webui/chromeos/login/DEPS

 specific_include_rules = {
-  # TODO(mash): Fix. https://crbug.com/770866
+  # TODO(mash): Fix. https://crbug.com/856375.
   "core_oobe_handler\.cc": [
     "+ash/shell.h",
   ],
-  # TODO(mash): Fix. https://crbug.com/678990
+  # TODO(mash): Fix or deprecate. https://crbug.com/678990
   "signin_screen_handler\.cc": [
     "+ash/detachable_base",
     "+ash/shell.h",

chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc

@@ -27,6 +27,7 @@
 #include "chrome/browser/chromeos/system/timezone_resolver_manager.h"
 #include "chrome/browser/chromeos/tpm_firmware_update.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/tablet_mode_client.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
@@ -47,8 +48,6 @@
 #include "services/service_manager/public/cpp/connector.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/ui_base_features.h"
-#include "ui/display/display.h"
-#include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
 #include "ui/events/event_sink.h"
 #include "ui/gfx/geometry/size.h"
@@ -89,7 +88,8 @@ CoreOobeHandler::CoreOobeHandler(OobeUI* oobe_ui,
                                  JSCallsContainer* js_calls_container)
     : BaseWebUIHandler(js_calls_container),
       oobe_ui_(oobe_ui),
-      version_info_updater_(this) {
+      version_info_updater_(this),
+      weak_ptr_factory_(this) {
   DCHECK(js_calls_container);
   set_call_js_prefix(kJsScreenPath);
   if (features::IsAshInBrowserProcess()) {
@@ -106,6 +106,13 @@ CoreOobeHandler::CoreOobeHandler(OobeUI* oobe_ui,
   tablet_mode_client->AddObserver(this);
 
   OobeConfiguration::Get()->AddObserver(this);
+
+  // |connector| may be null in tests.
+  auto* connector = ash_util::GetServiceManagerConnector();
+  if (connector) {
+    connector->BindInterface(ash::mojom::kServiceName,
+                             &cros_display_config_ptr_);
+  }
 }
 
 CoreOobeHandler::~CoreOobeHandler() {
@@ -207,6 +214,9 @@ void CoreOobeHandler::RegisterMessages() {
   AddCallback("raiseTabKeyEvent", &CoreOobeHandler::HandleRaiseTabKeyEvent);
   AddCallback("setOobeBootstrappingSlave",
               &CoreOobeHandler::HandleSetOobeBootstrappingSlave);
+  // 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("setupDemoMode", &CoreOobeHandler::HandleSetupDemoMode);
@@ -614,15 +624,25 @@ void CoreOobeHandler::HandleGetPrimaryDisplayNameForTesting(
   const base::Value* callback_id;
   CHECK(args->Get(0, &callback_id));
 
-  const auto primary_display_id =
-      display::Screen::GetScreen()->GetPrimaryDisplay().id();
-  const display::DisplayManager* display_manager =
-      ash::Shell::Get()->display_manager();
-  const std::string display_name =
-      display_manager->GetDisplayNameForId(primary_display_id);
+  cros_display_config_ptr_->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();
-  ResolveJavascriptCallback(*callback_id, base::Value(display_name));
+  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::HandleSetupDemoMode() {

chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h

@@ -9,8 +9,10 @@
 #include <string>
 #include <vector>
 
+#include "ash/public/interfaces/cros_display_config.mojom.h"
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_mode_detector.h"
@@ -23,6 +25,7 @@
 
 namespace base {
 class ListValue;
+class Value;
 }
 
 namespace ui {
@@ -140,6 +143,9 @@ class CoreOobeHandler : public BaseWebUIHandler,
   void HandleHeaderBarVisible();
   void HandleSetOobeBootstrappingSlave();
   void HandleGetPrimaryDisplayNameForTesting(const base::ListValue* args);
+  void GetPrimaryDisplayNameCallback(
+      const base::Value& callback_id,
+      std::vector<ash::mojom::DisplayUnitInfoPtr> info_list);
   void HandleSetupDemoMode();
 
   // When keyboard_utils.js arrow key down event is reached, raise it
@@ -184,6 +190,10 @@ class CoreOobeHandler : public BaseWebUIHandler,
 
   DemoModeDetector demo_mode_detector_;
 
+  ash::mojom::CrosDisplayConfigControllerPtr cros_display_config_ptr_;
+
+  base::WeakPtrFactory<CoreOobeHandler> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(CoreOobeHandler);
 };
 

chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc

@@ -8,6 +8,7 @@
 
 #include "ash/public/interfaces/constants.mojom.h"
 #include "base/strings/string_number_conversions.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/service_manager_connection.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -39,11 +40,8 @@ bool IsWhiteListedVendorId(uint16_t vendor_id) {
 
 OobeDisplayChooser::OobeDisplayChooser()
     : scoped_observer_(this), weak_ptr_factory_(this) {
-  // |manager_connection| or |connector| may be null in tests.
-  content::ServiceManagerConnection* manager_connection =
-      content::ServiceManagerConnection::GetForProcess();
-  service_manager::Connector* connector =
-      manager_connection ? manager_connection->GetConnector() : nullptr;
+  // |connector| may be null in tests.
+  auto* connector = ash_util::GetServiceManagerConnector();
   if (connector) {
     connector->BindInterface(ash::mojom::kServiceName,
                              &cros_display_config_ptr_);