Commit 50e43d96 authored by spang's avatar spang Committed by Commit bot

ozone: evdev: Make cancellation of repeats on key up more robust

Key releases enqueue a task on KeyboardEvdev that dispatches the release
ui::Event as well as cancels any scheduled repeats for that key. The
release itself is enqueued from the dedicated evdev thread and so can be
expected to arrive in a timely manner.

Unfotunately, due to the way base::Timer works, this is not robust when
the UI thread is very busy. The base::Timer object posts a delayed task,
and upon running checks to see if enough real time has passed (see
Timer::RunScheduledTask). If so, it runs the timer callback immediately.
This is fine, except that base::Timer also reuses scheduled tasks from
previous calls to Start() if they are still in the future (see
Timer::Reset). This can mean the timeout callback runs from a task that
was scheduled before the current timer was started.

The net effect of this is that the timer task may run first even if
(initial timer start + delay) > (key release task timestamp). The message
loop queue itself would have run the tasks in the correct order.

So, if we simply use PostDelayedTask() ourselves, there's no cleverness
in base::Timer to worry about and the tasks run in the needed order.

BUG=473446
TEST=Press Ctrl-W to close a tab with lots of activity (multiple copies
  of gmail loading, etc). Only one tab closes. Also tested with
  simulated jank (1s delay in tab closure).

Review URL: https://codereview.chromium.org/1052273006

Cr-Commit-Position: refs/heads/master@{#324081}
parent d2d3689d
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "ui/events/ozone/evdev/keyboard_evdev.h" #include "ui/events/ozone/evdev/keyboard_evdev.h"
#include "base/single_thread_task_runner.h"
#include "base/thread_task_runner_handle.h"
#include "ui/events/event.h" #include "ui/events/event.h"
#include "ui/events/event_constants.h" #include "ui/events/event_constants.h"
#include "ui/events/event_utils.h" #include "ui/events/event_utils.h"
...@@ -59,7 +61,9 @@ KeyboardEvdev::KeyboardEvdev(EventModifiersEvdev* modifiers, ...@@ -59,7 +61,9 @@ KeyboardEvdev::KeyboardEvdev(EventModifiersEvdev* modifiers,
modifiers_(modifiers), modifiers_(modifiers),
keyboard_layout_engine_(keyboard_layout_engine), keyboard_layout_engine_(keyboard_layout_engine),
repeat_enabled_(true), repeat_enabled_(true),
repeat_key_(KEY_RESERVED) { repeat_key_(KEY_RESERVED),
repeat_sequence_(0),
weak_ptr_factory_(this) {
repeat_delay_ = base::TimeDelta::FromMilliseconds(kRepeatDelayMs); repeat_delay_ = base::TimeDelta::FromMilliseconds(kRepeatDelayMs);
repeat_interval_ = base::TimeDelta::FromMilliseconds(kRepeatIntervalMs); repeat_interval_ = base::TimeDelta::FromMilliseconds(kRepeatIntervalMs);
} }
...@@ -146,31 +150,30 @@ void KeyboardEvdev::UpdateKeyRepeat(unsigned int key, bool down) { ...@@ -146,31 +150,30 @@ void KeyboardEvdev::UpdateKeyRepeat(unsigned int key, bool down) {
void KeyboardEvdev::StartKeyRepeat(unsigned int key) { void KeyboardEvdev::StartKeyRepeat(unsigned int key) {
repeat_key_ = key; repeat_key_ = key;
repeat_delay_timer_.Start( repeat_sequence_++;
FROM_HERE, repeat_delay_,
base::Bind(&KeyboardEvdev::OnRepeatDelayTimeout, base::Unretained(this))); ScheduleKeyRepeat(repeat_delay_);
repeat_interval_timer_.Stop();
} }
void KeyboardEvdev::StopKeyRepeat() { void KeyboardEvdev::StopKeyRepeat() {
repeat_key_ = KEY_RESERVED; repeat_sequence_++;
repeat_delay_timer_.Stop();
repeat_interval_timer_.Stop();
} }
void KeyboardEvdev::OnRepeatDelayTimeout() { void KeyboardEvdev::ScheduleKeyRepeat(const base::TimeDelta& delay) {
DispatchKey(repeat_key_, true /* down */, true /* repeat */, base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
EventTimeForNow()); FROM_HERE, base::Bind(&KeyboardEvdev::OnRepeatTimeout,
weak_ptr_factory_.GetWeakPtr(), repeat_sequence_),
repeat_interval_timer_.Start( delay);
FROM_HERE, repeat_interval_,
base::Bind(&KeyboardEvdev::OnRepeatIntervalTimeout,
base::Unretained(this)));
} }
void KeyboardEvdev::OnRepeatIntervalTimeout() { void KeyboardEvdev::OnRepeatTimeout(unsigned int sequence) {
if (repeat_sequence_ != sequence)
return;
DispatchKey(repeat_key_, true /* down */, true /* repeat */, DispatchKey(repeat_key_, true /* down */, true /* repeat */,
EventTimeForNow()); EventTimeForNow());
ScheduleKeyRepeat(repeat_interval_);
} }
void KeyboardEvdev::DispatchKey(unsigned int key, void KeyboardEvdev::DispatchKey(unsigned int key,
......
...@@ -53,8 +53,8 @@ class EVENTS_OZONE_EVDEV_EXPORT KeyboardEvdev { ...@@ -53,8 +53,8 @@ class EVENTS_OZONE_EVDEV_EXPORT KeyboardEvdev {
void UpdateKeyRepeat(unsigned int key, bool down); void UpdateKeyRepeat(unsigned int key, bool down);
void StartKeyRepeat(unsigned int key); void StartKeyRepeat(unsigned int key);
void StopKeyRepeat(); void StopKeyRepeat();
void OnRepeatDelayTimeout(); void ScheduleKeyRepeat(const base::TimeDelta& delay);
void OnRepeatIntervalTimeout(); void OnRepeatTimeout(unsigned int sequence);
void DispatchKey(unsigned int key, void DispatchKey(unsigned int key,
bool down, bool down,
bool repeat, bool repeat,
...@@ -81,10 +81,11 @@ class EVENTS_OZONE_EVDEV_EXPORT KeyboardEvdev { ...@@ -81,10 +81,11 @@ class EVENTS_OZONE_EVDEV_EXPORT KeyboardEvdev {
// Key repeat state. // Key repeat state.
bool repeat_enabled_; bool repeat_enabled_;
unsigned int repeat_key_; unsigned int repeat_key_;
unsigned int repeat_sequence_;
base::TimeDelta repeat_delay_; base::TimeDelta repeat_delay_;
base::TimeDelta repeat_interval_; base::TimeDelta repeat_interval_;
base::OneShotTimer<KeyboardEvdev> repeat_delay_timer_;
base::RepeatingTimer<KeyboardEvdev> repeat_interval_timer_; base::WeakPtrFactory<KeyboardEvdev> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(KeyboardEvdev); DISALLOW_COPY_AND_ASSIGN(KeyboardEvdev);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment