Merge 120449 - Fix crash on disconnect

    
This change wraps the end of the input pipeline (ClientEventDispatcher) in a
forwarder (newly introduced InputFilter). This abstraction relieves
ChromotingInstance, and everything else, from the awkward and (provably)
bug-prone task of tracking the component's lifetime. It also allows the input
pipeline to be initialized alongside the connection, making for a simpler
contract and providing better code readability, among other things.
    
BUG=112116


Review URL: http://codereview.chromium.org/9320047

TBR=dcaiafa@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9325070

git-svn-id: svn://svn.chromium.org/chrome/branches/1025/src@120575 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 89 insertions, 27 deletions

diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index e64425b..a9f3c33 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -140,6 +140,15 @@ void ChromotingInstance::Connect(const ClientConfig& config) {
                                      view_.get(), rectangle_decoder_.get(),
                                      base::Closure()));
 
+  // Construct the input pipeline
+  mouse_input_filter_.reset(
+      new MouseInputFilter(host_connection_->input_stub()));
+  mouse_input_filter_->set_input_size(view_->get_view_size());
+  key_event_tracker_.reset(
+      new protocol::KeyEventTracker(mouse_input_filter_.get()));
+  input_handler_.reset(
+      new PepperInputHandler(key_event_tracker_.get()));
+
   LOG(INFO) << "Connecting to " << config.host_jid
             << ". Local jid: " << config.local_jid << ".";
 
