Move ConnectionToClient::EventHandler from ChromotingHost to ClientSession

remoting/protocol/connection_to_client.cc

 // Copyright (c) 2011 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/connection_to_client.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/message_loop_proxy.h"
 #include "google/protobuf/message.h"
 #include "net/base/io_buffer.h"
 #include "remoting/protocol/client_control_sender.h"
 #include "remoting/protocol/host_message_dispatcher.h"
 #include "remoting/protocol/host_stub.h"
 #include "remoting/protocol/input_stub.h"
 
 // TODO(hclam): Remove this header once MessageDispatcher is used.
 #include "remoting/base/compound_buffer.h"

[Wez, 2011/11/09 01:35:07]

Can this be removed now?  (I realise it's not part of this CL!)

[Sergey Ulanov, 2011/11/09 19:26:13]

Done.

 
 namespace remoting {
 namespace protocol {
 
 // Determine how many update streams we should count to find the size of
 // average update stream.
 static const size_t kAverageUpdateStream = 10;

[Wez, 2011/11/09 01:35:07]

I can't see this being used anywhere, so maybe remove it while you're here?

[Sergey Ulanov, 2011/11/09 19:26:13]

Done.

 
 ConnectionToClient::ConnectionToClient(base::MessageLoopProxy* message_loop,
-                                       EventHandler* handler)
+                                       protocol::Session* session)
     : message_loop_(message_loop),
-      handler_(handler),
+      handler_(NULL),
       host_stub_(NULL),
       input_stub_(NULL),
+      session_(session),
       control_connected_(false),
       input_connected_(false),
       video_connected_(false) {
   DCHECK(message_loop_);
-  DCHECK(handler_);
+  session_->SetStateChangeCallback(
+      base::Bind(&ConnectionToClient::OnSessionStateChange,
+                 base::Unretained(this)));

[Wez, 2011/11/09 01:35:07]

Why not do this in SetEventHandler()?  (Not saying you should, just curious as
to why it moved!)

[Sergey Ulanov, 2011/11/09 19:26:13]

Just because I moved |session| parameter from Init() to here. SetEventHandler()
may be called more than once and we want to handle session state changes event
before SetEventHandler() is called.

 }
 
 ConnectionToClient::~ConnectionToClient() {
   // TODO(hclam): When we shut down the viewer we may have to close the
   // connection.
 }
 
-void ConnectionToClient::Init(protocol::Session* session) {
+void ConnectionToClient::SetEventHandler(EventHandler* event_handler) {
   DCHECK(message_loop_->BelongsToCurrentThread());
-  session_.reset(session);
-  session_->SetStateChangeCallback(
-      base::Bind(&ConnectionToClient::OnSessionStateChange,
-                 base::Unretained(this)));
+  handler_ = event_handler;
 }
 
 protocol::Session* ConnectionToClient::session() {
   return session_.get();
 }
 
 void ConnectionToClient::Disconnect() {
   // This method can be called from main thread so perform threading switching.
   if (!message_loop_->BelongsToCurrentThread()) {
     message_loop_->PostTask(
         FROM_HERE,
         NewRunnableMethod(this, &ConnectionToClient::Disconnect));
     return;
   }
 
   CloseChannels();
 
   // If there is a session then release it, causing it to close.
   if (session_.get())
     session_.reset();
 }
 
 void ConnectionToClient::UpdateSequenceNumber(int64 sequence_number) {
-  handler_->OnSequenceNumberUpdated(this, sequence_number);
+  if (handler_)
+    handler_->OnSequenceNumberUpdated(this, sequence_number);
 }
 
 VideoStub* ConnectionToClient::video_stub() {
   return video_writer_.get();
 }
 
 // Return pointer to ClientStub.
 ClientStub* ConnectionToClient::client_stub() {
   return client_control_sender_.get();
 }
 
 void ConnectionToClient::set_host_stub(protocol::HostStub* host_stub) {
   host_stub_ = host_stub;
 }
 
 void ConnectionToClient::set_input_stub(protocol::InputStub* input_stub) {
   input_stub_ = input_stub;
 }
 
 void ConnectionToClient::OnSessionStateChange(protocol::Session::State state) {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
-  DCHECK(handler_);
   switch(state) {
     case protocol::Session::CONNECTING:
       // Don't care about this message.
       break;
 
     case protocol::Session::CONNECTED:
       video_writer_.reset(
           VideoWriter::Create(message_loop_, session_->config()));
       video_writer_->Init(
           session_.get(), base::Bind(&ConnectionToClient::OnVideoInitialized,
                                      base::Unretained(this)));
       break;
 
     case protocol::Session::CONNECTED_CHANNELS:
       client_control_sender_.reset(
           new ClientControlSender(message_loop_, session_->control_channel()));
       dispatcher_.reset(new HostMessageDispatcher());
       dispatcher_->Initialize(this, host_stub_, input_stub_);
 
       control_connected_ = true;
       input_connected_ = true;
       NotifyIfChannelsReady();
       break;
 
     case protocol::Session::CLOSED:
       CloseChannels();
-      handler_->OnConnectionClosed(this);
+      if (handler_)
+        handler_->OnConnectionClosed(this);
       break;
 
     case protocol::Session::FAILED:
       CloseOnError();
       break;
 
     default:
       // We shouldn't receive other states.
       NOTREACHED();
   }
 }
 
 void ConnectionToClient::OnVideoInitialized(bool successful) {
   if (!successful) {
     LOG(ERROR) << "Failed to connect video channel";
     CloseOnError();
     return;
   }
 
   video_connected_ = true;
   NotifyIfChannelsReady();
 }
 
 void ConnectionToClient::NotifyIfChannelsReady() {
-  if (control_connected_ && input_connected_ && video_connected_)
-    handler_->OnConnectionOpened(this);
+  if (control_connected_ && input_connected_ && video_connected_) {
+    if (handler_)
+      handler_->OnConnectionOpened(this);

[Wez, 2011/11/09 01:35:07]

Doesn't this mean we'll never get this notification if there's no handler when
the channels become ready?

[Sergey Ulanov, 2011/11/09 19:26:13]

Fixed it. Now SetEventHandler() is required to be called when this object is
created.

+  }
 }
 
 void ConnectionToClient::CloseOnError() {
   CloseChannels();
-  handler_->OnConnectionFailed(this);
+  if (handler_)
+    handler_->OnConnectionFailed(this);
 }
 
 void ConnectionToClient::CloseChannels() {
   if (video_writer_.get())
     video_writer_->Close();
   if (client_control_sender_.get())
     client_control_sender_->Close();
 }
 
 }  // namespace protocol
 }  // namespace remoting