[Remoting Host] Handle text event characters that are not presented on the keyboard

remoting/host/linux/x11_character_injector.cc

Index: remoting/host/linux/x11_character_injector.cc
diff --git a/remoting/host/linux/x11_character_injector.cc b/remoting/host/linux/x11_character_injector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e485dac24811260c99231a1fbb86e867d7818b99
--- /dev/null
+++ b/remoting/host/linux/x11_character_injector.cc
@@ -0,0 +1,157 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/linux/x11_character_injector.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "remoting/host/linux/x11_keyboard.h"
+
+namespace {
+
+constexpr base::TimeDelta kMappingExpireDuration =
+    base::TimeDelta::FromMilliseconds(200);
+
+}  // namespace
+
+namespace remoting {
+
+struct X11CharacterInjector::KeyInfo {
+  uint32_t keycode;
+  base::TimeTicks expire_at;
+
+  bool operator<(const KeyInfo& other) const {
+    // When expire time are the same, prefer larger keycode for higher test
+    // predictability and less impact on more important layout.
+    return expire_at != other.expire_at ? expire_at < other.expire_at
+                                        : keycode > other.keycode;
+  }
+};
+
+struct X11CharacterInjector::MapResult {
+  bool success;
+
+  uint32_t keycode;
+  uint32_t modifiers;
+
+  // If success == false and |retry_after| is not zero, user may retry
+  // AddNewCharacter() after |retry_after| has elapsed.
+  base::TimeDelta retry_after;
+};
+
+X11CharacterInjector::X11CharacterInjector(
+    std::unique_ptr<X11Keyboard> keyboard)
+    : keyboard_(std::move(keyboard)), weak_factory_(this) {
+  std::vector<uint32_t> keycodes = keyboard_->GetUnusedKeycodes();
+  for (int keycode : keycodes) {
+    available_keycodes_.push_back({keycode, base::TimeTicks()});
+  }
+  std::sort(available_keycodes_.begin(), available_keycodes_.end());
+}
+
+X11CharacterInjector::~X11CharacterInjector() {
+  // Clear all used key mappings.
+  for (const KeyInfo& info : available_keycodes_) {
+    if (!info.expire_at.is_null()) {
+      keyboard_->ChangeKeyMapping(info.keycode, 0);
+    }
+  }
+  keyboard_->Sync();
+}
+
+void X11CharacterInjector::Inject(uint32_t code_point) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  characters_queue_.push(code_point);
+  Schedule(base::TimeDelta());
+}
+
+void X11CharacterInjector::Schedule(base::TimeDelta delay) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  base::TimeTicks expected_consume_time = base::TimeTicks::Now() + delay;
+  if (consume_timer_.IsRunning() &&
+      scheduled_consume_time_ > expected_consume_time) {

[Sergey Ulanov, 2016/09/27 18:40:48]

I don't think you really need this condition. Timer is always scheduled for the
earliest moment when we will have a key is expected to be available. So this
function could just check that the consume_timer_ is scheduled or not, if it is
then it can just return. No need to compare scheduled compute time. I don't
think there is a case when the timer needs to be moved forward.

[Yuwei, 2016/09/27 22:44:34]

My concern was if there is an upcoming Consume() scheduled by Inject() then it
should be delayed. I think you are right. This won't happen since only one
Consume() can be scheduled at a time.

[Yuwei, 2016/09/28 18:45:55]

Removed.

+    return;
+  }
+  consume_timer_.Start(FROM_HERE, delay, this, &X11CharacterInjector::Consume);
+  scheduled_consume_time_ = expected_consume_time;

[Sergey Ulanov, 2016/09/27 18:40:48]

OneShotTimer::desired_run_time() can be used instead of
scheduled_consumer_time_, but you don't really need either of them - see my
comment above.

[Yuwei, 2016/09/28 18:45:55]

Acknowledged.

