Implement ChannelMultiplexer.

remoting/protocol/channel_multiplexer.h

+// 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_CHANNEL_MULTIPLEXER_H_
+#define REMOTING_PROTOCOL_CHANNEL_MULTIPLEXER_H_
+
+#include "remoting/proto/mux.pb.h"
+#include "remoting/protocol/buffered_socket_writer.h"
+#include "remoting/protocol/channel_factory.h"
+#include "remoting/protocol/message_reader.h"
+
+namespace remoting {
+namespace protocol {
+
+class ChannelMultiplexer : public ChannelFactory {
+ public:
+  static const char kMuxChannelName[];
+
+  // |factory| is used to create underlying sockets.

[Wez, 2012/08/06 23:14:07]

nit: You mean it's used to create the channel(s) upon which to multiplex?

[Sergey Ulanov, 2012/08/07 20:12:22]

Done.

+  ChannelMultiplexer(ChannelFactory* factory);

[Wez, 2012/08/06 23:14:07]

nit: Allow the multiplex channel name (or channel name suffix) to be supplied by
the caller, so that multiple multiplers can run over a single underlying
ChannelFactory, e.g. one multiplexing input & clipboard, one multiplexing audio
& video, etc.

[Sergey Ulanov, 2012/08/07 20:12:22]

Done.

+  virtual ~ChannelMultiplexer();
+
+  virtual void CreateStreamChannel(

[Wez, 2012/08/06 23:14:07]

nit: Precede with comment "ChannelFactory interface."

[Sergey Ulanov, 2012/08/07 20:12:22]

Done.

+      const std::string& name,
+      const StreamChannelCallback& callback) OVERRIDE;
+  virtual void CreateDatagramChannel(
+      const std::string& name,
+      const DatagramChannelCallback& callback) OVERRIDE;
+
+ private:
+  struct PendingChannel;
+  class MuxChannel;
+  class MuxSocket;
+  friend class MuxChannel;
+
+  void OnBaseChannelReady(scoped_ptr<net::StreamSocket> socket);
+  MuxChannel* GetOrCreateChannel(const std::string& name);
+
+  void OnWriteFailed(int error);
+  void OnIncomingPacket(scoped_ptr<MuxPacket> packet,
+                        const base::Closure& done_task);
+  // Called by MuxChannel.

[Wez, 2012/08/06 23:14:07]

nit: Why is there a comment on this method, but not on the others?

[Sergey Ulanov, 2012/08/07 20:12:22]

Done.

+  void DoWrite(scoped_ptr<MuxPacket> packet, const base::Closure& done_task);
+
+  // The underlying socket.

[Wez, 2012/08/06 23:14:07]

nit: The channel over which to multiplex?

[Sergey Ulanov, 2012/08/07 20:12:22]

Done.

+  scoped_ptr<net::StreamSocket> base_channel_;
+
+  // True if creation of |base_channel_| has failed.
+  bool base_channel_failed_;
+
+  // List of requested channels while we are waiting for |base_channel_|.
+  std::list<PendingChannel> pending_channels_;
+
+  int next_channel_id_;
+  std::map<std::string, MuxChannel*> channels_;

[Wez, 2012/08/06 23:14:07]

nit: channels_by_name_

[Sergey Ulanov, 2012/08/07 20:12:22]

I think the current naming is better because it makes in clearer that channels_
contain all channels and "owns" them, while channels_by_remote_id_ is just an
auxiliary map

+  std::map<int, MuxChannel*> channels_by_remote_id_;

[Wez, 2012/08/06 23:14:07]

nit: channels_by_id_

[Wez, 2012/08/06 23:14:07]

nit: Why map name->channel and id->channel rather than name->id and id->channel,
which would make it clearer which structure "owns" the channel?

[Sergey Ulanov, 2012/08/07 20:12:22]

that would be confusing because each channel has two IDs and MuxChannel has id()
that is not what is used in this map.

[Sergey Ulanov, 2012/08/07 20:12:22]

initially, when a channel is created, we don't know "receive id" for it - that
ID is assigned by the remote side. We could have id->name and name->channel maps
but that way we would have to do string map lookup on every incoming packet
which is slower.

[Wez, 2012/08/07 21:44:44]

You've renamed the id in MuxChannel to receive_id, but in this may you've called
it remote_id?

[Wez, 2012/08/07 21:44:44]

I see what you mean. I'm beginning to wonder whether the ID optimization is
worth the additional complexity...

[Sergey Ulanov, 2012/08/07 22:25:14]

I renamed it to channels_by_receive_id_

+
+  BufferedSocketWriter writer_;
+  ProtobufMessageReader<MuxPacket> reader_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChannelMultiplexer);
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+
+#endif  // REMOTING_PROTOCOL_CHANNEL_MULTIPLEXER_H_