Revert of Remove confusing keyboard test & focus on input device (patchset #9 id:160001 of https://codereview.chromium.org/2964823002/ )

Reason for revert:
A follow up fix for a race condition on cold boot was reverted in https://chromium-review.googlesource.com/c/574731/. With only this patch the functionality is broken in the user facing scenario.

The code state without either this cl or the follow up is better than with only this cl.

Original issue's description:
> Remove confusing keyboard test & inspect input device
>
> In order to make the decision for when to trigger the OOBE display
> chooser less confusing for end users I'm removing the keyboard check.
> Instead an explicit remora requisition check is used to limit the
> effects.
>
> BUG=738885
>
> Review-Url: https://codereview.chromium.org/2964823002
> Cr-Commit-Position: refs/heads/master@{#485019}
> Committed: https://chromium.googlesource.com/chromium/src/+/c923b817665a649b4e18243116c31c68404af7dd

[email protected],[email protected]
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=738885

Review-Url: https://codereview.chromium.org/2977293002
Cr-Commit-Position: refs/heads/master@{#487262}

Author: felixe

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

@@ -4,17 +4,13 @@
 
 #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;
 
@@ -27,9 +23,6 @@ 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) {}
@@ -58,21 +51,18 @@ void OobeDisplayChooser::TryToPlaceUiOnTouchDisplay() {
 void OobeDisplayChooser::MoveToTouchDisplay() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  const ui::DeviceDataManager* device_manager =
-      ui::DeviceDataManager::GetInstance();
-  for (const ui::TouchscreenDevice& device :
-       device_manager->GetTouchscreenDevices()) {
-    if (!base::ContainsValue(kDeviceIds, device.vendor_id))
-      continue;
-
-    int64_t display_id =
-        device_manager->GetTargetDisplayForTouchDevice(device.id);
-    if (display_id == display::kInvalidDisplayId)
-      continue;
-
-    ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(
-        display_id);
-    break;
+  const display::Displays& displays =
+      ash::Shell::Get()->display_manager()->active_only_display_list();
+
+  if (displays.size() <= 1)
+    return;
+
+  for (const display::Display& display : displays) {
+    if (TouchSupportAvailable(display)) {
+      ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(
+          display.id());
+      break;
+    }
   }
 }
 

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

@@ -5,7 +5,6 @@
 #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"
@@ -14,12 +13,9 @@
 #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 {
 
@@ -29,93 +25,63 @@ class OobeDisplayChooserTest : public ash::test::AshTestBase {
  public:
   OobeDisplayChooserTest() : ash::test::AshTestBase() {}
 
-  int64_t GetPrimaryDisplay() {
-    return display::Screen::GetScreen()->GetPrimaryDisplay().id();
+  void SetUp() override {
+    ash::test::AshTestBase::SetUp();
+    display_manager_test_api_.reset(
+        new display::test::DisplayManagerTestApi(display_manager()));
   }
 
-  void UpdateTouchscreenDevices(const ui::TouchscreenDevice& touchscreen) {
-    std::vector<ui::TouchscreenDevice> devices{touchscreen};
+  void EnableTouch(int64_t id) {
+    display_manager_test_api_->SetTouchSupport(
+        id, display::Display::TouchSupport::TOUCH_SUPPORT_AVAILABLE);
+  }
 
-    ui::DeviceHotplugEventObserver* manager =
-        ui::DeviceDataManager::GetInstance();
-    manager->OnTouchscreenDevicesUpdated(devices);
+  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();
   }
 
  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) {
-  // 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();
-
-  // 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();
+  OobeDisplayChooser display_chooser;
 
-  // 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();
+  UpdateDisplay("3000x2000,800x600");
+  display::DisplayIdList ids = display_manager()->GetCurrentDisplayIdList();
+  DisableTouch(ids[0]);
+  EnableTouch(ids[1]);
 
-  OobeDisplayChooser display_chooser;
-  EXPECT_EQ(1, GetPrimaryDisplay());
+  EXPECT_EQ(ids[0], GetPrimaryDisplay());
   display_chooser.TryToPlaceUiOnTouchDisplay();
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(2, GetPrimaryDisplay());
-}
-
-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();
 
-  // Make sure the non-touch display is primary
-  ash::Shell::Get()->window_tree_host_manager()->SetPrimaryDisplayId(1);
+  EXPECT_EQ(ids[1], GetPrimaryDisplay());
+}
 
-  // 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();
+TEST_F(OobeDisplayChooserTest, AddingSecondTouchDisplayShouldbeNOP) {
+  OobeDisplayChooser display_chooser;
 
-  // 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();
+  UpdateDisplay("3000x2000,800x600");
+  display::DisplayIdList ids = display_manager()->GetCurrentDisplayIdList();
+  EnableTouch(ids[0]);
+  EnableTouch(ids[1]);
 
-  OobeDisplayChooser display_chooser;
-  EXPECT_EQ(1, GetPrimaryDisplay());
+  EXPECT_EQ(ids[0], GetPrimaryDisplay());
   display_chooser.TryToPlaceUiOnTouchDisplay();
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(1, GetPrimaryDisplay());
+
+  EXPECT_EQ(ids[0], GetPrimaryDisplay());
 }
 
 }  // namespace chromeos

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

@@ -226,12 +226,17 @@ std::string GetDisplayType(const GURL& url) {
   return path;
 }
 
-bool IsRemoraRequisitioned() {
-  policy::DeviceCloudPolicyManagerChromeOS* policy_manager =
-      g_browser_process->platform_part()
-          ->browser_policy_connector_chromeos()
-          ->GetDeviceCloudPolicyManager();
-  return policy_manager && policy_manager->IsRemoraRequisition();
+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;
 }
 
 }  // namespace
@@ -371,7 +376,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() && IsRemoraRequisitioned())
+  if (!ash_util::IsRunningInMash() && !IsKeyboardConnected())
     oobe_display_chooser_ = base::MakeUnique<OobeDisplayChooser>();
 }