Move ClientSession's input logic into separate components.

remoting/host/client_session.cc

 // Copyright (c) 2012 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/client_session.h"
 
 #include <algorithm>
 
 #include "base/message_loop_proxy.h"
 #include "remoting/host/capturer.h"
@@ skipping 15 matching lines @@
 using protocol::MouseEvent;
 
 ClientSession::ClientSession(
     EventHandler* event_handler,
     protocol::ConnectionToClient* connection,
     protocol::InputStub* input_stub,
     Capturer* capturer)
     : 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),
       remote_mouse_button_state_(0) {
   connection_->SetEventHandler(this);
 
   // TODO(sergeyu): Currently ConnectionToClient expects stubs to be
   // set before channels are connected. Make it possible to set stubs
   // later and set them only when connection is authenticated.
   connection_->set_host_stub(this);
   connection_->set_input_stub(this);
 }
 
 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());
+  if (!authenticated_)
+    return;
+  if (ShouldIgnoreRemoteMouseInput(event))
+    return;
 
-      // 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;
+  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());
 
-      injected_mouse_positions_.push_back(pos);
-      if (injected_mouse_positions_.size() > kNumRemoteMousePositions) {
-        VLOG(1) << "Injected mouse positions queue full.";
-        injected_mouse_positions_.pop_front();
-      }
+    // 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(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
   authenticated_ = true;
   event_handler_->OnSessionAuthenticated(this);
 }
 
@@ skipping 91 matching lines @@
 }
 
 bool ClientSession::ShouldIgnoreRemoteKeyboardInput(
     const KeyEvent& event) const {
   DCHECK(CalledOnValidThread());
 
   // If the host user has not yet approved the continuation of the connection,
   // 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());
 
   if (event.has_button() && event.has_button_down()) {
     // Button values are defined in remoting/proto/event.proto.
     if (event.button() >= 1 && event.button() < MouseEvent::BUTTON_MAX) {
       uint32 button_change = 1 << (event.button() - 1);
       if (event.button_down()) {
         remote_mouse_button_state_ |= button_change;
       } else {
         remote_mouse_button_state_ &= ~button_change;
       }
     }
   }
 }
 
 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++) {
     if (remote_mouse_button_state_ & (1 << (i - 1))) {
       MouseEvent mouse;
       // TODO(wez): Shouldn't [need to] set position here.
       mouse.set_x(remote_mouse_pos_.x());
       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;
 }
 
 }  // namespace remoting