Skip to content

Commit

Permalink
Revert "chromeos: Converts OobeDisplayChooser to use InputDeviceManager"
Browse files Browse the repository at this point in the history
This reverts commit 6d57ca2.

Reason for revert: This makes Chrome not draw anything.

Original change's description:
> chromeos: Converts OobeDisplayChooser to use InputDeviceManager
> 
> As part of this I'm adding AreTouchscreenTargetDisplaysValid(), which
> lets client now whether TouchscreenDevice::target_display_id is valid.
> 
> BUG=720917
> TEST=covered by test
> 
> Change-Id: I672b33c6239b8acb9e42b7d038f902b87776ebd8
> Reviewed-on: https://chromium-review.googlesource.com/617589
> Reviewed-by: kylechar <kylechar@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#495286}

TBR=sky@chromium.org,tsepez@chromium.org,kylechar@chromium.org

Change-Id: I1cb6cb071048f3e69fe3b6e27738afdcf4a51dc3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 720917
Reviewed-on: https://chromium-review.googlesource.com/619629
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495367}
  • Loading branch information
Scott Violet authored and Commit Bot committed Aug 17, 2017
1 parent 2a7796a commit 803b9e7
Show file tree
Hide file tree
Showing 18 changed files with 45 additions and 208 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ include_rules = [
"+ui/events/devices/device_hotplug_event_observer.h",
"+ui/events/devices/input_device.h",
"+ui/events/devices/input_device_event_observer.h",
"+ui/events/devices/input_device_manager.h",
"+ui/events/devices/stylus_state.h",
"+ui/events/devices/touchscreen_device.h",
"+ui/events/devices/input_device_manager.h",

# Only used by X11 (non-ChromeOS) code, which can use X directly.
"+ui/events/devices/x11",
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/chromeos/login/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
specific_include_rules = {
# TODO(mash): Remove. http://crbug.com/720917.
"oobe_display_chooser_unittest.cc": [
"oobe_display_chooser\.*": [
"+ui/events/devices/device_data_manager.h",
]
}
61 changes: 17 additions & 44 deletions chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "ui/display/display.h"
#include "ui/display/manager/display_manager.h"
#include "ui/display/screen.h"
#include "ui/events/devices/input_device_manager.h"
#include "ui/events/devices/device_data_manager.h"
#include "ui/events/devices/touchscreen_device.h"

using content::BrowserThread;
Expand All @@ -30,16 +30,9 @@ bool TouchSupportAvailable(const display::Display& display) {
// TODO(felixe): More context at crbug.com/738885
const uint16_t kDeviceIds[] = {0x0457, 0x266e};

// Returns true if |vendor_id| is a valid vendor id that may be made the primary
// display.
bool IsWhiteListedVendorId(uint16_t vendor_id) {
return base::ContainsValue(kDeviceIds, vendor_id);
}

} // namespace

OobeDisplayChooser::OobeDisplayChooser()
: scoped_observer_(this), weak_ptr_factory_(this) {}
OobeDisplayChooser::OobeDisplayChooser() : weak_ptr_factory_(this) {}

OobeDisplayChooser::~OobeDisplayChooser() {}

Expand All @@ -57,50 +50,30 @@ void OobeDisplayChooser::TryToPlaceUiOnTouchDisplay() {
if (primary_display.is_valid() && !TouchSupportAvailable(primary_display)) {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::BindOnce(&OobeDisplayChooser::MaybeMoveToTouchDisplay,
base::BindOnce(&OobeDisplayChooser::MoveToTouchDisplay,
weak_ptr_factory_.GetWeakPtr()));
}
}

void OobeDisplayChooser::MaybeMoveToTouchDisplay() {
ui::InputDeviceManager* input_device_manager =
ui::InputDeviceManager::GetInstance();
if (input_device_manager->AreDeviceListsComplete() &&
input_device_manager->AreTouchscreenTargetDisplaysValid()) {
MoveToTouchDisplay();
} else if (!scoped_observer_.IsObserving(input_device_manager)) {
scoped_observer_.Add(input_device_manager);
}
}

void OobeDisplayChooser::MoveToTouchDisplay() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

scoped_observer_.RemoveAll();

const ui::InputDeviceManager* input_device_manager =
ui::InputDeviceManager::GetInstance();
const ui::DeviceDataManager* device_manager =
ui::DeviceDataManager::GetInstance();
for (const ui::TouchscreenDevice& device :
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);
break;
}
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;
}
}

