Reland OOBE display chooser commits + add Mash guard

This CL will reland / revert revert 2 commits and add a non-Mash guard
for listening to DeviceDataManager. A TODO has been added about using
InputDeviceClient in Mash, but it's not super urgent as the reaction to
the event is a NOP in Mash further down the call chain.

Revert "Revert of Remove confusing keyboard test & focus on input device"
  commit 8a2f3aa1

Revert "Revert "Listen to changes to touch input devices""
  commit 0cd134a5

Bug: 738885
Change-Id: Ic805ca3966df2969d8444b2c441eac24f75dce62
Reviewed-on: https://chromium-review.googlesource.com/575138
Commit-Queue: Felix Ekblom <felixe@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488622}

chrome/browser/chromeos/login/ui/login_display_host_impl.cc

@@ -99,6 +99,7 @@
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/events/devices/device_data_manager.h"
 #include "ui/events/event_handler.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/geometry/rect.h"
@@ -384,6 +385,12 @@ LoginDisplayHostImpl::LoginDisplayHostImpl(const gfx::Rect& wallpaper_bounds)
 
   display::Screen::GetScreen()->AddObserver(this);
 
+  // TODO(crbug.com/747267): Add Mash case. Not strictly needed since callee in
+  // observer method is NOP in Mash, but good for symmetry and to avoid leaking
+  // implementation details about OobeUI.
+  if (!ash_util::IsRunningInMash())
+    ui::DeviceDataManager::GetInstance()->AddObserver(this);
+
   // We need to listen to CLOSE_ALL_BROWSERS_REQUEST but not APP_TERMINATING
   // because/ APP_TERMINATING will never be fired as long as this keeps
   // ref-count. CLOSE_ALL_BROWSERS_REQUEST is safe here because there will be no
