Use cros_display_config.mojom in OobeDisplayChooser

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

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

@@ -3,10 +3,6 @@ specific_include_rules = {
   "core_oobe_handler\.cc": [
     "+ash/shell.h",
   ],
-  "oobe_display_chooser\.cc": [
-    "+ash/display/window_tree_host_manager.h",
-    "+ash/shell.h",
-  ],
   # TODO(mash): Fix. https://crbug.com/678990
   "signin_screen_handler\.cc": [
     "+ash/detachable_base",
@@ -21,11 +17,11 @@ specific_include_rules = {
     "+ash/shell.h",
   ],
   "oobe_display_chooser_unittest.cc": [
+    # TODO(mash) Use a non-ash test ui::InputDeviceEventObserver
+    # environment for testing this.
     "+ash/display/display_configuration_controller.h",
     "+ash/shell.h",
     "+ash/test/ash_test_base.h",
-    # TODO(mash): Remove. http://crbug.com/720917.
-    "+ui/events/devices/device_data_manager.h",
   ],
   "signin_userlist_unittest\.cc": [
     "+ash/test/ash_test_base.h"

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

@@ -6,10 +6,11 @@
 
 #include <stdint.h>
 
-#include "ash/display/window_tree_host_manager.h"
-#include "ash/shell.h"
-#include "base/stl_util.h"
+#include "ash/public/interfaces/constants.mojom.h"
+#include "base/strings/string_number_conversions.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/devices/input_device_manager.h"
@@ -37,7 +38,17 @@ bool IsWhiteListedVendorId(uint16_t vendor_id) {
 }  // namespace
 
 OobeDisplayChooser::OobeDisplayChooser()
-    : scoped_observer_(this), weak_ptr_factory_(this) {}
+    : 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;
+  if (connector) {
+    connector->BindInterface(ash::mojom::kServiceName,
+                             &cros_display_config_ptr_);
+  }
+}
 
 OobeDisplayChooser::~OobeDisplayChooser() {}
 
@@ -82,8 +93,11 @@ void OobeDisplayChooser::MoveToTouchDisplay() {
        input_device_manager->GetTouchscreenDevices()) {
     if (IsWhiteListedVendorId(device.vendor_id) &&
         device.target_display_id != display::kInvalidDisplayId) {
-      ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(
-          device.target_display_id);
+      auto config_properties = ash::mojom::DisplayConfigProperties::New();
+      config_properties->set_primary = true;
+      cros_display_config_ptr_->SetDisplayProperties(
+          base::Int64ToString(device.target_display_id),
+          std::move(config_properties), base::DoNothing());
       break;
     }
   }

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

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_OOBE_DISPLAY_CHOOSER_H_
 #define CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_OOBE_DISPLAY_CHOOSER_H_
 
+#include "ash/public/interfaces/cros_display_config.mojom.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
@@ -25,6 +26,11 @@ class OobeDisplayChooser : public ui::InputDeviceEventObserver {
   // Must be called on the BrowserThread::UI thread.
   void TryToPlaceUiOnTouchDisplay();
 
+  void set_cros_display_config_ptr_for_test(
+      ash::mojom::CrosDisplayConfigControllerPtr cros_display_config_ptr) {
+    cros_display_config_ptr_ = std::move(cros_display_config_ptr);
+  }
+
  private:
   // Calls MoveToTouchDisplay() if touch device list is ready, otherwise adds an
   // observer that calls MoveToTouchDisplay() once ready.
@@ -39,6 +45,7 @@ class OobeDisplayChooser : public ui::InputDeviceEventObserver {
 
   ScopedObserver<ui::InputDeviceManager, ui::InputDeviceEventObserver>
       scoped_observer_;
+  ash::mojom::CrosDisplayConfigControllerPtr cros_display_config_ptr_;
 
   base::WeakPtrFactory<OobeDisplayChooser> weak_ptr_factory_;
 

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

@@ -11,6 +11,8 @@
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "services/ui/public/cpp/input_devices/input_device_client_test_api.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display.h"
@@ -25,6 +27,52 @@ namespace chromeos {
 
 namespace {
 
+class TestCrosDisplayConfig : public ash::mojom::CrosDisplayConfigController {
+ public:
+  TestCrosDisplayConfig() : binding_(this) {}
+
+  ash::mojom::CrosDisplayConfigControllerPtr CreateInterfacePtrAndBind() {
+    ash::mojom::CrosDisplayConfigControllerPtr ptr;
+    binding_.Bind(mojo::MakeRequest(&ptr));
+    return ptr;
+  }
+
+  // ash::mojom::CrosDisplayConfigController:
+  void AddObserver(ash::mojom::CrosDisplayConfigObserverAssociatedPtrInfo
+                       observer) override {}
+  void GetDisplayLayoutInfo(GetDisplayLayoutInfoCallback callback) override {}
+  void SetDisplayLayoutInfo(ash::mojom::DisplayLayoutInfoPtr info,
+                            SetDisplayLayoutInfoCallback callback) override {}
+  void GetDisplayUnitInfoList(
+      bool single_unified,
+      GetDisplayUnitInfoListCallback callback) override {}
+  void SetDisplayProperties(const std::string& id,
+                            ash::mojom::DisplayConfigPropertiesPtr properties,
+                            SetDisplayPropertiesCallback callback) override {
+    if (properties->set_primary) {
+      int64_t display_id;
+      base::StringToInt64(id, &display_id);
+      ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(
+          display_id);
+    }
+    std::move(callback).Run(ash::mojom::DisplayConfigResult::kSuccess);
+  }
+  void SetUnifiedDesktopEnabled(bool enabled) override {}
+  void OverscanCalibration(const std::string& display_id,
+                           ash::mojom::DisplayConfigOperation op,
+                           const base::Optional<gfx::Insets>& delta,
+                           OverscanCalibrationCallback callback) override {}
+  void TouchCalibration(const std::string& display_id,
+                        ash::mojom::DisplayConfigOperation op,
+                        ash::mojom::TouchCalibrationPtr calibration,
+                        TouchCalibrationCallback callback) override {}
+
+ private:
+  mojo::Binding<ash::mojom::CrosDisplayConfigController> binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestCrosDisplayConfig);
+};
+
 class OobeDisplayChooserTest : public ash::AshTestBase {
  public:
   OobeDisplayChooserTest() : ash::AshTestBase() {}
@@ -36,10 +84,21 @@ class OobeDisplayChooserTest : public ash::AshTestBase {
   // ash::AshTestBase:
   void SetUp() override {
     ash::AshTestBase::SetUp();
+
+    cros_display_config_ = std::make_unique<TestCrosDisplayConfig>();
+    display_chooser_ = std::make_unique<OobeDisplayChooser>();
+    display_chooser_->set_cros_display_config_ptr_for_test(
+        cros_display_config_->CreateInterfacePtrAndBind());
+
     ui::InputDeviceClientTestApi().OnDeviceListsComplete();
   }
 
+  OobeDisplayChooser* display_chooser() { return display_chooser_.get(); }
+
  private:
+  std::unique_ptr<TestCrosDisplayConfig> cros_display_config_;
+  std::unique_ptr<OobeDisplayChooser> display_chooser_;
+
   DISALLOW_COPY_AND_ASSIGN(OobeDisplayChooserTest);
 };
 
@@ -83,9 +142,8 @@ TEST_F(OobeDisplayChooserTest, PreferTouchAsPrimary) {
   touchscreen.target_display_id = display_info[1].id();
   ui::InputDeviceClientTestApi().SetTouchscreenDevices({touchscreen}, true);
 
-  OobeDisplayChooser display_chooser;
   EXPECT_EQ(1, GetPrimaryDisplay());
-  display_chooser.TryToPlaceUiOnTouchDisplay();
+  display_chooser()->TryToPlaceUiOnTouchDisplay();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(2, GetPrimaryDisplay());
 }
@@ -119,9 +177,8 @@ TEST_F(OobeDisplayChooserTest, DontSwitchFromTouch) {
   display_manager()->OnNativeDisplaysChanged(display_info);
   base::RunLoop().RunUntilIdle();
 
-  OobeDisplayChooser display_chooser;
   EXPECT_EQ(1, GetPrimaryDisplay());
-  display_chooser.TryToPlaceUiOnTouchDisplay();
+  display_chooser()->TryToPlaceUiOnTouchDisplay();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, GetPrimaryDisplay());
 }

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

@@ -390,9 +390,7 @@ OobeUI::OobeUI(content::WebUI* web_ui, const GURL& url)
   content::WebContents* contents = web_ui->GetWebContents();
   extensions::TabHelper::CreateForWebContents(contents);
 
-  // TODO(felixe): Display iteration and primary display selection not supported
-  // in Mash. See http://crbug.com/720917.
-  if (features::IsAshInBrowserProcess() && IsRemoraRequisitioned())
+  if (IsRemoraRequisitioned())
     oobe_display_chooser_ = std::make_unique<OobeDisplayChooser>();
 }