Skip to content

GitLab

T
tangled
Commit 6d3cbbce authored by Adithya Srinivasan's avatar Adithya Srinivasan Committed by Commit Bot
Browse files
Options

Revert "Reland "Fix auto key repeat event interpretation."" 

This reverts commit 7b356bed.

Reason for revert: Seems like WaylandKeyboardTest.EventAutoRepeat is failing again in ozone_unittests (https://ci.chromium.org/p/chromium/builders/ci/Linux%20Ozone%20Tester%20%28Headless%29/20279), this seems the likely culprit.

Original change's description:
> Reland "Fix auto key repeat event interpretation."
>
> This is a reland of 7fdaa429
>
> Original change's description:
> > Fix auto key repeat event interpretation.
> >
> > The original code interprets "rate" (the second argument) in
> > millisecond units. However, by definition, it is the number of
> > key events per second.
> >
> > Bug: 1141473
> > Test: Ran on DUT. Ran unittest locally.
> > Change-Id: Iccdcd2c1e5fc3160da054617bc3e173bb23e9657
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507412
> > Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
> > Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#827223}
>
> Bug: 1141473
> Change-Id: I9af00fa55fc77cd931d3e203afd47e3a48e4b826
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537793
> Reviewed-by: Markus Heintz <markusheintz@chromium.org>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Commit-Queue: Markus Heintz <markusheintz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#827287}

TBR=rjkroege@chromium.org,mukai@chromium.org,markusheintz@chromium.org,hidehiko@chromium.org

Change-Id: I55a9cccf78410ee4a2718649de626c0691b0da20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1141473
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537135Reviewed-by: default avatarAdithya Srinivasan <adithyas@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827328}
parent 9751430a
Hide whitespace changes
Inline Side-by-side
Showing with 90 additions and 76 deletions
ui/ozone/platform/wayland/host/wayland_keyboard.cc
View file @ 6d3cbbce
......@@ -159,25 +159,12 @@ void WaylandKeyboard::RepeatInfo(void* data,
wl_keyboard* obj,
int32_t rate,
int32_t delay) {
// Negative values for either rate or delay are illegal.
if (rate < 0 || delay < 0) {
LOG(ERROR) << "Invalid rate or delay: " << rate << ", " << delay;
return;
}
WaylandKeyboard* keyboard = static_cast<WaylandKeyboard*>(data);
DCHECK(keyboard);
if (rate == 0) {
// A rate of zero will disable any repeating (regardless of the value of
// delay).
keyboard->auto_repeat_handler_.SetAutoRepeatEnabled(false);
} else {
keyboard->auto_repeat_handler_.SetAutoRepeatEnabled(true);
// The rate is in characters per second
keyboard->auto_repeat_handler_.SetAutoRepeatRate(
base::TimeDelta::FromMilliseconds(delay),
base::TimeDelta::FromSecondsD(1.0 / rate));
}
keyboard->auto_repeat_handler_.SetAutoRepeatRate(
base::TimeDelta::FromMilliseconds(delay),
base::TimeDelta::FromMilliseconds(rate));
}
void WaylandKeyboard::FlushInput(base::OnceClosure closure) {
......
ui/ozone/platform/wayland/host/wayland_keyboard_unittest.cc
View file @ 6d3cbbce
......@@ -25,14 +25,10 @@
#endif
using ::testing::_;
using ::testing::AtLeast;
using ::testing::NotNull;
using ::testing::SaveArg;
namespace ui {
constexpr base::TimeDelta kTimeError = base::TimeDelta::FromMilliseconds(3);
class WaylandKeyboardTest : public WaylandTest {
public:
WaylandKeyboardTest() {}
......@@ -79,23 +75,6 @@ class WaylandKeyboardTest : public WaylandTest {
protected:
wl::TestKeyboard* keyboard_;
// There may be a pending wl_display_sync event, which is triggered by auto
// key repeat and needs to be processed. Wait for its completion.
void SyncDisplay() {
base::RunLoop run_loop;
wl::Object<wl_callback> sync_callback(
wl_display_sync(connection_->display()));
wl_callback_listener listener = {
[](void* data, struct wl_callback* cb, uint32_t time) {
static_cast<base::RunLoop*>(data)->Quit();
}};
wl_callback_add_listener(sync_callback.get(), &listener, &run_loop);
server_.Resume();
run_loop.Run();
server_.Pause();
}
private:
#if BUILDFLAG(USE_XKBCOMMON)
// The Xkb state used for the keyboard.
......@@ -374,37 +353,36 @@ TEST_P(WaylandKeyboardTest, EventAutoRepeat) {
&empty);
wl_array_release(&empty);
constexpr int32_t rate = 5; // num key events per second.
constexpr int32_t delay = 25; // in milliseconds.
// Auto repeat info in ms.
uint32_t rate = 75;
uint32_t delay = 25;
wl_keyboard_send_repeat_info(keyboard_->resource(), rate, delay);
// Keep the key pressed.
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
wl_keyboard_send_key(keyboard_->resource(), 2, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_PRESSED);
Sync();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
// First key repeat event happens after |delay|.
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(delay) +
kTimeError);
std::unique_ptr<Event> event;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce(CloneEvent(&event));
Sync();
// There may be pending wl_display_sync event, which is triggered by auto
// key repeat and needs to be processed. Wait for its completion.
SyncDisplay();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
// The next key event happens after interval = 1/rate seconds.
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
task_environment_.FastForwardBy(
base::TimeDelta::FromMilliseconds(1000 / rate) + kTimeError);
{
scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner =
new base::TestMockTimeTaskRunner();
base::TestMockTimeTaskRunner::ScopedContext scoped_context(
mock_time_task_runner.get());
// Keep it pressed for doubled the rate time, to ensure events get fired.
mock_time_task_runner->FastForwardBy(
base::TimeDelta::FromMilliseconds(rate * 2));
}
Sync();
SyncDisplay();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
// Then release.
std::unique_ptr<Event> event2;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillRepeatedly(CloneEvent(&event2));
wl_keyboard_send_key(keyboard_->resource(), 3, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_RELEASED);
Sync();
......@@ -418,39 +396,88 @@ TEST_P(WaylandKeyboardTest, NoEventAutoRepeatOnLeave) {
&empty);
wl_array_release(&empty);
constexpr int32_t rate = 5; // num key events per second.
constexpr int32_t delay = 25; // in milliseconds.
// Auto repeat info in ms.
uint32_t rate = 75;
uint32_t delay = 25;
wl_keyboard_send_repeat_info(keyboard_->resource(), rate, delay);
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
wl_keyboard_send_key(keyboard_->resource(), 2, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_PRESSED);
Sync();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
// Check first key repeating event.
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(delay) +
kTimeError);
std::unique_ptr<Event> event;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce(CloneEvent(&event));
Sync();
SyncDisplay();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
// Then leave.
{
scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner =
new base::TestMockTimeTaskRunner();
base::TestMockTimeTaskRunner::ScopedContext scoped_context(
mock_time_task_runner.get());
// Keep it pressed for doubled the rate time, to ensure events get fired.
mock_time_task_runner->FastForwardBy(
base::TimeDelta::FromMilliseconds(rate * 2));
}
wl_keyboard_send_leave(keyboard_->resource(), 3, surface_->resource());
Sync();
// After that, no key repeat events are expected.
EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(0);
task_environment_.FastForwardBy(
base::TimeDelta::FromMilliseconds(1000 / rate) + kTimeError);
EXPECT_CALL(delegate_, DispatchEvent(_)).Times(0);
wl_keyboard_send_key(keyboard_->resource(), 4, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_RELEASED);
Sync();
}
TEST_P(WaylandKeyboardTest, NoEventAutoRepeatBeforeTimeout) {
struct wl_array empty;
wl_array_init(&empty);
wl_keyboard_send_enter(keyboard_->resource(), 1, surface_->resource(),
&empty);
wl_array_release(&empty);
// Auto repeat info in ms.
uint32_t rate = 500;
uint32_t delay = 50;
wl_keyboard_send_repeat_info(keyboard_->resource(), rate, delay);
wl_keyboard_send_key(keyboard_->resource(), 2, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_PRESSED);
std::unique_ptr<Event> event;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce(CloneEvent(&event));
Sync();
::testing::Mock::VerifyAndClearExpectations(&delegate_);
{
scoped_refptr<base::TestMockTimeTaskRunner> mock_time_task_runner =
new base::TestMockTimeTaskRunner();
base::TestMockTimeTaskRunner::ScopedContext scoped_context(
mock_time_task_runner.get());
// Keep it pressed for a fifth of the rate time, and no auto repeat events
// should get dispatched.
mock_time_task_runner->FastForwardBy(
base::TimeDelta::FromMilliseconds(rate / 5));
}
wl_keyboard_send_key(keyboard_->resource(), 4, 0, 30 /* a */,
WL_KEYBOARD_KEY_STATE_RELEASED);
std::unique_ptr<Event> event2;
EXPECT_CALL(delegate_, DispatchEvent(_)).WillOnce(CloneEvent(&event2));
Sync();
ASSERT_TRUE(event2);
ASSERT_TRUE(event2->IsKeyEvent());
auto* key_event2 = event2->AsKeyEvent();
EXPECT_EQ(ET_KEY_RELEASED, key_event2->type());
}
INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
......
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