@@ -494,6 +501,12 @@ LoginDisplayHostImpl::~LoginDisplayHostImpl() {
   CrasAudioHandler::Get()->RemoveAudioObserver(this);
   display::Screen::GetScreen()->RemoveObserver(this);
 
+  // TODO(crbug.com/747267): Add Mash case. Not strictly needed since callee in
+  // observer method is NOP in Mash, but good for symmetry and to avoid leaking
+  // implementation details about OobeUI.
+  if (!ash_util::IsRunningInMash())
+    ui::DeviceDataManager::GetInstance()->RemoveObserver(this);
+
   if (login_view_ && login_window_)
     login_window_->RemoveRemovalsObserver(this);
 
@@ -1017,6 +1030,13 @@ void LoginDisplayHostImpl::OnDisplayMetricsChanged(
   }
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// LoginDisplayHostImpl, ui::InputDeviceEventObserver
+void LoginDisplayHostImpl::OnTouchscreenDeviceConfigurationChanged() {
+  if (GetOobeUI())
+    GetOobeUI()->OnDisplayConfigurationChanged();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // LoginDisplayHostImpl, views::WidgetRemovalsObserver:
 void LoginDisplayHostImpl::OnWillRemoveView(views::Widget* widget,

chrome/browser/chromeos/login/ui/login_display_host_impl.h

@@ -28,6 +28,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "ui/display/display_observer.h"
+#include "ui/events/devices/input_device_event_observer.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/widget/widget_removals_observer.h"
 #include "ui/wm/public/scoped_drag_drop_disabler.h"
@@ -51,6 +52,7 @@ class LoginDisplayHostImpl : public LoginDisplayHost,
                              public chromeos::SessionManagerClient::Observer,
                              public chromeos::CrasAudioHandler::AudioObserver,
                              public display::DisplayObserver,
+                             public ui::InputDeviceEventObserver,
                              public views::WidgetRemovalsObserver,
                              public chrome::MultiUserWindowManager::Observer {
  public:
@@ -122,6 +124,9 @@ class LoginDisplayHostImpl : public LoginDisplayHost,
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
 
+  // Overridden from ui::InputDeviceEventObserver
+  void OnTouchscreenDeviceConfigurationChanged() override;
+
   // Overriden from views::WidgetRemovalsObserver:
   void OnWillRemoveView(views::Widget* widget, views::View* view) override;
 

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

@@ -4,13 +4,17 @@
 
 #include "chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.h"
 
+#include <stdint.h>
+
 #include "ash/display/window_tree_host_manager.h"
 #include "ash/shell.h"
+#include "base/stl_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/display/display.h"
-#include "ui/display/display_layout.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
+#include "ui/events/devices/device_data_manager.h"
+#include "ui/events/devices/touchscreen_device.h"
 
 using content::BrowserThread;
 
@@ -23,6 +27,9 @@ bool TouchSupportAvailable(const display::Display& display) {
          display::Display::TouchSupport::TOUCH_SUPPORT_AVAILABLE;
 }
 
+// TODO(felixe): More context at crbug.com/738885
+const uint16_t kDeviceIds[] = {0x0457, 0x266e};
+
 }  // namespace
 
 OobeDisplayChooser::OobeDisplayChooser() : weak_ptr_factory_(this) {}
@@ -51,19 +58,22 @@ void OobeDisplayChooser::TryToPlaceUiOnTouchDisplay() {
 void OobeDisplayChooser::MoveToTouchDisplay() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  const display::Displays& displays =
-      ash::Shell::Get()->display_manager()->active_only_display_list();
+  const ui::DeviceDataManager* device_manager =
+      ui::DeviceDataManager::GetInstance();
+  for (const ui::TouchscreenDevice& device :
+       device_manager->GetTouchscreenDevices()) {
+    if (!base::ContainsValue(kDeviceIds, device.vendor_id))
+      continue;
 
-  if (displays.size() <= 1)
-    return;
+    int64_t display_id =
+        device_manager->GetTargetDisplayForTouchDevice(device.id);
+    if (display_id == display::kInvalidDisplayId)
+      continue;
 
-  for (const display::Display& display : displays) {
-    if (TouchSupportAvailable(display)) {
     ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(
-          display.id());
+        display_id);
     break;
   }
-  }
 }
 
 }  // namespace chromeos

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

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.h"
 
 #include <memory>
+#include <vector>
 
 #include "ash/display/display_configuration_controller.h"
 #include "ash/shell.h"
@@ -13,9 +14,12 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display.h"
 #include "ui/display/display_observer.h"
+#include "ui/display/manager/chromeos/touchscreen_util.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
 #include "ui/display/test/display_manager_test_api.h"
+#include "ui/events/devices/device_data_manager.h"
+#include "ui/events/devices/touchscreen_device.h"
 
 namespace chromeos {
 
@@ -25,63 +29,93 @@ class OobeDisplayChooserTest : public ash::AshTestBase {
  public:
   OobeDisplayChooserTest() : ash::AshTestBase() {}
 
-  void SetUp() override {
-    ash::AshTestBase::SetUp();
-    display_manager_test_api_.reset(
-        new display::test::DisplayManagerTestApi(display_manager()));
+  int64_t GetPrimaryDisplay() {
+    return display::Screen::GetScreen()->GetPrimaryDisplay().id();
   }
 
-  void EnableTouch(int64_t id) {
-    display_manager_test_api_->SetTouchSupport(
-        id, display::Display::TouchSupport::TOUCH_SUPPORT_AVAILABLE);
-  }
+  void UpdateTouchscreenDevices(const ui::TouchscreenDevice& touchscreen) {
+    std::vector<ui::TouchscreenDevice> devices{touchscreen};
 
-  void DisableTouch(int64_t id) {
-    display_manager_test_api_->SetTouchSupport(
-        id, display::Display::TouchSupport::TOUCH_SUPPORT_UNAVAILABLE);
-  }
-
-  int64_t GetPrimaryDisplay() {
-    return display::Screen::GetScreen()->GetPrimaryDisplay().id();
+    ui::DeviceHotplugEventObserver* manager =
+        ui::DeviceDataManager::GetInstance();
+    manager->OnTouchscreenDevicesUpdated(devices);
   }
 
  private:
-  std::unique_ptr<display::test::DisplayManagerTestApi>
-      display_manager_test_api_;
-
   DISALLOW_COPY_AND_ASSIGN(OobeDisplayChooserTest);
 };
 
+const uint16_t kWhitelistedId = 0x266e;
+
 }  // namespace
 
 TEST_F(OobeDisplayChooserTest, PreferTouchAsPrimary) {
-  OobeDisplayChooser display_chooser;
+  // Setup 2 displays, second one is intended to be a touch display
+  std::vector<display::ManagedDisplayInfo> display_info;
+  display_info.push_back(
+      display::ManagedDisplayInfo::CreateFromSpecWithID("0+0-3000x2000", 1));
+  display_info.push_back(
+      display::ManagedDisplayInfo::CreateFromSpecWithID("3000+0-800x600", 2));
+  display_manager()->OnNativeDisplaysChanged(display_info);
+  base::RunLoop().RunUntilIdle();
 
-  UpdateDisplay("3000x2000,800x600");
-  display::DisplayIdList ids = display_manager()->GetCurrentDisplayIdList();
-  DisableTouch(ids[0]);
-  EnableTouch(ids[1]);
+  // Make sure the non-touch display is primary
+  ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(1);
 
-  EXPECT_EQ(ids[0], GetPrimaryDisplay());
-  display_chooser.TryToPlaceUiOnTouchDisplay();
+  // Setup corresponding TouchscreenDevice object
+  ui::TouchscreenDevice touchscreen =
+      ui::TouchscreenDevice(1, ui::InputDeviceType::INPUT_DEVICE_EXTERNAL,
+                            "Touchscreen", gfx::Size(800, 600), 1);
+  touchscreen.vendor_id = kWhitelistedId;
+  UpdateTouchscreenDevices(touchscreen);
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(ids[1], GetPrimaryDisplay());
-}
+  // Associate touchscreen device with display
+  display_info[1].AddInputDevice(touchscreen.id);
+  display_info[1].set_touch_support(display::Display::TOUCH_SUPPORT_AVAILABLE);
+  display_manager()->OnNativeDisplaysChanged(display_info);
+  base::RunLoop().RunUntilIdle();
 
-TEST_F(OobeDisplayChooserTest, AddingSecondTouchDisplayShouldbeNOP) {
   OobeDisplayChooser display_chooser;
+  EXPECT_EQ(1, GetPrimaryDisplay());
+  display_chooser.TryToPlaceUiOnTouchDisplay();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2, GetPrimaryDisplay());
+}
 
-  UpdateDisplay("3000x2000,800x600");
-  display::DisplayIdList ids = display_manager()->GetCurrentDisplayIdList();
-  EnableTouch(ids[0]);
-  EnableTouch(ids[1]);
+TEST_F(OobeDisplayChooserTest, DontSwitchFromTouch) {
+  // Setup 2 displays, second one is intended to be a touch display
+  std::vector<display::ManagedDisplayInfo> display_info;
+  display_info.push_back(
+      display::ManagedDisplayInfo::CreateFromSpecWithID("0+0-3000x2000", 1));
+  display_info.push_back(
+      display::ManagedDisplayInfo::CreateFromSpecWithID("3000+0-800x600", 2));
+  display_info[0].set_touch_support(display::Display::TOUCH_SUPPORT_AVAILABLE);
+  display_manager()->OnNativeDisplaysChanged(display_info);
+  base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(ids[0], GetPrimaryDisplay());
-  display_chooser.TryToPlaceUiOnTouchDisplay();
+  // Make sure the non-touch display is primary
+  ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(1);
+
+  // Setup corresponding TouchscreenDevice object
+  ui::TouchscreenDevice touchscreen =
+      ui::TouchscreenDevice(1, ui::InputDeviceType::INPUT_DEVICE_EXTERNAL,
+                            "Touchscreen", gfx::Size(800, 600), 1);
+  touchscreen.vendor_id = kWhitelistedId;
+  UpdateTouchscreenDevices(touchscreen);
+  base::RunLoop().RunUntilIdle();
+
+  // Associate touchscreen device with display
+  display_info[1].AddInputDevice(touchscreen.id);
+  display_info[1].set_touch_support(display::Display::TOUCH_SUPPORT_AVAILABLE);
+  display_manager()->OnNativeDisplaysChanged(display_info);
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(ids[0], GetPrimaryDisplay());
+  OobeDisplayChooser display_chooser;
+  EXPECT_EQ(1, GetPrimaryDisplay());
+  display_chooser.TryToPlaceUiOnTouchDisplay();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, GetPrimaryDisplay());
 }
 
 }  // namespace chromeos

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

@@ -226,17 +226,12 @@ std::string GetDisplayType(const GURL& url) {
   return path;
 }
 
-bool IsKeyboardConnected() {
-  const std::vector<ui::InputDevice>& keyboards =
-      ui::InputDeviceManager::GetInstance()->GetKeyboardDevices();
-  for (const ui::InputDevice& keyboard : keyboards) {
-    if (keyboard.type == ui::INPUT_DEVICE_INTERNAL ||
-        keyboard.type == ui::INPUT_DEVICE_EXTERNAL) {
-      return true;
-    }
-  }
-
-  return false;
+bool IsRemoraRequisitioned() {
+  policy::DeviceCloudPolicyManagerChromeOS* policy_manager =
+      g_browser_process->platform_part()
+          ->browser_policy_connector_chromeos()
+          ->GetDeviceCloudPolicyManager();
+  return policy_manager && policy_manager->IsRemoraRequisition();
 }
 
 }  // namespace
@@ -376,7 +371,7 @@ OobeUI::OobeUI(content::WebUI* web_ui, const GURL& url)
 
   // TODO(felixe): Display iteration and primary display selection not supported
   // in Mash. See http://crbug.com/720917.
-  if (!ash_util::IsRunningInMash() && !IsKeyboardConnected())
+  if (!ash_util::IsRunningInMash() && IsRemoraRequisitioned())
     oobe_display_chooser_ = base::MakeUnique<OobeDisplayChooser>();
 }