Allow input & clipboard filters to be disabled without NULLing target stub.

This CL also:
* Adds unit tests for InputFilter and CLipboardFilter.
* Updates ClientSession to enabled/disable filters for authentication.

BUG=118511


Review URL: https://chromiumcodereview.appspot.com/10860033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152418 0039d316-1c4b-4281-b951-d872f2087c98

remoting/host/client_session.cc

@@ -25,12 +25,15 @@ ClientSession::ClientSession(
     : event_handler_(event_handler),
       connection_(connection.Pass()),
       client_jid_(connection_->session()->jid()),
-      is_authenticated_(false),
       host_clipboard_stub_(host_clipboard_stub),
       host_input_stub_(host_input_stub),
       input_tracker_(host_input_stub_),
       remote_input_filter_(&input_tracker_),
       mouse_input_filter_(&remote_input_filter_),
+      disable_input_filter_(&mouse_input_filter_),
+      disable_clipboard_filter_(clipboard_echo_filter_.host_filter()),
+      auth_input_filter_(&disable_input_filter_),
+      auth_clipboard_filter_(&disable_clipboard_filter_),
       client_clipboard_factory_(clipboard_echo_filter_.client_filter()),
       capturer_(capturer),
       max_duration_(max_duration) {
@@ -43,6 +46,10 @@ ClientSession::ClientSession(
   connection_->set_host_stub(this);
   connection_->set_input_stub(this);
   clipboard_echo_filter_.set_host_stub(host_clipboard_stub_);
+
+  // |auth_*_filter_|'s states reflect whether the session is authenticated.
+  auth_input_filter_.set_enabled(false);
+  auth_clipboard_filter_.set_enabled(false);
 }
 
 ClientSession::~ClientSession() {
@@ -86,9 +93,8 @@ void ClientSession::OnConnectionAuthenticated(
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
 
-  is_authenticated_ = true;
-  auth_input_filter_.set_input_stub(&disable_input_filter_);
-  auth_clipboard_filter_.set_clipboard_stub(&disable_clipboard_filter_);
+  auth_input_filter_.set_enabled(true);
+  auth_clipboard_filter_.set_enabled(true);
 
   clipboard_echo_filter_.set_client_stub(connection_->client_stub());
 
@@ -115,10 +121,15 @@ void ClientSession::OnConnectionClosed(
     protocol::ErrorCode error) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
-  if (!is_authenticated_)
+
+  if (!auth_input_filter_.enabled())
     event_handler_->OnSessionAuthenticationFailed(this);
-  auth_input_filter_.set_input_stub(NULL);
-  auth_clipboard_filter_.set_clipboard_stub(NULL);
+
+  // Block any further input events from the client.
+  // TODO(wez): Fix ChromotingHost::OnSessionClosed not to check our
+  // is_authenticated(), so that we can disable |auth_*_filter_| here.
+  disable_input_filter_.set_enabled(false);
+  disable_clipboard_filter_.set_enabled(false);
 
   // Ensure that any pressed keys or buttons are released.
   input_tracker_.ReleaseAll();
@@ -161,15 +172,11 @@ void ClientSession::LocalMouseMoved(const SkIPoint& mouse_pos) {
 void ClientSession::SetDisableInputs(bool disable_inputs) {
   DCHECK(CalledOnValidThread());
 
-  if (disable_inputs) {
-    disable_input_filter_.set_input_stub(NULL);
-    disable_clipboard_filter_.set_clipboard_stub(NULL);
+  if (disable_inputs)
     input_tracker_.ReleaseAll();
-  } else {
-    disable_input_filter_.set_input_stub(&mouse_input_filter_);
-    disable_clipboard_filter_.set_clipboard_stub(
-        clipboard_echo_filter_.host_filter());
-  }
+
+  disable_input_filter_.set_enabled(!disable_inputs);
+  disable_clipboard_filter_.set_enabled(!disable_inputs);
 }
 
 scoped_ptr<protocol::ClipboardStub> ClientSession::CreateClipboardProxy() {

remoting/host/client_session.h

@@ -110,7 +110,7 @@ class ClientSession : public protocol::HostStub,
 
   const std::string& client_jid() { return client_jid_; }
 
-  bool is_authenticated() { return is_authenticated_;  }
+  bool is_authenticated() { return auth_input_filter_.enabled();  }
 
   // Indicate that local mouse activity has been detected. This causes remote
   // inputs to be ignored for a short time so that the local user will always
@@ -131,7 +131,6 @@ class ClientSession : public protocol::HostStub,
   scoped_ptr<protocol::ConnectionToClient> connection_;
 
   std::string client_jid_;
-  bool is_authenticated_;
 
   // The host clipboard and input stubs to which this object delegates.
   // These are the final elements in the clipboard & input pipelines, which
@@ -148,6 +147,10 @@ class ClientSession : public protocol::HostStub,
   // Filter used to clamp mouse events to the current display dimensions.
   protocol::MouseInputFilter mouse_input_filter_;
 
+  // Filter to used to stop clipboard items sent from the client being echoed
+  // back to it.
+  protocol::ClipboardEchoFilter clipboard_echo_filter_;
+
   // Filters used to manage enabling & disabling of input & clipboard.
   protocol::InputFilter disable_input_filter_;
   protocol::ClipboardFilter disable_clipboard_filter_;
@@ -156,10 +159,6 @@ class ClientSession : public protocol::HostStub,
   protocol::InputFilter auth_input_filter_;
   protocol::ClipboardFilter auth_clipboard_filter_;
 
-  // Filter to used to stop clipboard items sent from the client being echoed
-  // back to it.
-  protocol::ClipboardEchoFilter clipboard_echo_filter_;
-
   // Factory for weak pointers to the client clipboard stub.
   // This must appear after |clipboard_echo_filter_|, so that it won't outlive
   // it.

remoting/protocol/clipboard_filter.cc

@@ -8,7 +8,11 @@
 namespace remoting {
 namespace protocol {
 
-ClipboardFilter::ClipboardFilter() : clipboard_stub_(NULL) {
+ClipboardFilter::ClipboardFilter() : clipboard_stub_(NULL), enabled_(true) {
+}
+
+ClipboardFilter::ClipboardFilter(ClipboardStub* clipboard_stub)
+    : clipboard_stub_(clipboard_stub), enabled_(true) {
 }
 
 ClipboardFilter::~ClipboardFilter() {
@@ -19,7 +23,7 @@ void ClipboardFilter::set_clipboard_stub(ClipboardStub* clipboard_stub) {
 }
 
 void ClipboardFilter::InjectClipboardEvent(const ClipboardEvent& event) {
-  if (clipboard_stub_ != NULL)
+  if (enabled_ && clipboard_stub_ != NULL)
     clipboard_stub_->InjectClipboardEvent(event);
 }
 

remoting/protocol/clipboard_filter.h

@@ -12,21 +12,28 @@
 namespace remoting {
 namespace protocol {
 
-// Forwards clipboard events to |clipboard_stub|, iff |clipboard_stub| is not
-// NULL.
+// Forwards clipboard events to |clipboard_stub|, if configured.  Event
+// forwarding may also be disabled independently of the configured
+// |clipboard_stub|. ClipboardFilters initially have event forwarding enabled.
 class ClipboardFilter : public ClipboardStub {
  public:
   ClipboardFilter();
+  explicit ClipboardFilter(ClipboardStub* clipboard_stub);
   virtual ~ClipboardFilter();
 
   // Set the ClipboardStub that events will be forwarded to.
   void set_clipboard_stub(ClipboardStub* clipboard_stub);
 
+  // Enable/disable forwarding of clipboard events to the ClipboardStub.
+  void set_enabled(bool enabled) { enabled_ = enabled; }
+  bool enabled() const { return enabled_; }
+
   // ClipboardStub interface.
   virtual void InjectClipboardEvent(const ClipboardEvent& event) OVERRIDE;
 
  private:
   ClipboardStub* clipboard_stub_;
+  bool enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(ClipboardFilter);
 };

remoting/protocol/clipboard_filter_unittest.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/clipboard_filter.h"
+
+#include "remoting/proto/event.pb.h"
+#include "remoting/protocol/protocol_mock_objects.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+
+namespace remoting {
+namespace protocol {
+
+MATCHER_P2(EqualsClipboardEvent, mime_type, data, "") {
+  return arg.mime_type() == mime_type && arg.data() == data;
+}
+
+static ClipboardEvent MakeClipboardEvent(const std::string& mime_type,
+                                         const std::string& data) {
+  ClipboardEvent event;
+  event.set_mime_type(mime_type);
+  event.set_data(data);
+  return event;
+}
+
+// Verify that the filter passes events on correctly to a configured stub.
+TEST(ClipboardFilterTest, EventsPassThroughFilter) {
+  MockClipboardStub clipboard_stub;
+  ClipboardFilter clipboard_filter(&clipboard_stub);
+
+  EXPECT_CALL(clipboard_stub,
+      InjectClipboardEvent(EqualsClipboardEvent("text", "foo")));
+
+  clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text","foo"));
+}
+
+// Verify that the filter ignores events if disabled.
+TEST(ClipboardFilterTest, IgnoreEventsIfDisabled) {
+  MockClipboardStub clipboard_stub;
+  ClipboardFilter clipboard_filter(&clipboard_stub);
+
+  clipboard_filter.set_enabled(false);
+
+  EXPECT_CALL(clipboard_stub, InjectClipboardEvent(_)).Times(0);
+
+  clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text","foo"));
+}
+
+// Verify that the filter ignores events if not configured.
+TEST(ClipboardFilterTest, IgnoreEventsIfNotConfigured) {
+  ClipboardFilter clipboard_filter;
+
+  clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text","foo"));
+}
+
+} // namespace protocol
+} // namespace remoting

remoting/protocol/input_filter.cc

@@ -7,22 +7,23 @@
 namespace remoting {
 namespace protocol {
 
-InputFilter::InputFilter() : input_stub_(NULL) {
+InputFilter::InputFilter() : input_stub_(NULL), enabled_(true) {
 }
 
-InputFilter::InputFilter(InputStub* input_stub) : input_stub_(input_stub) {
+InputFilter::InputFilter(InputStub* input_stub)
+  : input_stub_(input_stub), enabled_(true) {
 }
 
 InputFilter::~InputFilter() {
 }
 
 void InputFilter::InjectKeyEvent(const KeyEvent& event) {
-  if (input_stub_ != NULL)
+  if (enabled_ && input_stub_ != NULL)
     input_stub_->InjectKeyEvent(event);
 }
 
 void InputFilter::InjectMouseEvent(const MouseEvent& event) {
-  if (input_stub_ != NULL)
+  if (enabled_ && input_stub_ != NULL)
     input_stub_->InjectMouseEvent(event);
 }
 

remoting/protocol/input_filter.h

@@ -12,27 +12,35 @@
 namespace remoting {
 namespace protocol {
 
-// Forwards input events to |input_stub|, iif |input_stub| is not NULL.
+// Forwards input events to |input_stub|, if configured.  Input forwarding may
+// also be disabled independently of the |input_stub| being set.  InputFilters
+// initially have input forwarding enabled.
 class InputFilter : public InputStub {
  public:
   InputFilter();
   explicit InputFilter(InputStub* input_stub);
   virtual ~InputFilter();
 
-  // Return the InputStub that events will be forwarded to.
-  InputStub* input_stub() { return input_stub_; }
-
   // Set the InputStub that events will be forwarded to.
   void set_input_stub(InputStub* input_stub) {
     input_stub_ = input_stub;
   }
 
+  // Enable/disable routing of events to the InputStub.
+  void set_enabled(bool enabled) {
+    enabled_ = enabled;
+  }
+  bool enabled() const {
+    return enabled_;
+  }
+
   // InputStub interface.
   virtual void InjectKeyEvent(const KeyEvent& event) OVERRIDE;
   virtual void InjectMouseEvent(const MouseEvent& event) OVERRIDE;
 
  private:
   InputStub* input_stub_;
+  bool enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(InputFilter);
 };

remoting/protocol/input_filter_unittest.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_filter.h"
+
+#include "remoting/proto/event.pb.h"
+#include "remoting/protocol/protocol_mock_objects.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+
+namespace remoting {
+namespace protocol {
+
+MATCHER_P2(EqualsKeyEvent, usb_keycode, pressed, "") {
+  return arg.usb_keycode() == static_cast<uint32>(usb_keycode) &&
+         arg.pressed() == pressed;
+}
+
+MATCHER_P2(EqualsMouseMoveEvent, x, y, "") {
+  return arg.x() == x && arg.y() == y;
+}
+
+static KeyEvent NewKeyEvent(uint32 usb_keycode, bool pressed) {
+  KeyEvent event;
+  event.set_usb_keycode(usb_keycode);
+  event.set_pressed(pressed);
+  return event;
+}
+
+static MouseEvent MouseMoveEvent(int x, int y) {
+  MouseEvent event;
+  event.set_x(x);
+  event.set_y(y);
+  return event;
+}
+
+static void InjectTestSequence(protocol::InputStub* input_stub) {
+  // Inject a key event.
+  input_stub->InjectKeyEvent(NewKeyEvent(0, true));
+  input_stub->InjectKeyEvent(NewKeyEvent(0, false));
+
+  // Inject mouse movemement.
+  input_stub->InjectMouseEvent(MouseMoveEvent(10, 20));
+}
+
+// Verify that the filter passes events on correctly to a configured stub.
+TEST(InputFilterTest, EventsPassThroughFilter) {
+  MockInputStub input_stub;
+  InputFilter input_filter(&input_stub);
+
+  EXPECT_CALL(input_stub, InjectKeyEvent(EqualsKeyEvent(0, true)));
+  EXPECT_CALL(input_stub, InjectKeyEvent(EqualsKeyEvent(0, false)));
+  EXPECT_CALL(input_stub, InjectMouseEvent(EqualsMouseMoveEvent(10, 20)));
+
+  InjectTestSequence(&input_filter);
+}
+
+// Verify that the filter ignores events if disabled.
+TEST(InputFilterTest, IgnoreEventsIfDisabled) {
+  MockInputStub input_stub;
+  InputFilter input_filter(&input_stub);
+
+  input_filter.set_enabled(false);
+
+  EXPECT_CALL(input_stub, InjectKeyEvent(_)).Times(0);
+  EXPECT_CALL(input_stub, InjectMouseEvent(_)).Times(0);
+
+  InjectTestSequence(&input_filter);
+}
+
+// Verify that the filter ignores events if not configured.
+TEST(InputFilterTest, IgnoreEventsIfNotConfigured) {
+  InputFilter input_filter;
+
+  InjectTestSequence(&input_filter);
+}
+
+} // namespace protocol
+} // namespace remoting

remoting/remoting.gyp

@@ -1766,6 +1766,7 @@
         'protocol/buffered_socket_writer_unittest.cc',
         'protocol/channel_multiplexer_unittest.cc',
         'protocol/clipboard_echo_filter_unittest.cc',
+        'protocol/clipboard_filter_unittest.cc',
         'protocol/connection_tester.cc',
         'protocol/connection_tester.h',
         'protocol/connection_to_client_unittest.cc',
@@ -1774,9 +1775,10 @@
         'protocol/fake_authenticator.h',
         'protocol/fake_session.cc',
         'protocol/fake_session.h',
+        'protocol/input_event_tracker_unittest.cc',
+        'protocol/input_filter_unittest.cc',
         'protocol/jingle_messages_unittest.cc',
         'protocol/jingle_session_unittest.cc',
-        'protocol/input_event_tracker_unittest.cc',
         'protocol/message_decoder_unittest.cc',
         'protocol/message_reader_unittest.cc',
         'protocol/mouse_input_filter_unittest.cc',