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),
+      disable_inputs_(true),
       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 (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());
+  if (ShouldIgnoreInput())
+    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;
+  SetDisableInputs(false);
   event_handler_->OnSessionAuthenticated(this);
 }
 
 void ClientSession::OnConnectionClosed(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
   event_handler_->OnSessionClosed(this);
 }
 
@@ skipping 22 matching lines @@
     const net::IPEndPoint& local_end_point) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
   event_handler_->OnSessionRouteChange(this, channel_name, remote_end_point,
                                        local_end_point);
 }
 
 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.
   connection_->Disconnect();
 }
 
 void ClientSession::LocalMouseMoved(const SkIPoint& mouse_pos) {
   DCHECK(CalledOnValidThread());
 
   // If this is a genuine local input event (rather than an echo of a remote
@@ skipping 11 matching lines @@
     // successfully injected (or the local input monitor might have skipped over
     // some positions), and not because the events were out-of-sequence.  These
     // spurious positions should therefore be discarded.
     injected_mouse_positions_.erase(injected_mouse_positions_.begin(),
                                     ++found_position);
   } else {
     latest_local_input_time_ = base::Time::Now();
   }
 }
 
-bool ClientSession::ShouldIgnoreRemoteMouseInput(
-    const protocol::MouseEvent& event) const {
+void ClientSession::SetDisableInputs(bool disable_inputs) {
   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_)
+  disable_inputs_ = disable_inputs;
+
+  if (disable_inputs_)
+    RestoreEventState();
+}
+
+bool ClientSession::ShouldIgnoreInput() const {
+  DCHECK(CalledOnValidThread());
+
+  // 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)
     return true;
   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());
 
   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