void OobeDisplayChooser::OnTouchDeviceAssociationChanged() {
MaybeMoveToTouchDisplay();
}

void OobeDisplayChooser::OnTouchscreenDeviceConfigurationChanged() {
MaybeMoveToTouchDisplay();
}

void OobeDisplayChooser::OnDeviceListsComplete() {
MaybeMoveToTouchDisplay();
}

} // namespace chromeos
22 changes: 2 additions & 20 deletions chrome/browser/ui/webui/chromeos/login/oobe_display_chooser.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,21 @@

#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "ui/events/devices/input_device_event_observer.h"

namespace ui {
class InputDeviceManager;
}

namespace chromeos {

class OobeDisplayChooser : public ui::InputDeviceEventObserver {
class OobeDisplayChooser {
public:
OobeDisplayChooser();
~OobeDisplayChooser() override;
~OobeDisplayChooser();

// Tries to put the OOBE UI on a connected touch display (if available).
// Must be called on the BrowserThread::UI thread.
void TryToPlaceUiOnTouchDisplay();

private:
// Calls MoveToTouchDisplay() if touch device list is ready, otherwise adds an
// observer that calls MoveToTouchDisplay() once ready.
void MaybeMoveToTouchDisplay();

void MoveToTouchDisplay();

// ui::InputDeviceEventObserver:
void OnTouchDeviceAssociationChanged() override;
void OnTouchscreenDeviceConfigurationChanged() override;
void OnDeviceListsComplete() override;

ScopedObserver<ui::InputDeviceManager, ui::InputDeviceEventObserver>
scoped_observer_;

base::WeakPtrFactory<OobeDisplayChooser> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(OobeDisplayChooser);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ class OobeDisplayChooserTest : public ash::AshTestBase {
manager->OnTouchscreenDevicesUpdated(devices);
}

// ash::AshTestBase:
void SetUp() override {
ash::AshTestBase::SetUp();
static_cast<ui::DeviceHotplugEventObserver*>(
ui::DeviceDataManager::GetInstance())
->OnDeviceListsComplete();
}

private:
DISALLOW_COPY_AND_ASSIGN(OobeDisplayChooserTest);
};
Expand Down
29 changes: 8 additions & 21 deletions services/ui/input_devices/input_device_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,13 @@ void InputDeviceServer::OnKeyboardDeviceConfigurationChanged() {
}

void InputDeviceServer::OnTouchscreenDeviceConfigurationChanged() {
CallOnTouchscreenDeviceConfigurationChanged();
if (!manager_->AreDeviceListsComplete())
return;

auto& devices = manager_->GetTouchscreenDevices();
observers_.ForAllPtrs([&devices](mojom::InputDeviceObserverMojo* observer) {
observer->OnTouchscreenDeviceConfigurationChanged(devices);
});
}

void InputDeviceServer::OnMouseDeviceConfigurationChanged() {
Expand Down Expand Up @@ -107,32 +113,13 @@ void InputDeviceServer::OnStylusStateChanged(StylusState state) {
});
}

void InputDeviceServer::OnTouchDeviceAssociationChanged() {
CallOnTouchscreenDeviceConfigurationChanged();
}

void InputDeviceServer::SendDeviceListsComplete(
mojom::InputDeviceObserverMojo* observer) {
DCHECK(manager_->AreDeviceListsComplete());

observer->OnDeviceListsComplete(
manager_->GetKeyboardDevices(), manager_->GetTouchscreenDevices(),
manager_->GetMouseDevices(), manager_->GetTouchpadDevices(),
manager_->AreTouchscreenTargetDisplaysValid());
}

void InputDeviceServer::CallOnTouchscreenDeviceConfigurationChanged() {
if (!manager_->AreDeviceListsComplete())
return;

auto& devices = manager_->GetTouchscreenDevices();
const bool are_touchscreen_target_displays_valid =
manager_->AreTouchscreenTargetDisplaysValid();
observers_.ForAllPtrs([&devices, are_touchscreen_target_displays_valid](
mojom::InputDeviceObserverMojo* observer) {
observer->OnTouchscreenDeviceConfigurationChanged(
devices, are_touchscreen_target_displays_valid);
});
manager_->GetMouseDevices(), manager_->GetTouchpadDevices());
}

