Move ClientSession's input logic into separate components.

remoting/host/client_session.cc

Index: remoting/host/client_session.cc
diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 7989e094a37456371486e8b3c40ab97bc8186d25..ca13eeb32b241344fbe51c75a70d94a28575a730 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -33,7 +33,7 @@ ClientSession::ClientSession(
     : event_handler_(event_handler),
       connection_(connection),
       client_jid_(connection->session()->jid()),
-      input_stub_(input_stub),
+      key_tracker_(input_stub),
       capturer_(capturer),
       authenticated_(false),
       awaiting_continue_approval_(false),
@@ -53,43 +53,48 @@ ClientSession::~ClientSession() {
 void ClientSession::InjectKeyEvent(const KeyEvent& event) {
   DCHECK(CalledOnValidThread());
 
-  if (authenticated_ && !ShouldIgnoreRemoteKeyboardInput(event)) {
-    RecordKeyEvent(event);
-    input_stub_->InjectKeyEvent(event);
-  }
+  if (!authenticated_)
+    return;
+  if (ShouldIgnoreRemoteKeyboardInput(event))
+    return;
+
+  key_tracker_.InjectKeyEvent(event);
 }
 
 void ClientSession::InjectMouseEvent(const MouseEvent& event) {
   DCHECK(CalledOnValidThread());
 
-  if (authenticated_ && !ShouldIgnoreRemoteMouseInput(event)) {
-    RecordMouseButtonState(event);
-    MouseEvent event_to_inject = event;
-    if (event.has_x() && event.has_y()) {
-      // In case the client sends events with off-screen coordinates, modify
-      // the event to lie within the current screen area.  This is better than
-      // simply discarding the event, which might lose a button-up event at the
-      // end of a drag'n'drop (or cause other related problems).
-      SkIPoint pos(SkIPoint::Make(event.x(), event.y()));
-      const SkISize& screen = capturer_->size_most_recent();
-      pos.setX(std::max(0, std::min(screen.width() - 1, pos.x())));
-      pos.setY(std::max(0, std::min(screen.height() - 1, pos.y())));
-      event_to_inject.set_x(pos.x());
-      event_to_inject.set_y(pos.y());
-
-      // Record the mouse position so we can use it if we need to inject
-      // fake mouse button events. Note that we need to do this after we
-      // clamp the values to the screen area.
-      remote_mouse_pos_ = pos;
-
-      injected_mouse_positions_.push_back(pos);
-      if (injected_mouse_positions_.size() > kNumRemoteMousePositions) {
-        VLOG(1) << "Injected mouse positions queue full.";
-        injected_mouse_positions_.pop_front();
-      }
+  if (!authenticated_)
+    return;
+  if (ShouldIgnoreRemoteMouseInput(event))
+    return;
+
+  RecordMouseButtonState(event);
+  MouseEvent event_to_inject = event;
+  if (event.has_x() && event.has_y()) {
+    // In case the client sends events with off-screen coordinates, modify
+    // the event to lie within the current screen area.  This is better than
+    // simply discarding the event, which might lose a button-up event at the
+    // end of a drag'n'drop (or cause other related problems).
+    SkIPoint pos(SkIPoint::Make(event.x(), event.y()));
+    const SkISize& screen = capturer_->size_most_recent();
+    pos.setX(std::max(0, std::min(screen.width() - 1, pos.x())));
+    pos.setY(std::max(0, std::min(screen.height() - 1, pos.y())));
+    event_to_inject.set_x(pos.x());
+    event_to_inject.set_y(pos.y());
+
+    // Record the mouse position so we can use it if we need to inject
+    // fake mouse button events. Note that we need to do this after we
+    // clamp the values to the screen area.
+    remote_mouse_pos_ = pos;
+
+    injected_mouse_positions_.push_back(pos);
+    if (injected_mouse_positions_.size() > kNumRemoteMousePositions) {
+      VLOG(1) << "Injected mouse positions queue full.";
+      injected_mouse_positions_.pop_front();
     }
-    input_stub_->InjectMouseEvent(event_to_inject);
   }
+  key_tracker_.InjectMouseEvent(event_to_inject);
 }
 
 void ClientSession::OnConnectionOpened(
@@ -201,22 +206,11 @@ bool ClientSession::ShouldIgnoreRemoteKeyboardInput(
   // then all remote keyboard input is ignored, except to release keys that
   // were already pressed.
   if (awaiting_continue_approval_) {
-    return event.pressed() ||
-        (pressed_keys_.find(event.keycode()) == pressed_keys_.end());

[Wez, 2012/02/27 18:59:04]

Removing this actually fixes the client-side repeat suppression issue.

[Jamie, 2012/02/27 19:39:14]

How? This code only runs if the continue dialog is visible.

[Wez, 2012/02/27 20:01:21]

You're right; confused myself by splitting the fix from the cleanup.  Actually,
this CL needs a little more work so let's go with the existing fix as-is and
I'll tidy this up for re-submission.

+    return event.pressed();
   }
   return false;
 }
 
-void ClientSession::RecordKeyEvent(const KeyEvent& event) {
-  DCHECK(CalledOnValidThread());
-
-  if (event.pressed()) {
-    pressed_keys_.insert(event.keycode());
-  } else {
-    pressed_keys_.erase(event.keycode());
-  }
-}
-
 void ClientSession::RecordMouseButtonState(const MouseEvent& event) {
   DCHECK(CalledOnValidThread());
 
@@ -237,14 +231,7 @@ void ClientSession::RestoreEventState() {
   DCHECK(CalledOnValidThread());
 
   // Undo any currently pressed keys.
-  std::set<int>::iterator i;
-  for (i = pressed_keys_.begin(); i != pressed_keys_.end(); ++i) {
-    KeyEvent key;
-    key.set_keycode(*i);
-    key.set_pressed(false);
-    input_stub_->InjectKeyEvent(key);
-  }
-  pressed_keys_.clear();
+  key_tracker_.ReleaseAllKeys();
 
   // Undo any currently pressed mouse buttons.
   for (int i = 1; i < MouseEvent::BUTTON_MAX; i++) {
@@ -255,7 +242,7 @@ void ClientSession::RestoreEventState() {
       mouse.set_y(remote_mouse_pos_.y());
       mouse.set_button((MouseEvent::MouseButton)i);
       mouse.set_button_down(false);
-      input_stub_->InjectMouseEvent(mouse);
+      key_tracker_.InjectMouseEvent(mouse);
     }
   }
   remote_mouse_button_state_ = 0;