+}
+
+void X11CharacterInjector::Consume() {

[Sergey Ulanov, 2016/09/27 18:40:48]

Maybe call this function DoInject()?

[Yuwei, 2016/09/28 18:45:55]

Done.

+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!scheduled_consume_time_.is_null() &&
+         base::TimeTicks::Now() >= scheduled_consume_time_);
+  scheduled_consume_time_ = base::TimeTicks();
+  while (!characters_queue_.empty()) {
+    uint32_t code_point = characters_queue_.front();
+    MapResult result = MapCharacter(code_point);
+    if (!result.success) {
+      if (result.retry_after.is_zero()) {
+        continue;

[Sergey Ulanov, 2016/09/27 18:40:48]

This doesn't look right. If MapCharacter() fails to map a character this
function would loop indefinitely. Move character_queue_.pop() above the
MapCharacter() call?

[Yuwei, 2016/09/27 22:44:34]

Yep. You just caught a bug :P

[Yuwei, 2016/09/28 18:45:55]

Fixed this by adding a pop before continue. Moving pop above MapCharacter
doesn't quite work since the schedule-and-break route should not pop the
character...

+      }
+      Schedule(result.retry_after);
+      break;
+    }
+    keyboard_->PressKey(result.keycode, result.modifiers);
+
+    characters_queue_.pop();
+  }
+
+  keyboard_->Flush();
+}
+
+X11CharacterInjector::MapResult X11CharacterInjector::MapCharacter(
+    uint32_t code_point) {
+  MapResult result{false, 0, 0, base::TimeDelta()};
+  if (keyboard_->FindKeycode(code_point, &result.keycode, &result.modifiers)) {
+    uint32_t keycode = result.keycode;
+    auto position = std::find_if(
+        available_keycodes_.begin(), available_keycodes_.end(),
+        [keycode](const KeyInfo& info) { return info.keycode == keycode; });
+    if (position != available_keycodes_.end()) {
+      ResetKeyInfoExpirationTime(position);
+    }
+    result.success = true;
+    return result;
+  }
+
+  if (available_keycodes_.empty()) {
+    return result;
+  }
+
+  KeyInfo& info = available_keycodes_.front();
+
+  base::TimeTicks now = base::TimeTicks::Now();
+  if (info.expire_at > now) {
+    result.retry_after = info.expire_at - now;
+    return result;
+  }
+
+  if (!keyboard_->ChangeKeyMapping(info.keycode, code_point)) {
+    return result;
+  }
+
+  result.success = true;
+  result.keycode = info.keycode;
+
+  // Modifiers can always be 0 since |code_point| is mapped to both upper and
+  // lower case.
+
+  ResetKeyInfoExpirationTime(available_keycodes_.begin());
+
+  keyboard_->Sync();
+  return result;
+}
+
+void X11CharacterInjector::ResetKeyInfoExpirationTime(

[Sergey Ulanov, 2016/09/27 18:40:48]

Also pass base::TimeTicks::Now() value to avoid calling Now() multiple times
when MapCharacter() is executed.

[Yuwei, 2016/09/28 18:45:55]

Done.

+    std::vector<KeyInfo>::iterator position) {
+  position->expire_at = base::TimeTicks::Now() + kMappingExpireDuration;
+  while (position + 1 < available_keycodes_.end() &&

[Sergey Ulanov, 2016/09/27 18:40:48]

Shouldn't this always move the key to the end of the queue
(kMappingExpireDuration is the same for all keys and Now() increasing)? i.e. you
just need to erase() it from the current position and then push_back() to the
end?

[Yuwei, 2016/09/28 18:45:55]

Mostly yes. The only case it doesn't work is when multiple KeyInfo's have the
same expiry time, then the one with lower keycode should be the last.

I think the preference of higher keycode is barely useful (as long as
GetUnusedKeycode returns the list in descending order) so I'll just remove that
field from the sorting criteria.
Yep. Done.

+         *(position + 1) < *position) {
+    std::swap(*position, *(position + 1));
+    position++;
+  }
+}
+
+}  // namespace remoting