Skip to content

Commit

Permalink
ozone: evdev: Fix pen-related infinite recursion
Browse files Browse the repository at this point in the history
If the pen device is disabled while the pen is near the screen it leads
to infinite recursion. This is because we make a call to disable palm
suppression, which revalidates device enablement. Post a task instead
of disabling inline to fix this.

The same thing happens if we lose events since Reinitialize() gained
a call to ReleaseButtons() when pen support was added, which calls
ReportEvents(), which can again call Reinitialize(). To fix that one,
kill ReleaseButtons() as ReleaseTouches() should be sufficient.

BUG=814071
TEST=powerd_dbus_suspend with pen touching screen

Change-Id: I8bb66feddc7961f1fa28d062133671ad1c46948c
Reviewed-on: https://chromium-review.googlesource.com/961386
Reviewed-by: Kevin Schoedel <kpschoedel@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543185}
  • Loading branch information
mspang authored and Commit Bot committed Mar 14, 2018
1 parent ca14de9 commit c642ba4
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 29 deletions.
11 changes: 11 additions & 0 deletions ui/events/ozone/evdev/input_device_factory_evdev.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <utility>

#include "base/bind.h"
#include "base/files/scoped_file.h"
#include "base/memory/ptr_util.h"
#include "base/threading/thread_task_runner_handle.h"
Expand Down Expand Up @@ -509,6 +510,16 @@ void InputDeviceFactoryEvdev::EnablePalmSuppression(bool enabled) {
return;
palm_suppression_enabled_ = enabled;

// This function can be called while disabling pen devices, so don't disable
// inline here.
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&InputDeviceFactoryEvdev::EnableDevices,
weak_ptr_factory_.GetWeakPtr()));
}

void InputDeviceFactoryEvdev::EnableDevices() {
// TODO(spang): Fix the UI to not dismiss menus when we use
// ApplyInputDeviceSettings() instead of this function.
for (const auto& it : converters_)
it.second->SetEnabled(IsDeviceEnabled(it.second.get()));
}
Expand Down
4 changes: 2 additions & 2 deletions ui/events/ozone/evdev/input_device_factory_evdev.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class EVENTS_OZONE_EVDEV_EXPORT InputDeviceFactoryEvdev {

base::WeakPtr<InputDeviceFactoryEvdev> GetWeakPtr();

void EnablePalmSuppression(bool enabled);

private:
// Open device at path & starting processing events.
void AttachInputDevice(std::unique_ptr<EventConverterEvdev> converter);
Expand Down Expand Up @@ -94,6 +92,8 @@ class EVENTS_OZONE_EVDEV_EXPORT InputDeviceFactoryEvdev {
void SetBoolPropertyForOneType(const EventDeviceType type,
const std::string& name,
bool value);
void EnablePalmSuppression(bool enabled);
void EnableDevices();

// Task runner for our thread.
scoped_refptr<base::TaskRunner> task_runner_;
Expand Down
7 changes: 1 addition & 6 deletions ui/events/ozone/evdev/touch_evdev_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ struct EVENTS_OZONE_EVDEV_EXPORT InProgressTouchEvdev {
float tilt_y = 0;
ui::EventPointerType reported_tool_type =
ui::EventPointerType::POINTER_TYPE_TOUCH;

struct ButtonState {
bool down = false;
bool changed = false;
};
ButtonState btn_stylus;
bool stylus_button = false;
};

} // namespace ui
Expand Down
24 changes: 4 additions & 20 deletions ui/events/ozone/evdev/touch_event_converter_evdev.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ void TouchEventConverterEvdev::Initialize(const EventDeviceInfo& info) {
MT_TOOL_FINGER);
events_[i].tool_type = tool_type;
events_[i].major = touch_major;
events_[i].stylus_button = false;
if (events_[i].cancelled)
cancelled_state = true;
}
Expand Down Expand Up @@ -264,8 +265,6 @@ void TouchEventConverterEvdev::Initialize(const EventDeviceInfo& info) {
}

void TouchEventConverterEvdev::Reinitialize() {
ReleaseButtons();

EventDeviceInfo info;
if (!info.Initialize(fd_, path_)) {
LOG(ERROR) << "Failed to synchronize state for touch device: "
Expand Down Expand Up @@ -297,7 +296,6 @@ void TouchEventConverterEvdev::OnEnabled() {

void TouchEventConverterEvdev::OnDisabled() {
ReleaseTouches();
ReleaseButtons();
if (enable_palm_suppression_callback_) {
enable_palm_suppression_callback_.Run(false);
}
Expand Down Expand Up @@ -391,8 +389,7 @@ void TouchEventConverterEvdev::EmulateMultitouchEvent(
void TouchEventConverterEvdev::ProcessKey(const input_event& input) {
switch (input.code) {
case BTN_STYLUS:
events_[current_slot_].btn_stylus.down = input.value;
events_[current_slot_].btn_stylus.changed = true;
events_[current_slot_].stylus_button = input.value;
events_[current_slot_].altered = true;
break;
case BTN_TOOL_PEN:
Expand Down Expand Up @@ -520,7 +517,7 @@ void TouchEventConverterEvdev::ReportTouchEvent(
ui::PointerDetails details(event.reported_tool_type, /* pointer_id*/ 0,
event.radius_x, event.radius_y, event.pressure,
/* twist */ 0, event.tilt_x, event.tilt_y);
int flags = event.btn_stylus.down ? ui::EventFlags::EF_LEFT_MOUSE_BUTTON : 0;
int flags = event.stylus_button ? ui::EventFlags::EF_LEFT_MOUSE_BUTTON : 0;
dispatcher_->DispatchTouchEvent(TouchEventParams(
input_device_.id, event.slot, event_type, gfx::PointF(event.x, event.y),
details, timestamp, flags));
Expand Down Expand Up @@ -587,7 +584,6 @@ void TouchEventConverterEvdev::ReportEvents(base::TimeTicks timestamp) {
event->was_touching = event->touching;
event->was_delayed = event->delayed;
event->altered = false;
event->btn_stylus.changed = false;
}
}

Expand All @@ -609,26 +605,14 @@ void TouchEventConverterEvdev::UpdateTrackingId(int slot, int tracking_id) {

void TouchEventConverterEvdev::ReleaseTouches() {
for (size_t slot = 0; slot < events_.size(); slot++) {
events_[slot].stylus_button = false;
events_[slot].cancelled = true;
events_[slot].altered = true;
}

ReportEvents(EventTimeForNow());
}

void TouchEventConverterEvdev::ReleaseButtons() {
for (size_t slot = 0; slot < events_.size(); slot++) {
InProgressTouchEvdev* event = &events_[slot];

if (event->btn_stylus.down) {
event->btn_stylus.down = false;
event->btn_stylus.changed = true;
}
}

ReportEvents(EventTimeForNow());
}

float TouchEventConverterEvdev::ScalePressure(int32_t value) {
float pressure = value - pressure_min_;
if (pressure_max_ - pressure_min_)
Expand Down
1 change: 0 additions & 1 deletion ui/events/ozone/evdev/touch_event_converter_evdev.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ class EVENTS_OZONE_EVDEV_EXPORT TouchEventConverterEvdev

void UpdateTrackingId(int slot, int tracking_id);
void ReleaseTouches();
void ReleaseButtons();
void CancelAllTouches();
bool IsPalm(const InProgressTouchEvdev& touch);
// Normalize pressure value to [0, 1].
Expand Down

0 comments on commit c642ba4

Please sign in to comment.