Move ClientSession's input logic into separate components.

remoting/protocol/input_event_tracker.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/protocol/input_event_tracker.h"
+
+#include "base/logging.h"
+#include "remoting/proto/event.pb.h"
+
+namespace remoting {
+namespace protocol {
+
+InputEventTracker::InputEventTracker(InputStub* input_stub)
+    : input_stub_(input_stub),
+      mouse_pos_(SkIPoint::Make(0, 0)),
+      mouse_button_state_(0) {
+}
+
+InputEventTracker::~InputEventTracker() {
+}
+
+void InputEventTracker::InjectKeyEvent(const KeyEvent& event) {
+  DCHECK(event.has_pressed());
+  DCHECK(event.has_keycode());

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

Since this event comes from a remote source, I think the check should be more
rigourous than a DCHECK, which won't do anything in a release build. Closing the
connection is probably the right thing to do, but failing that, we should ignore
the event. It can be a separate CL though, and InputFilter might be a better
place for it (or even a separate InputValidator).

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

Good spot.

I prefer to test for fields where they are used, rather than relying on
pre-filtering.  Will address this in a follow-up CL.

+  if (event.pressed()) {
+    pressed_keys_.insert(event.keycode());
+  } else {
+    pressed_keys_.erase(event.keycode());
+  }
+  input_stub_->InjectKeyEvent(event);
+}
+
+void InputEventTracker::InjectMouseEvent(const MouseEvent& event) {
+  if (event.has_x() && event.has_y()) {
+    mouse_pos_ = SkIPoint::Make(event.x(), event.y());
+  }
+  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()) {
+        mouse_button_state_ |= button_change;
+      } else {
+        mouse_button_state_ &= ~button_change;
+      }
+    }
+  }
+  input_stub_->InjectMouseEvent(event);
+}
+
+void InputEventTracker::ReleaseAll() {
+  std::set<int>::iterator i;
+  for (i = pressed_keys_.begin(); i != pressed_keys_.end(); ++i) {
+    KeyEvent event;
+    event.set_keycode(*i);
+    event.set_pressed(false);
+    input_stub_->InjectKeyEvent(event);
+  }
+  pressed_keys_.clear();
+
+  for (int i = 1; i < MouseEvent::BUTTON_MAX; i++) {
+    if (mouse_button_state_ & (1 << (i - 1))) {
+      MouseEvent mouse;
+
+      // TODO(wez): EventInjectors should cope with positionless events by
+      // using the current cursor position, and we wouldn't set position here.

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

I don't think I agree with this. Having the input sanitization in one
cross-platform place is a good thing, surely? Whether or not this is that place
is another question, but I don't think the EventInjectors should have to cope
with it.

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

This isn't input sanitization.  When a client disconnects with buttons down, or
the host-side blocks remote inputs, we release buttons.  This code forces us to
release them at the position of the last remote movememnt, rather than wherever
the cursor is locally.

We do this to work around the platform-specific code having a vaguely-specified
requirement for positional fields, which is detail specific to the EventInjector
implementations.  "Sanitizing" events elsewhere, it's not clear what constitutes
a "sane" event.

+      mouse.set_x(mouse_pos_.x());
+      mouse.set_y(mouse_pos_.y());
+
+      mouse.set_button((MouseEvent::MouseButton)i);
+      mouse.set_button_down(false);
+      input_stub_->InjectMouseEvent(mouse);
+    }
+  }
+  mouse_button_state_ = 0;
+}
+
+}  // namespace protocol
+}  // namespace remoting