void InputDeviceServer::BindInputDeviceServerRequest(
Expand Down
3 changes: 0 additions & 3 deletions services/ui/input_devices/input_device_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,11 @@ class InputDeviceServer : public mojom::InputDeviceServer,
void OnTouchpadDeviceConfigurationChanged() override;
void OnDeviceListsComplete() override;
void OnStylusStateChanged(StylusState state) override;
void OnTouchDeviceAssociationChanged() override;

private:
// Sends the current state of all input-devices to an observer.
void SendDeviceListsComplete(mojom::InputDeviceObserverMojo* observer);

void CallOnTouchscreenDeviceConfigurationChanged();

void BindInputDeviceServerRequest(
mojom::InputDeviceServerRequest request,
const service_manager::BindSourceInfo& source_info);
Expand Down
20 changes: 4 additions & 16 deletions services/ui/public/cpp/input_devices/input_device_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ bool InputDeviceClient::AreTouchscreensEnabled() const {
return true;
}

bool InputDeviceClient::AreTouchscreenTargetDisplaysValid() const {
return are_touchscreen_target_displays_valid_;
}

void InputDeviceClient::AddObserver(ui::InputDeviceEventObserver* observer) {
observers_.AddObserver(observer);
}
Expand Down Expand Up @@ -81,11 +77,8 @@ void InputDeviceClient::OnKeyboardDeviceConfigurationChanged(
}

void InputDeviceClient::OnTouchscreenDeviceConfigurationChanged(
const std::vector<ui::TouchscreenDevice>& devices,
bool are_touchscreen_target_displays_valid) {
const std::vector<ui::TouchscreenDevice>& devices) {
touchscreen_devices_ = devices;
are_touchscreen_target_displays_valid_ =
are_touchscreen_target_displays_valid;
for (auto& observer : observers_)
observer.OnTouchscreenDeviceConfigurationChanged();
}
Expand All @@ -108,17 +101,12 @@ void InputDeviceClient::OnDeviceListsComplete(
const std::vector<ui::InputDevice>& keyboard_devices,
const std::vector<ui::TouchscreenDevice>& touchscreen_devices,
const std::vector<ui::InputDevice>& mouse_devices,
const std::vector<ui::InputDevice>& touchpad_devices,
bool are_touchscreen_target_displays_valid) {
are_touchscreen_target_displays_valid_ =
are_touchscreen_target_displays_valid;
const std::vector<ui::InputDevice>& touchpad_devices) {
// Update the cached device lists if the received list isn't empty.
if (!keyboard_devices.empty())
OnKeyboardDeviceConfigurationChanged(keyboard_devices);
if (!touchscreen_devices.empty()) {
OnTouchscreenDeviceConfigurationChanged(
touchscreen_devices, are_touchscreen_target_displays_valid);
}
if (!touchscreen_devices.empty())
OnTouchscreenDeviceConfigurationChanged(touchscreen_devices);
if (!mouse_devices.empty())
OnMouseDeviceConfigurationChanged(mouse_devices);
if (!touchpad_devices.empty())
Expand Down
8 changes: 2 additions & 6 deletions services/ui/public/cpp/input_devices/input_device_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class InputDeviceClient : public mojom::InputDeviceObserverMojo,

bool AreDeviceListsComplete() const override;
bool AreTouchscreensEnabled() const override;
bool AreTouchscreenTargetDisplaysValid() const override;

void AddObserver(ui::InputDeviceEventObserver* observer) override;
void RemoveObserver(ui::InputDeviceEventObserver* observer) override;
Expand All @@ -57,8 +56,7 @@ class InputDeviceClient : public mojom::InputDeviceObserverMojo,
void OnKeyboardDeviceConfigurationChanged(
const std::vector<ui::InputDevice>& devices) override;
void OnTouchscreenDeviceConfigurationChanged(
const std::vector<ui::TouchscreenDevice>& devices,
bool are_touchscreen_target_displays_valid) override;
const std::vector<ui::TouchscreenDevice>& devices) override;
void OnMouseDeviceConfigurationChanged(
const std::vector<ui::InputDevice>& devices) override;
void OnTouchpadDeviceConfigurationChanged(
Expand All @@ -67,8 +65,7 @@ class InputDeviceClient : public mojom::InputDeviceObserverMojo,
const std::vector<ui::InputDevice>& keyboard_devices,
const std::vector<ui::TouchscreenDevice>& touchscreen_devices,
const std::vector<ui::InputDevice>& mouse_devices,
const std::vector<ui::InputDevice>& touchpad_devices,
bool are_touchscreen_target_displays_valid) override;
const std::vector<ui::InputDevice>& touchpad_devices) override;
void OnStylusStateChanged(StylusState state) override;

