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..0a5ee7aef37c3fe96b37f3d901232b86a6388194 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -33,18 +33,15 @@ ClientSession::ClientSession(
     : event_handler_(event_handler),
       connection_(connection),
       client_jid_(connection->session()->jid()),
-      input_stub_(input_stub),
-      capturer_(capturer),
-      authenticated_(false),
-      awaiting_continue_approval_(false),
-      remote_mouse_button_state_(0) {
+      input_tracker_(input_stub),
+      capturer_(capturer) {
   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);
+  connection_->set_input_stub(&input_filter_);
 }
 
 ClientSession::~ClientSession() {
@@ -53,50 +50,45 @@ ClientSession::~ClientSession() {
 void ClientSession::InjectKeyEvent(const KeyEvent& event) {
   DCHECK(CalledOnValidThread());
 
-  if (authenticated_ && !ShouldIgnoreRemoteKeyboardInput(event)) {
-    RecordKeyEvent(event);
-    input_stub_->InjectKeyEvent(event);
-  }
+  if (ShouldIgnoreInput())
+    return;
+
+  input_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;
+
+  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();
     }
-    input_stub_->InjectMouseEvent(event_to_inject);
   }
+  input_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 +131,9 @@ void ClientSession::OnRouteChange(
 void ClientSession::Disconnect() {
   DCHECK(CalledOnValidThread());
   DCHECK(connection_.get());
-  authenticated_ = false;
-  RestoreEventState();
+
+  // SetDisableInputs(true) implicitly restores the input event state.
+  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,93 +165,26 @@ void ClientSession::LocalMouseMoved(const SkIPoint& mouse_pos) {
   }
 }
 
-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_)
-    return true;
-  // Otherwise, ignore remote mouse 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());
+  if (disable_inputs) {
+    input_filter_.set_input_stub(NULL);
+    input_tracker_.ReleaseAll();
   } 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;
-      }
-    }
+    input_filter_.set_input_stub(this);
   }
 }
 
-void ClientSession::RestoreEventState() {
+bool ClientSession::ShouldIgnoreInput() const {
   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();
-
-  // 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);
-    }
-  }
-  remote_mouse_button_state_ = 0;
+  // 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;

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

Presumably you could get rid of ShouldIgnoreInput altogether if the local input
monitor disconnected the input_filter instead of setting
|latest_local_input_time_|. That's fine as a follow-up CL though.

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

This moved into RemoteInputFilter.

 }
 
 }  // namespace remoting