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}

ui/ozone/platform/wayland/host/wayland_keyboard.cc

@@ -159,12 +159,25 @@ 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);
 
-  keyboard->auto_repeat_handler_.SetAutoRepeatRate(
-      base::TimeDelta::FromMilliseconds(delay),
-      base::TimeDelta::FromMilliseconds(rate));
+  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));
+  }
 }
 
 void WaylandKeyboard::FlushInput(base::OnceClosure closure) {

ui/ozone/platform/wayland/host/wayland_keyboard_unittest.cc

@@ -25,10 +25,14 @@
 #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() {}
@@ -75,6 +79,23 @@ 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.
@@ -353,36 +374,37 @@ TEST_P(WaylandKeyboardTest, EventAutoRepeat) {
                          &empty);
   wl_array_release(&empty);
 
-  // Auto repeat info in ms.
-  uint32_t rate = 75;
-  uint32_t delay = 25;
+  constexpr int32_t rate = 5;    // num key events per second.
+  constexpr int32_t delay = 25;  // in milliseconds.
 
   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);
-
-  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 doubled the rate time, to ensure events get fired.
-    mock_time_task_runner->FastForwardBy(
-        base::TimeDelta::FromMilliseconds(rate * 2));
-  }
-
+  // First key repeat event happens after |delay|.
+  EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(delay) +
+                                  kTimeError);
   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);
+  Sync();
+  SyncDisplay();
+  ::testing::Mock::VerifyAndClearExpectations(&delegate_);
 
-  std::unique_ptr<Event> event2;
-  EXPECT_CALL(delegate_, DispatchEvent(_)).WillRepeatedly(CloneEvent(&event2));
-
+  // Then release.
   wl_keyboard_send_key(keyboard_->resource(), 3, 0, 30 /* a */,
                        WL_KEYBOARD_KEY_STATE_RELEASED);
   Sync();
@@ -396,88 +418,39 @@ TEST_P(WaylandKeyboardTest, NoEventAutoRepeatOnLeave) {
                          &empty);
   wl_array_release(&empty);
 
-  // Auto repeat info in ms.
-  uint32_t rate = 75;
-  uint32_t delay = 25;
+  constexpr int32_t rate = 5;    // num key events per second.
+  constexpr int32_t delay = 25;  // in milliseconds.
 
   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);
-
-  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 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());
-
+  // Check first key repeating event.
+  EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(1);
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(delay) +
+                                  kTimeError);
   Sync();
+  SyncDisplay();
+  ::testing::Mock::VerifyAndClearExpectations(&delegate_);
 
-  EXPECT_CALL(delegate_, DispatchEvent(_)).Times(0);
-
-  wl_keyboard_send_key(keyboard_->resource(), 4, 0, 30 /* a */,
-                       WL_KEYBOARD_KEY_STATE_RELEASED);
+  // Then leave.
+  wl_keyboard_send_leave(keyboard_->resource(), 3, surface_->resource());
   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));
 
+  // After that, no key repeat events are expected.
+  EXPECT_CALL(delegate_, DispatchEvent(NotNull())).Times(0);
+  task_environment_.FastForwardBy(
+      base::TimeDelta::FromMilliseconds(1000 / rate) + kTimeError);
   Sync();
-
-  {
-    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));
-  }
+  ::testing::Mock::VerifyAndClearExpectations(&delegate_);
 
   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,