@@ -202,26 +211,13 @@ void ChromotingInstance::DidChangeView(const pp::Rect& position,
 bool ChromotingInstance::HandleInputEvent(const pp::InputEvent& event) {
   DCHECK(plugin_message_loop_->BelongsToCurrentThread());
 
-  // Never inject events if the end of the input pipeline doesn't exist.
-  // If it does exist but the pipeline doesn't, construct a pipeline.
-  // TODO(wez): This is really ugly.  We should create the pipeline when
-  // the ConnectionToHost's InputStub exists.
-  if (!host_connection_.get()) {
+  if (!input_handler_.get())
     return false;
-  } else if (!input_handler_.get()) {
-    protocol::InputStub* input_stub = host_connection_->input_stub();
-    if (!input_stub)
-      return false;
-    mouse_input_filter_.reset(new MouseInputFilter(input_stub));
-    mouse_input_filter_->set_input_size(view_->get_view_size());
-    key_event_tracker_.reset(
-        new protocol::KeyEventTracker(mouse_input_filter_.get()));
-    input_handler_.reset(
-        new PepperInputHandler(key_event_tracker_.get()));
-  }
 
   // TODO(wez): When we have a good hook into Host dimensions changes, move
   // this there.
+  // If |input_handler_| is valid, then |mouse_input_filter_| must also be
+  // since they are constructed together as part of the input pipeline
   mouse_input_filter_->set_output_size(view_->get_host_size());
 
   return input_handler_->HandleInputEvent(event);
diff --git a/remoting/protocol/connection_to_host.cc b/remoting/protocol/connection_to_host.cc
index 1ab6bd1..0bdc5c3 100644
--- a/remoting/protocol/connection_to_host.cc
+++ b/remoting/protocol/connection_to_host.cc
@@ -43,15 +43,7 @@ ConnectionToHost::~ConnectionToHost() {
 }
 
 InputStub* ConnectionToHost::input_stub() {
-  if (event_dispatcher_.get() && !event_dispatcher_->is_connected())
-    return NULL;
-  return event_dispatcher_.get();
-}
-
-HostStub* ConnectionToHost::host_stub() {
-  if (control_dispatcher_.get() && !control_dispatcher_->is_connected())
-    return NULL;
-  return control_dispatcher_.get();
+  return &event_forwarder_;
 }
 
 void ConnectionToHost::Connect(scoped_refptr<XmppProxy> xmpp_proxy,
@@ -220,6 +212,8 @@ void ConnectionToHost::NotifyIfChannelsReady() {
       event_dispatcher_.get() && event_dispatcher_->is_connected() &&
       video_reader_.get() && video_reader_->is_connected() &&
       state_ == CONNECTING) {
+    // Start forwarding input events to |event_dispatcher_|.
+    event_forwarder_.set_input_stub(event_dispatcher_.get());
     SetState(CONNECTED, OK);
   }
 }
@@ -232,6 +226,7 @@ void ConnectionToHost::CloseOnError(Error error) {
 void ConnectionToHost::CloseChannels() {
   control_dispatcher_.reset();
   event_dispatcher_.reset();
+  event_forwarder_.set_input_stub(NULL);
   video_reader_.reset();
 }
 
diff --git a/remoting/protocol/connection_to_host.h b/remoting/protocol/connection_to_host.h
index 16d648b..77fa497 100644
--- a/remoting/protocol/connection_to_host.h
+++ b/remoting/protocol/connection_to_host.h
@@ -12,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "remoting/jingle_glue/signal_strategy.h"
 #include "remoting/proto/internal.pb.h"
+#include "remoting/protocol/input_filter.h"
 #include "remoting/protocol/message_reader.h"
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/session_manager.h"
@@ -87,8 +88,6 @@ class ConnectionToHost : public SignalStrategy::Listener,
 
   virtual InputStub* input_stub();
 
-  virtual HostStub* host_stub();
-
   // SignalStrategy::StatusObserver interface.
   virtual void OnSignalStrategyStateChange(
       SignalStrategy::State state) OVERRIDE;
@@ -145,6 +144,7 @@ class ConnectionToHost : public SignalStrategy::Listener,
   scoped_ptr<VideoReader> video_reader_;
   scoped_ptr<ClientControlDispatcher> control_dispatcher_;
   scoped_ptr<ClientEventDispatcher> event_dispatcher_;
+  InputFilter event_forwarder_;
 
   // Internal state of the connection.
   State state_;
diff --git a/remoting/protocol/input_filter.cc b/remoting/protocol/input_filter.cc
new file mode 100644
index 0000000..c9be6a11
--- /dev/null
+++ b/remoting/protocol/input_filter.cc
@@ -0,0 +1,27 @@
+// 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"
+
+namespace remoting {
+namespace protocol {
+
+InputFilter::InputFilter() : input_stub_(NULL) {
+}
+
+InputFilter::~InputFilter() {
+}
+
+void InputFilter::InjectKeyEvent(const KeyEvent& event) {
+  if (input_stub_ != NULL)
+    input_stub_->InjectKeyEvent(event);
+}
+
+void InputFilter::InjectMouseEvent(const MouseEvent& event) {
+  if (input_stub_ != NULL)
+    input_stub_->InjectMouseEvent(event);
+}
+
+}  // namespace protocol
+}  // namespace remoting
diff --git a/remoting/protocol/input_filter.h b/remoting/protocol/input_filter.h
new file mode 100644
index 0000000..56ad8c1
--- /dev/null
+++ b/remoting/protocol/input_filter.h
@@ -0,0 +1,42 @@
+// 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.
+
+#ifndef REMOTING_PROTOCOL_INPUT_FILTER_H_
+#define REMOTING_PROTOCOL_INPUT_FILTER_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "remoting/protocol/input_stub.h"
+
+namespace remoting {
+namespace protocol {
+
+// Forwards input events to |input_stub|, iif |input_stub| is not NULL.
+class InputFilter : public InputStub {
+ public:
+  InputFilter();
+  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;
+  }
+
+  // InputStub interface.
+  virtual void InjectKeyEvent(const KeyEvent& event) OVERRIDE;
+  virtual void InjectMouseEvent(const MouseEvent& event) OVERRIDE;
+
+ private:
+  InputStub* input_stub_;
+
+  DISALLOW_COPY_AND_ASSIGN(InputFilter);
+};
+
+} // namespace protocol
+} // namespace remoting
+
+#endif // REMOTING_PROTOCOL_INPUT_FILTER_H_
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 996edab..d69ab45 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -773,6 +773,8 @@
         'protocol/host_event_dispatcher.cc',
         'protocol/host_event_dispatcher.h',
         'protocol/host_stub.h',
+        'protocol/input_filter.cc',
+        'protocol/input_filter.h',
         'protocol/input_stub.h',
         'protocol/it2me_host_authenticator_factory.cc',
         'protocol/it2me_host_authenticator_factory.h',