Refactor client channel dispatchers.

remoting/protocol/client_control_dispatcher.cc

Index: remoting/protocol/client_control_dispatcher.cc
diff --git a/remoting/protocol/client_control_dispatcher.cc b/remoting/protocol/client_control_dispatcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4398aad0a402d0ef395065876dda8c2fb81e1697
--- /dev/null
+++ b/remoting/protocol/client_control_dispatcher.cc
@@ -0,0 +1,61 @@
+// 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/client_control_dispatcher.h"
+
+#include "base/memory/ref_counted.h"
+#include "net/base/io_buffer.h"
+#include "remoting/proto/control.pb.h"
+#include "remoting/proto/event.pb.h"
+#include "remoting/proto/internal.pb.h"
+#include "remoting/protocol/client_stub.h"
+#include "remoting/protocol/input_stub.h"
+#include "remoting/protocol/message_reader.h"
+#include "remoting/protocol/session.h"
+
+namespace remoting {
+namespace protocol {
+
+ClientControlDispatcher::ClientControlDispatcher(
+    base::MessageLoopProxy* message_loop)
+    : client_stub_(NULL),
+      writer_(new BufferedSocketWriter(message_loop)) {
+}
+
+ClientControlDispatcher::~ClientControlDispatcher() {
+  Close();
+}
+
+void ClientControlDispatcher::Init(
+    protocol::Session* session, ClientStub* client_stub) {
+  DCHECK(session);
+  DCHECK(client_stub);
+
+  client_stub_ = client_stub;
+
+  writer_->Init(session->control_channel(),
+                         BufferedSocketWriter::WriteFailedCallback());

[Wez, 2011/11/17 01:51:56]

Seems strange to ignore write errors?

[Sergey Ulanov, 2011/11/17 19:29:06]

Write may fail only when the channel is disconnected, and this should trigger
error from the session layer anyway. In either case I agree that it's not
correct to ignore this error, this should be fixed, but in a separate CL.

+
+  reader_.Init(session->control_channel(), base::Bind(
+      &ClientControlDispatcher::OnMessageReceived, base::Unretained(this)));

[Wez, 2011/11/17 01:51:56]

Can either of these Init() calls fail?

[Sergey Ulanov, 2011/11/17 19:29:06]

No. 

+  return;
+}
+
+void ClientControlDispatcher::OnMessageReceived(
+    ControlMessage* message, const base::Closure& done_task) {
+  if (message->has_begin_session_deprecated()) {
+    // Host sends legacy BeginSession message for compatibility with
+    // older clients. Ignore it.
+  } else {
+    LOG(WARNING) << "Invalid control message received.";

[Wez, 2011/11/17 01:51:56]

nit: The message isn't necessarily "invalid", just "unknown".

[Sergey Ulanov, 2011/11/17 19:29:06]

Done.

+  }
+  done_task.Run();
+}
+
+void ClientControlDispatcher::Close() {
+  writer_->Close();

[Wez, 2011/11/17 01:51:56]

Fold this in to the destructor?  Do we even need to call Close() on the writer,
rather than just dropping our reference to it?

Are we guaranteed that when the reader is torn down, we'll get no more callbacks
from it?

[Sergey Ulanov, 2011/11/17 19:29:06]

Done.
writer is currently ref-counted so we need to explicitly close it so that it
stops using the socket. Actually now there is no need to the writer to be
ref-counted because we use it only from one thread, so I'll make it
non-ref-counted.

+}
+
+}  // namespace protocol
+}  // namespace remoting