Use ClipboardFilter in ClientSession to implement auth & disable-input blocking.

This removes the need for ClientSession to implement ClipboardStub
and proxy clipboard events itself.

BUG=118511


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

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

5 files changed, 60 insertions, 49 deletions

diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
index 6e7e3d7..540277e 100644
--- a/remoting/host/client_session.cc
+++ b/remoting/host/client_session.cc
@@ -39,7 +39,7 @@ ClientSession::ClientSession(
   // 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_clipboard_stub(this);
+  connection_->set_clipboard_stub(&auth_clipboard_filter_);
   connection_->set_host_stub(this);
   connection_->set_input_stub(this);
   clipboard_echo_filter_.set_host_stub(host_clipboard_stub_);
@@ -48,20 +48,6 @@ ClientSession::ClientSession(
 ClientSession::~ClientSession() {
 }
 
-void ClientSession::InjectClipboardEvent(
-    const protocol::ClipboardEvent& event) {
-  DCHECK(CalledOnValidThread());
-
-  // TODO(wez): Disable clipboard in both directions on local activity, and
-  // replace these tests with a HostInputFilter (or ClipboardFilter).
-  if (auth_input_filter_.input_stub() == NULL)
-    return;
-  if (disable_input_filter_.input_stub() == NULL)
-    return;
-
-  clipboard_echo_filter_.host_filter()->InjectClipboardEvent(event);
-}
-
 void ClientSession::InjectKeyEvent(const protocol::KeyEvent& event) {
   DCHECK(CalledOnValidThread());
   auth_input_filter_.InjectKeyEvent(event);
@@ -99,15 +85,20 @@ void ClientSession::OnConnectionAuthenticated(
     protocol::ConnectionToClient* connection) {
   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_);
+
   clipboard_echo_filter_.set_client_stub(connection_->client_stub());
+
   if (max_duration_ > base::TimeDelta()) {
     // TODO(simonmorris): Let Disconnect() tell the client that the
     // disconnection was caused by the session exceeding its maximum duration.
     max_duration_timer_.Start(FROM_HERE, max_duration_,
                               this, &ClientSession::Disconnect);
   }
+
   event_handler_->OnSessionAuthenticated(this);
 }
 
@@ -127,6 +118,7 @@ void ClientSession::OnConnectionClosed(
   if (!is_authenticated_)
     event_handler_->OnSessionAuthenticationFailed(this);
   auth_input_filter_.set_input_stub(NULL);
+  auth_clipboard_filter_.set_clipboard_stub(NULL);
 
   // Ensure that any pressed keys or buttons are released.
   input_tracker_.ReleaseAll();
@@ -171,9 +163,12 @@ void ClientSession::SetDisableInputs(bool disable_inputs) {
 
   if (disable_inputs) {
     disable_input_filter_.set_input_stub(NULL);
+    disable_clipboard_filter_.set_clipboard_stub(NULL);
     input_tracker_.ReleaseAll();
   } else {
     disable_input_filter_.set_input_stub(&mouse_input_filter_);
+    disable_clipboard_filter_.set_clipboard_stub(
+        clipboard_echo_filter_.host_filter());
   }
 }
 
diff --git a/remoting/host/client_session.h b/remoting/host/client_session.h
index 39b2273..0dd65d4 100644
--- a/remoting/host/client_session.h
+++ b/remoting/host/client_session.h
@@ -12,6 +12,7 @@
 #include "base/threading/non_thread_safe.h"
 #include "remoting/host/remote_input_filter.h"
 #include "remoting/protocol/clipboard_echo_filter.h"
+#include "remoting/protocol/clipboard_filter.h"
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/host_stub.h"
@@ -27,8 +28,7 @@ class VideoFrameCapturer;
 
 // A ClientSession keeps a reference to a connection to a client, and maintains
 // per-client state.
-class ClientSession : public protocol::ClipboardStub,
-                      public protocol::HostStub,
+class ClientSession : public protocol::HostStub,
                       public protocol::InputStub,
                       public protocol::ConnectionToClient::EventHandler,
                       public base::NonThreadSafe {
@@ -74,10 +74,6 @@ class ClientSession : public protocol::ClipboardStub,
                 const base::TimeDelta& max_duration);
   virtual ~ClientSession();
 
-  // protocol::ClipboardStub interface.
-  virtual void InjectClipboardEvent(
-      const protocol::ClipboardEvent& event) OVERRIDE;
-
   // protocol::InputStub interface.
   virtual void InjectKeyEvent(const protocol::KeyEvent& event) OVERRIDE;
   virtual void InjectMouseEvent(const protocol::MouseEvent& event) OVERRIDE;
@@ -152,11 +148,13 @@ class ClientSession : public protocol::ClipboardStub,
   // Filter used to clamp mouse events to the current display dimensions.
   protocol::MouseInputFilter mouse_input_filter_;
 
-  // Filter used to manage enabling & disabling of client input events.
+  // Filters used to manage enabling & disabling of input & clipboard.
   protocol::InputFilter disable_input_filter_;
+  protocol::ClipboardFilter disable_clipboard_filter_;
 
-  // Filter used to disable inputs when we're not authenticated.
+  // Filters used to disable input & clipboard when we're not authenticated.
   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.
@@ -165,7 +163,7 @@ class ClientSession : public protocol::ClipboardStub,
   // Factory for weak pointers to the client clipboard stub.
   // This must appear after |clipboard_echo_filter_|, so that it won't outlive
   // it.
-  base::WeakPtrFactory<ClipboardStub> client_clipboard_factory_;
+  base::WeakPtrFactory<protocol::ClipboardStub> client_clipboard_factory_;
 
   // VideoFrameCapturer, used to determine current screen size for ensuring
   // injected mouse events fall within the screen area.
diff --git a/remoting/host/client_session_unittest.cc b/remoting/host/client_session_unittest.cc
index ebf4821..84783bb 100644
--- a/remoting/host/client_session_unittest.cc
+++ b/remoting/host/client_session_unittest.cc
@@ -41,6 +41,7 @@ class ClientSessionTest : public testing::Test {
     EXPECT_CALL(*session, Close());
     scoped_ptr<protocol::ConnectionToClient> connection(
         new protocol::ConnectionToClient(session));
+    connection_ = connection.get();
     client_session_.reset(new ClientSession(
         &session_event_handler_, connection.Pass(),
         &host_clipboard_stub_, &host_input_stub_, &capturer_,
@@ -71,6 +72,9 @@ class ClientSessionTest : public testing::Test {
   MockVideoFrameCapturer capturer_;
   MockClientSessionEventHandler session_event_handler_;
   scoped_ptr<ClientSession> client_session_;
+
+  // ClientSession owns |connection_| but tests need it to inject fake events.
+  protocol::ConnectionToClient* connection_;
 };
 
 MATCHER_P2(EqualsClipboardEvent, m, d, "") {
@@ -100,15 +104,15 @@ TEST_F(ClientSessionTest, ClipboardStubFilter) {
 
   // This event should not get through to the clipboard stub,
   // because the client isn't authenticated yet.
-  client_session_->InjectClipboardEvent(clipboard_event1);
+  connection_->clipboard_stub()->InjectClipboardEvent(clipboard_event1);
   client_session_->OnConnectionAuthenticated(client_session_->connection());
   client_session_->OnConnectionChannelsConnected(client_session_->connection());
   // This event should get through to the clipboard stub.
-  client_session_->InjectClipboardEvent(clipboard_event2);
+  connection_->clipboard_stub()->InjectClipboardEvent(clipboard_event2);
   DisconnectClientSession();
   // This event should not get through to the clipboard stub,
   // because the client has disconnected.
-  client_session_->InjectClipboardEvent(clipboard_event3);
+  connection_->clipboard_stub()->InjectClipboardEvent(clipboard_event3);
 }
 
 MATCHER_P2(EqualsKeyEvent, keycode, pressed, "") {
@@ -162,19 +166,19 @@ TEST_F(ClientSessionTest, InputStubFilter) {
 
   // These events should not get through to the input stub,
   // because the client isn't authenticated yet.
-  client_session_->InjectKeyEvent(key_event1);
-  client_session_->InjectMouseEvent(mouse_event1);
+  connection_->input_stub()->InjectKeyEvent(key_event1);
+  connection_->input_stub()->InjectMouseEvent(mouse_event1);
   client_session_->OnConnectionAuthenticated(client_session_->connection());
   client_session_->OnConnectionChannelsConnected(client_session_->connection());
   // These events should get through to the input stub.
-  client_session_->InjectKeyEvent(key_event2_down);
-  client_session_->InjectKeyEvent(key_event2_up);
-  client_session_->InjectMouseEvent(mouse_event2);
+  connection_->input_stub()->InjectKeyEvent(key_event2_down);
+  connection_->input_stub()->InjectKeyEvent(key_event2_up);
+  connection_->input_stub()->InjectMouseEvent(mouse_event2);
   DisconnectClientSession();
   // These events should not get through to the input stub,
   // because the client has disconnected.
-  client_session_->InjectKeyEvent(key_event3);
-  client_session_->InjectMouseEvent(mouse_event3);
+  connection_->input_stub()->InjectKeyEvent(key_event3);
+  connection_->input_stub()->InjectMouseEvent(mouse_event3);
 }
 
 TEST_F(ClientSessionTest, LocalInputTest) {
@@ -198,15 +202,15 @@ TEST_F(ClientSessionTest, LocalInputTest) {
   client_session_->OnConnectionAuthenticated(client_session_->connection());
   client_session_->OnConnectionChannelsConnected(client_session_->connection());
   // This event should get through to the input stub.
-  client_session_->InjectMouseEvent(mouse_event1);
+  connection_->input_stub()->InjectMouseEvent(mouse_event1);
   // This one should too because the local event echoes the remote one.
   client_session_->LocalMouseMoved(SkIPoint::Make(mouse_event1.x(),
                                                   mouse_event1.y()));
-  client_session_->InjectMouseEvent(mouse_event2);
+  connection_->input_stub()->InjectMouseEvent(mouse_event2);
   // This one should not.
   client_session_->LocalMouseMoved(SkIPoint::Make(mouse_event1.x(),
                                                   mouse_event1.y()));
-  client_session_->InjectMouseEvent(mouse_event3);
+  connection_->input_stub()->InjectMouseEvent(mouse_event3);
   // TODO(jamiewalch): Verify that remote inputs are re-enabled eventually
   // (via dependency injection, not sleep!)
   DisconnectClientSession();
@@ -241,9 +245,9 @@ TEST_F(ClientSessionTest, RestoreEventState) {
   client_session_->OnConnectionAuthenticated(client_session_->connection());
   client_session_->OnConnectionChannelsConnected(client_session_->connection());
 
-  client_session_->InjectKeyEvent(key1);
-  client_session_->InjectKeyEvent(key2);
-  client_session_->InjectMouseEvent(mousedown);
+  connection_->input_stub()->InjectKeyEvent(key1);
+  connection_->input_stub()->InjectKeyEvent(key2);
+  connection_->input_stub()->InjectMouseEvent(mousedown);
 
   DisconnectClientSession();
 }
@@ -272,7 +276,7 @@ TEST_F(ClientSessionTest, ClampMouseEvents) {
       event.set_y(input_y[j]);
       EXPECT_CALL(host_input_stub_, InjectMouseEvent(EqualsMouseEvent(
           expected_x[i], expected_y[j])));
-      client_session_->InjectMouseEvent(event);
+      connection_->input_stub()->InjectMouseEvent(event);
     }
   }
 
diff --git a/remoting/protocol/connection_to_client.cc b/remoting/protocol/connection_to_client.cc
index e9ce52b..8abbe64 100644
--- a/remoting/protocol/connection_to_client.cc
+++ b/remoting/protocol/connection_to_client.cc
@@ -80,16 +80,31 @@ void ConnectionToClient::set_clipboard_stub(
   clipboard_stub_ = clipboard_stub;
 }
 
+ClipboardStub* ConnectionToClient::clipboard_stub() {
+  DCHECK(CalledOnValidThread());
+  return clipboard_stub_;
+}
+
 void ConnectionToClient::set_host_stub(protocol::HostStub* host_stub) {
   DCHECK(CalledOnValidThread());
   host_stub_ = host_stub;
 }
 
+HostStub* ConnectionToClient::host_stub() {
+  DCHECK(CalledOnValidThread());
+  return host_stub_;
+}
+
 void ConnectionToClient::set_input_stub(protocol::InputStub* input_stub) {
   DCHECK(CalledOnValidThread());
   input_stub_ = input_stub;
 }
 
+InputStub* ConnectionToClient::input_stub() {
+  DCHECK(CalledOnValidThread());
+  return input_stub_;
+}
+
 void ConnectionToClient::OnSessionStateChange(Session::State state) {
   DCHECK(CalledOnValidThread());
 
diff --git a/remoting/protocol/connection_to_client.h b/remoting/protocol/connection_to_client.h
index 67cfe3b..76cecc4 100644
--- a/remoting/protocol/connection_to_client.h
+++ b/remoting/protocol/connection_to_client.h
@@ -82,21 +82,20 @@ class ConnectionToClient : public base::NonThreadSafe,
   // will be called.
   virtual void UpdateSequenceNumber(int64 sequence_number);
 
-  // Send encoded update stream data to the viewer.
+  // Get the stubs used by the host to transmit messages to the client.
+  // Note that the audio stub will be NULL if audio is not enabled.
   virtual VideoStub* video_stub();
-
-  // Send audio stream data to the viewer.
-  // Returns NULL if audio is not enabled.
   virtual AudioStub* audio_stub();
-
-  // Send control data to the viewer/client.
   virtual ClientStub* client_stub();
 
-  // Stubs for receiving data from the client.
-  // These three setters should be called before Init().
+  // Set/get the stubs which will handle messages we receive from the client.
+  // All stubs MUST be set before the session's channels become connected.
   virtual void set_clipboard_stub(ClipboardStub* clipboard_stub);
+  virtual ClipboardStub* clipboard_stub();
   virtual void set_host_stub(HostStub* host_stub);
+  virtual HostStub* host_stub();
   virtual void set_input_stub(InputStub* input_stub);
+  virtual InputStub* input_stub();
 
   // Session::EventHandler interface.
   virtual void OnSessionStateChange(Session::State state) OVERRIDE;