private:
Expand All @@ -83,7 +80,6 @@ class InputDeviceClient : public mojom::InputDeviceObserverMojo,
std::vector<ui::InputDevice> mouse_devices_;
std::vector<ui::InputDevice> touchpad_devices_;
bool device_lists_complete_ = false;
bool are_touchscreen_target_displays_valid_ = false;

// List of in-process observers.
base::ObserverList<ui::InputDeviceEventObserver> observers_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ interface InputDeviceObserverMojo {

// Is called when the list of touchscreens changes.
OnTouchscreenDeviceConfigurationChanged(
array<ui.mojom.TouchscreenDevice> devices,
bool are_touchscreen_target_displays_valid);
array<ui.mojom.TouchscreenDevice> devices);

// Is called when the list of mice changes.
OnMouseDeviceConfigurationChanged(array<ui.mojom.InputDevice> devices);
Expand All @@ -29,8 +28,7 @@ interface InputDeviceObserverMojo {
array<ui.mojom.InputDevice> keyboard_devices,
array<ui.mojom.TouchscreenDevice> touchscreen_devices,
array<ui.mojom.InputDevice> mouse_devices,
array<ui.mojom.InputDevice> touchpad_devices,
bool are_touchscreen_target_displays_valid);
array<ui.mojom.InputDevice> touchpad_devices);

// Is called when a stylus is removed or inserted into the device.
OnStylusStateChanged(StylusState state);
Expand Down
5 changes: 1 addition & 4 deletions ui/events/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ if (!is_ios) {
"blink/input_scroll_elasticity_controller_unittest.cc",
"blink/web_input_event_traits_unittest.cc",
"blink/web_input_event_unittest.cc",
"devices/device_data_manager_unittest.cc",
"devices/mojo/device_struct_traits_unittest.cc",
"devices/mojo/touch_device_transform_struct_traits_unittest.cc",
"gestures/blink/web_gesture_curve_impl_unittest.cc",
Expand Down Expand Up @@ -447,10 +448,6 @@ if (!is_ios) {
]
}

if (use_x11 || use_ozone) {
sources += [ "devices/device_data_manager_unittest.cc" ]
}

if (use_ozone) {
sources += [
"ozone/chromeos/cursor_controller_unittest.cc",
Expand Down
13 changes: 4 additions & 9 deletions ui/events/devices/device_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ DeviceDataManager::~DeviceDataManager() {
InputDeviceManager::ClearInstance();
}

// static
DeviceDataManager* DeviceDataManager::instance() { return instance_; }

// static
void DeviceDataManager::set_instance(DeviceDataManager* instance) {
DCHECK(instance)
Expand All @@ -50,7 +53,7 @@ void DeviceDataManager::set_instance(DeviceDataManager* instance) {

// static
void DeviceDataManager::CreateInstance() {
if (instance_)
if (instance())
return;

set_instance(new DeviceDataManager());
Expand Down Expand Up @@ -83,9 +86,6 @@ void DeviceDataManager::ConfigureTouchDevices(
ClearTouchDeviceAssociations();
for (const TouchDeviceTransform& transform : transforms)
UpdateTouchInfoFromTransform(transform);
are_touchscreen_target_displays_valid_ = true;
for (InputDeviceEventObserver& observer : observers_)
observer.OnTouchDeviceAssociationChanged();
}

void DeviceDataManager::ClearTouchDeviceAssociations() {
Expand Down Expand Up @@ -169,7 +169,6 @@ void DeviceDataManager::OnTouchscreenDevicesUpdated(
InputDeviceEquals)) {
return;
}
are_touchscreen_target_displays_valid_ = false;
touchscreen_devices_ = devices;
for (TouchscreenDevice& touchscreen_device : touchscreen_devices_) {
touchscreen_device.target_display_id =
Expand Down Expand Up @@ -261,8 +260,4 @@ bool DeviceDataManager::AreTouchscreensEnabled() const {
return touch_screens_enabled_;
}

bool DeviceDataManager::AreTouchscreenTargetDisplaysValid() const {
return are_touchscreen_target_displays_valid_;
}

} // namespace ui
Loading

0 comments on commit 803b9e7

Please sign in to comment.