Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(122)

Unified Diff: ui/events/platform/x11/x11_hotplug_event_handler.cc

Issue 688163004: Move X11 device processing to worker threads (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@fix-blocking-touchscreen
Patch Set: Add null Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ui/events/platform/x11/x11_hotplug_event_handler.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/events/platform/x11/x11_hotplug_event_handler.cc
diff --git a/ui/events/platform/x11/x11_hotplug_event_handler.cc b/ui/events/platform/x11/x11_hotplug_event_handler.cc
index 3ce944efc3ebf3ea54e8701ea5d26a5bd7120c24..b615897cb26978b78c33d3fc1e57811ae409e874 100644
--- a/ui/events/platform/x11/x11_hotplug_event_handler.cc
+++ b/ui/events/platform/x11/x11_hotplug_event_handler.cc
@@ -4,6 +4,7 @@
#include "ui/events/platform/x11/x11_hotplug_event_handler.h"
+#include <X11/Xatom.h>
#include <X11/extensions/XInput.h>
#include <X11/extensions/XInput2.h>
@@ -13,11 +14,17 @@
#include <string>
#include <vector>
+#include "base/bind.h"
#include "base/command_line.h"
+#include "base/location.h"
#include "base/logging.h"
#include "base/process/launch.h"
+#include "base/single_thread_task_runner.h"
#include "base/strings/string_util.h"
#include "base/sys_info.h"
+#include "base/thread_task_runner_handle.h"
+#include "base/threading/worker_pool.h"
+#include "ui/events/devices/device_data_manager.h"
#include "ui/events/devices/device_hotplug_event_observer.h"
#include "ui/events/devices/device_util_linux.h"
#include "ui/events/devices/input_device.h"
@@ -35,6 +42,105 @@ const char kATKeyboardName[] = "AT Translated Set 2 keyboard";
// The prefix of xinput devices corresponding to CrOS EC internal keyboards.
const char kCrosEcKeyboardPrefix[] = "cros-ec";
+const char* kCachedAtomList[] = {
+ "Abs MT Position X",
+ "Abs MT Position Y",
+ NULL,
+};
+
+typedef base::Callback<void(const std::vector<KeyboardDevice>&)>
+ KeyboardDeviceCallback;
+
+typedef base::Callback<void(const std::vector<TouchscreenDevice>&)>
+ TouchscreenDeviceCallback;
+
+// Used for updating the state on the UI thread once device information is
+// parsed on helper threads.
+struct UiCallbacks {
+ KeyboardDeviceCallback keyboard_callback;
+ TouchscreenDeviceCallback touchscreen_callback;
+};
+
+// Stores a copy of the XIValuatorClassInfo values so X11 device processing can
+// happen on a worker thread. This is needed since X11 structs are not copyable.
+struct ValuatorClassInfo {
+ ValuatorClassInfo(const XIValuatorClassInfo& info)
+ : label(info.label),
+ max(info.max),
+ min(info.min),
+ mode(info.mode),
+ number(info.number) {}
+
+ Atom label;
+ double max;
+ double min;
+ int mode;
+ int number;
+};
+
+// Stores a copy of the XITouchClassInfo values so X11 device processing can
+// happen on a worker thread. This is needed since X11 structs are not copyable.
+struct TouchClassInfo {
+ TouchClassInfo(const XITouchClassInfo& info)
+ : mode(info.mode) {}
+
+ int mode;
+};
+
+struct DeviceInfo {
+ DeviceInfo(const XIDeviceInfo& device, const base::FilePath& path)
+ : id(device.deviceid),
+ name(device.name),
+ use(device.use),
+ enabled(device.enabled),
+ path(path) {
+ for (int i = 0; i < device.num_classes; ++i) {
+ switch (device.classes[i]->type) {
+ case XIValuatorClass:
+ valuator_class_infos.push_back(ValuatorClassInfo(
+ *reinterpret_cast<XIValuatorClassInfo*>(device.classes[i])));
+ break;
+#if defined(USE_XI2_MT)
+ case XITouchClass:
+ touch_class_infos.push_back(TouchClassInfo(
+ *reinterpret_cast<XITouchClassInfo*>(device.classes[i])));
+ break;
+#endif
+ default:
+ break;
+ }
+ }
+ }
+
+ // Unique device identifier.
+ int id;
+
+ // Internal device name.
+ std::string name;
+
+ // Device type (ie: XIMasterPointer)
+ int use;
+
+ // Specifies if the device is enabled and can send events.
+ bool enabled;
+
+ // Path to the actual device (ie: /dev/input/eventXX)
+ base::FilePath path;
+
+ std::vector<ValuatorClassInfo> valuator_class_infos;
+
+#if defined(USE_XI2_MT)
+ std::vector<TouchClassInfo> touch_class_infos;
+#endif
+};
+
+// X11 display cache used on worker threads. This is filled on the UI thread and
+// passed in to the worker threads.
+struct DisplayState {
+ Atom mt_position_x;
+ Atom mt_position_y;
+};
+
// Returns true if |name| is the name of a known keyboard device. Note, this may
// return false negatives.
bool IsKnownKeyboard(const std::string& name) {
@@ -59,23 +165,19 @@ bool IsTestKeyboard(const std::string& name) {
return name.find("XTEST") != std::string::npos;
}
-// We consider the touchscreen to be internal if it is an I2c device.
-// With the device id, we can query X to get the device's dev input
-// node eventXXX. Then we search all the dev input nodes registered
-// by I2C devices to see if we can find eventXXX.
-bool IsTouchscreenInternal(XDisplay* dpy, int device_id) {
-#if !defined(CHROMEOS)
- return false;
+base::FilePath GetDevicePath(XDisplay* dpy, int device_id) {
+#if !defined(OS_CHROMEOS)
+ return base::FilePath();
#else
if (!base::SysInfo::IsRunningOnChromeOS())
- return false;
+ return base::FilePath();
#endif
// Input device has a property "Device Node" pointing to its dev input node,
// e.g. Device Node (250): "/dev/input/event8"
Atom device_node = XInternAtom(dpy, "Device Node", False);
if (device_node == None)
- return false;
+ return base::FilePath();
Atom actual_type;
int actual_format;
@@ -83,7 +185,7 @@ bool IsTouchscreenInternal(XDisplay* dpy, int device_id) {
unsigned char* data;
XDevice* dev = XOpenDevice(dpy, device_id);
if (!dev)
- return false;
+ return base::FilePath();
if (XGetDeviceProperty(dpy,
dev,
@@ -98,40 +200,32 @@ bool IsTouchscreenInternal(XDisplay* dpy, int device_id) {
&bytes_after,
&data) != Success) {
XCloseDevice(dpy, dev);
- return false;
+ return base::FilePath();
}
- base::FilePath dev_node_path(reinterpret_cast<char*>(data));
- XFree(data);
- XCloseDevice(dpy, dev);
-
- return ui::IsTouchscreenInternal(dev_node_path);
-}
-} // namespace
+ std::string path;
+ // Make sure the returned value is a string.
+ if (actual_type == XA_STRING && actual_format == 8)
+ path = reinterpret_cast<char*>(data);
-X11HotplugEventHandler::X11HotplugEventHandler(
- DeviceHotplugEventObserver* delegate)
- : delegate_(delegate) {
-}
+ XFree(data);
+ XCloseDevice(dpy, dev);
-X11HotplugEventHandler::~X11HotplugEventHandler() {
+ return base::FilePath(path);
}
-void X11HotplugEventHandler::OnHotplugEvent() {
- const XIDeviceList& device_list =
- DeviceListCacheX11::GetInstance()->GetXI2DeviceList(gfx::GetXDisplay());
- HandleTouchscreenDevices(device_list);
- HandleKeyboardDevices(device_list);
-}
-
-void X11HotplugEventHandler::HandleKeyboardDevices(
- const XIDeviceList& x11_devices) {
+// Helper used to parse keyboard information. When it is done it uses
+// |reply_runner| and |callback| to update the state on the UI thread.
+void HandleKeyboardDevicesInWorker(
+ const std::vector<DeviceInfo>& device_infos,
+ scoped_refptr<base::TaskRunner> reply_runner,
+ const KeyboardDeviceCallback& callback) {
std::vector<KeyboardDevice> devices;
- for (int i = 0; i < x11_devices.count; i++) {
- if (!x11_devices[i].enabled || x11_devices[i].use != XISlaveKeyboard)
+ for (const DeviceInfo& device_info : device_infos) {
+ if (!device_info.enabled || device_info.use != XISlaveKeyboard)
continue; // Assume all keyboards are keyboard slaves
- std::string device_name(x11_devices[i].name);
+ std::string device_name(device_info.name);
base::TrimWhitespaceASCII(device_name, base::TRIM_TRAILING, &device_name);
if (IsTestKeyboard(device_name))
continue; // Skip test devices.
@@ -144,78 +238,136 @@ void X11HotplugEventHandler::HandleKeyboardDevices(
type = InputDeviceType::INPUT_DEVICE_UNKNOWN;
}
devices.push_back(
- KeyboardDevice(x11_devices[i].deviceid, type, device_name));
+ KeyboardDevice(device_info.id, type, device_name));
}
- delegate_->OnKeyboardDevicesUpdated(devices);
+
+ reply_runner->PostTask(FROM_HERE, base::Bind(callback, devices));
}
-void X11HotplugEventHandler::HandleTouchscreenDevices(
- const XIDeviceList& x11_devices) {
+// Helper used to parse touchscreen information. When it is done it uses
+// |reply_runner| and |callback| to update the state on the UI thread.
+void HandleTouchscreenDevicesInWorker(
+ const std::vector<DeviceInfo>& device_infos,
+ const DisplayState& display_state,
+ scoped_refptr<base::TaskRunner> reply_runner,
+ const TouchscreenDeviceCallback& callback) {
std::vector<TouchscreenDevice> devices;
- Display* display = gfx::GetXDisplay();
- Atom valuator_x = XInternAtom(display, "Abs MT Position X", False);
- Atom valuator_y = XInternAtom(display, "Abs MT Position Y", False);
- if (valuator_x == None || valuator_y == None)
+ if (display_state.mt_position_x == None ||
+ display_state.mt_position_y == None)
return;
std::set<int> no_match_touchscreen;
- for (int i = 0; i < x11_devices.count; i++) {
- if (!x11_devices[i].enabled || x11_devices[i].use != XIFloatingSlave)
+ for (const DeviceInfo& device_info : device_infos) {
+ if (!device_info.enabled || device_info.use != XIFloatingSlave)
continue; // Assume all touchscreens are floating slaves
double max_x = -1.0;
double max_y = -1.0;
bool is_direct_touch = false;
- for (int j = 0; j < x11_devices[i].num_classes; j++) {
- XIAnyClassInfo* class_info = x11_devices[i].classes[j];
-
- if (class_info->type == XIValuatorClass) {
- XIValuatorClassInfo* valuator_info =
- reinterpret_cast<XIValuatorClassInfo*>(class_info);
-
- if (valuator_x == valuator_info->label) {
- // Ignore X axis valuator with unexpected properties
- if (valuator_info->number == 0 && valuator_info->mode == Absolute &&
- valuator_info->min == 0.0) {
- max_x = valuator_info->max;
- }
- } else if (valuator_y == valuator_info->label) {
- // Ignore Y axis valuator with unexpected properties
- if (valuator_info->number == 1 && valuator_info->mode == Absolute &&
- valuator_info->min == 0.0) {
- max_y = valuator_info->max;
- }
+ for (const ValuatorClassInfo& valuator : device_info.valuator_class_infos) {
+ if (display_state.mt_position_x == valuator.label) {
+ // Ignore X axis valuator with unexpected properties
+ if (valuator.number == 0 && valuator.mode == Absolute &&
+ valuator.min == 0.0) {
+ max_x = valuator.max;
+ }
+ } else if (display_state.mt_position_y == valuator.label) {
+ // Ignore Y axis valuator with unexpected properties
+ if (valuator.number == 1 && valuator.mode == Absolute &&
+ valuator.min == 0.0) {
+ max_y = valuator.max;
}
}
+ }
+
#if defined(USE_XI2_MT)
- if (class_info->type == XITouchClass) {
- XITouchClassInfo* touch_info =
- reinterpret_cast<XITouchClassInfo*>(class_info);
- is_direct_touch = touch_info->mode == XIDirectTouch;
- }
-#endif
+ for (const TouchClassInfo& info : device_info.touch_class_infos) {
+ is_direct_touch = info.mode == XIDirectTouch;
}
+#endif
// Touchscreens should have absolute X and Y axes, and be direct touch
// devices.
if (max_x > 0.0 && max_y > 0.0 && is_direct_touch) {
- InputDeviceType type =
- IsTouchscreenInternal(display, x11_devices[i].deviceid)
+ InputDeviceType type = IsTouchscreenInternal(device_info.path)
? InputDeviceType::INPUT_DEVICE_INTERNAL
: InputDeviceType::INPUT_DEVICE_EXTERNAL;
- std::string name(x11_devices[i].name);
// |max_x| and |max_y| are inclusive values, so we need to add 1 to get
// the size.
devices.push_back(TouchscreenDevice(
- x11_devices[i].deviceid,
+ device_info.id,
type,
- name,
+ device_info.name,
gfx::Size(max_x + 1, max_y + 1)));
}
}
- delegate_->OnTouchscreenDevicesUpdated(devices);
+ reply_runner->PostTask(FROM_HERE, base::Bind(callback, devices));
+}
+
+// Called on a worker thread to parse the device information.
+void HandleHotplugEventInWorker(
+ const std::vector<DeviceInfo>& devices,
+ const DisplayState& display_state,
+ scoped_refptr<base::TaskRunner> reply_runner,
+ const UiCallbacks& callbacks) {
+ HandleTouchscreenDevicesInWorker(
+ devices, display_state, reply_runner, callbacks.touchscreen_callback);
+ HandleKeyboardDevicesInWorker(
+ devices, reply_runner, callbacks.keyboard_callback);
+}
+
+DeviceHotplugEventObserver* GetHotplugEventObserver() {
+ return DeviceDataManager::GetInstance();
+}
+
+void OnKeyboardDevices(const std::vector<KeyboardDevice>& devices) {
+ GetHotplugEventObserver()->OnKeyboardDevicesUpdated(devices);
+}
+
+void OnTouchscreenDevices(const std::vector<TouchscreenDevice>& devices) {
+ GetHotplugEventObserver()->OnTouchscreenDevicesUpdated(devices);
+}
+
+} // namespace
+
+X11HotplugEventHandler::X11HotplugEventHandler()
+ : atom_cache_(gfx::GetXDisplay(), kCachedAtomList) {
+}
+
+X11HotplugEventHandler::~X11HotplugEventHandler() {
+}
+
+void X11HotplugEventHandler::OnHotplugEvent() {
+ const XIDeviceList& device_list =
+ DeviceListCacheX11::GetInstance()->GetXI2DeviceList(gfx::GetXDisplay());
+ Display* display = gfx::GetXDisplay();
+
+ std::vector<DeviceInfo> device_infos;
+ for (int i = 0; i < device_list.count; ++i)
+ device_infos.push_back(DeviceInfo(
+ device_list[i], GetDevicePath(display, device_list[i].deviceid)));
+
+ // X11 is not thread safe, so first get all the required state.
+ DisplayState display_state;
+ display_state.mt_position_x = atom_cache_.GetAtom("Abs MT Position X");
+ display_state.mt_position_y = atom_cache_.GetAtom("Abs MT Position Y");
+
+ UiCallbacks callbacks;
+ callbacks.keyboard_callback = base::Bind(&OnKeyboardDevices);
+ callbacks.touchscreen_callback = base::Bind(&OnTouchscreenDevices);
+
+ // Parsing the device information may block, so delegate the operation to a
+ // worker thread. Once the device information is extracted the parsed devices
+ // will be returned via the callbacks.
+ base::WorkerPool::PostTask(FROM_HERE,
+ base::Bind(&HandleHotplugEventInWorker,
+ device_infos,
+ display_state,
+ base::ThreadTaskRunnerHandle::Get(),
+ callbacks),
+ true /* task_is_slow */);
}
} // namespace ui
« no previous file with comments | « ui/events/platform/x11/x11_hotplug_event_handler.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698