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 6ff6164add558a5ff006bc22d5a38b0f5ea16f77..d7456822dcf846af55a15024efdf147fe6c30819 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -33,10 +33,9 @@ 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),
+      disable_inputs_(true),
       remote_mouse_button_state_(0) {
   connection_->SetEventHandler(this);
 
@@ -53,50 +52,51 @@ ClientSession::~ClientSession() {
 void ClientSession::InjectKeyEvent(const KeyEvent& event) {
   DCHECK(CalledOnValidThread());
 
-  if (authenticated_ && !ShouldIgnoreRemoteKeyboardInput(event)) {
-    RecordKeyEvent(event);
-    input_stub_->InjectKeyEvent(event);
-  }
+  if (ShouldIgnoreInput())
+    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 (ShouldIgnoreInput())
+    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(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
-  authenticated_ = true;
+  SetDisableInputs(false);
   event_handler_->OnSessionAuthenticated(this);
 }
 
@@ -139,8 +139,9 @@ void ClientSession::OnRouteChange(
 void ClientSession::Disconnect() {
   DCHECK(CalledOnValidThread());
   DCHECK(connection_.get());
-  authenticated_ = false;
-  RestoreEventState();
+
+  // SetDisableInputs(true) implicitly restores the input event state.

[Jamie, 2012/03/15 18:38:56]

Are you talking about modifier keys here? This comment isn't clear. Why not just
call ReleaseAll? It feels odd to be disabling inputs on disconnect since you
won't get any after that point.

[Wez, 2012/03/15 22:25:29]

In principle no, we won't.  This is host-side code, and after Disconnect() the
ClientSession should not inject further events, so I'd prefer to keep this extra
protection.

I've tweaked the comment wording to be clearer.

+  SetDisableInputs(true);
 
   // This triggers OnSessionClosed() and the session may be destroyed
   // as the result, so this call must be the last in this method.
@@ -172,20 +173,23 @@ void ClientSession::LocalMouseMoved(const SkIPoint& mouse_pos) {
   }
 }
 
-bool ClientSession::ShouldIgnoreRemoteMouseInput(
-    const protocol::MouseEvent& event) const {
+void ClientSession::SetDisableInputs(bool disable_inputs) {
+  DCHECK(CalledOnValidThread());
+
+  disable_inputs_ = disable_inputs;
+
+  if (disable_inputs_)
+    RestoreEventState();
+}
+
+bool ClientSession::ShouldIgnoreInput() const {
   DCHECK(CalledOnValidThread());
 
-  // If the last remote input event was a click or a drag, then it's not safe
-  // to block remote mouse events. For example, it might result in the host
-  // missing the mouse-up event and being stuck with the button pressed.
-  if (remote_mouse_button_state_ != 0)
-    return false;
-  // Otherwise, if the host user has not yet approved the continuation of the
-  // connection, then ignore remote mouse events.
-  if (awaiting_continue_approval_)
+  // Ignore events if input from this client is disabled.
+  if (disable_inputs_)
     return true;
-  // Otherwise, ignore remote mouse events if the local mouse moved recently.
+
+  // Ignore remote events if the local mouse moved recently.
   int64 millis = (base::Time::Now() - latest_local_input_time_)
                  .InMilliseconds();
   if (millis < kRemoteBlockTimeoutMillis)
@@ -193,30 +197,6 @@ bool ClientSession::ShouldIgnoreRemoteMouseInput(
   return false;
 }
 
-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());
-  }
-  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 +217,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 +228,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;