Refactor channel dispatchers on the host side.

remoting/protocol/host_control_dispatcher.cc

Index: remoting/protocol/host_control_dispatcher.cc
diff --git a/remoting/protocol/host_control_dispatcher.cc b/remoting/protocol/host_control_dispatcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4e9d28fd49daa349c1acd6635aa6bf47f9ab7c6d
--- /dev/null
+++ b/remoting/protocol/host_control_dispatcher.cc
@@ -0,0 +1,56 @@
+// 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/host_control_dispatcher.h"
+
+#include "remoting/protocol/buffered_socket_writer.h"

[Wez, 2011/11/17 02:03:40]

This belongs after the remoting/proto/ includes?

[Sergey Ulanov, 2011/11/17 18:38:14]

Done.

+#include "remoting/proto/control.pb.h"
+#include "remoting/proto/internal.pb.h"
+#include "remoting/protocol/host_stub.h"
+#include "remoting/protocol/session.h"
+#include "remoting/protocol/util.h"
+
+namespace remoting {
+namespace protocol {
+
+HostControlDispatcher::HostControlDispatcher(
+    base::MessageLoopProxy* message_loop)
+    : host_stub_(NULL),
+      writer_(new BufferedSocketWriter(message_loop)) {
+}
+
+HostControlDispatcher::~HostControlDispatcher() {
+  Close();
+}
+
+void HostControlDispatcher::Init(Session* session, HostStub* host_stub) {
+  DCHECK(session);
+  DCHECK(host_stub);
+
+  host_stub_ = host_stub;
+
+  reader_.Init(session->control_channel(), base::Bind(
+      &HostControlDispatcher::OnMessageReceived, base::Unretained(this)));
+  writer_->Init(session->control_channel(),
+                BufferedSocketWriter::WriteFailedCallback());
+
+  // Write legacy BeginSession message.
+  protocol::ControlMessage message;
+  message.mutable_begin_session_deprecated()->mutable_login_status()->
+      set_success(true);
+  writer_->Write(SerializeAndFrameMessage(message), base::Closure());

[Wez, 2011/11/17 02:03:40]

Is it really the dispatcher's responsibility to do that?  It feels like it'd be
more appropriate for the calling code to do it, leaving this a pure helper
class.  Or the fact that Init() sends this message should be mentioned in the
header.

[Sergey Ulanov, 2011/11/17 18:38:14]

I'd like to hide version-dependent details inside dispatcher. I.e. the interface
of the dispatcher is always the same no matter what version of the protocol is
used. This message is here for backward compatibility and upper layers shouldn't
care about it.

+}
+
+void HostControlDispatcher::OnMessageReceived(
+    ControlMessage* message, const base::Closure& done_task) {
+  LOG(WARNING) << "Invalid control message received.";

[Wez, 2011/11/17 02:03:40]

nit: Not "invalid", just "unknown"!

[Sergey Ulanov, 2011/11/17 18:38:14]

Done.

+  done_task.Run();
+}
+
+void HostControlDispatcher::Close() {
+  writer_->Close();
+}
+
+}  // namespace protocol
+}  // namespace remoting