Fix ChannelMultiplexer to properly handle base channel creation failure.

remoting/protocol/fake_session.cc

 // 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/fake_session.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "net/base/address_list.h"
 #include "net/base/io_buffer.h"
@@ skipping 266 matching lines @@
 }
 bool FakeUdpSocket::SetSendBufferSize(int32 size) {
   NOTIMPLEMENTED();
   return false;
 }
 
 FakeSession::FakeSession()
     : event_handler_(NULL),
       candidate_config_(CandidateSessionConfig::CreateDefault()),
       config_(SessionConfig::GetDefault()),
-      message_loop_(NULL),
+      message_loop_(MessageLoop::current()),
+      async_creation_(false),
       jid_(kTestJid),
       error_(OK),
-      closed_(false) {
+      closed_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
 FakeSession::~FakeSession() { }
 
 FakeSocket* FakeSession::GetStreamChannel(const std::string& name) {
   return stream_channels_[name];
 }
 
 FakeUdpSocket* FakeSession::GetDatagramChannel(const std::string& name) {
   return datagram_channels_[name];
@@ skipping 29 matching lines @@
 
 ChannelFactory* FakeSession::GetMultiplexedChannelFactory() {
   return this;
 }
 
 void FakeSession::Close() {
   closed_ = true;
 }
 
 void FakeSession::CreateStreamChannel(
-    const std::string& name, const StreamChannelCallback& callback) {
-  scoped_ptr<FakeSocket> channel(new FakeSocket());
-  stream_channels_[name] = channel.get();
-  callback.Run(channel.PassAs<net::StreamSocket>());
+    const std::string& name,
+    const StreamChannelCallback& callback) {
+  scoped_ptr<FakeSocket> channel;
+  if (error_ == OK)
+    channel.reset(new FakeSocket());
+  stream_channels_[name] = channel.release();

[Wez, 2012/09/25 17:43:58]

Doesn't this mean we end up with a NULL channel in the map if |error_| is not
OK?  Do we want that?

[Sergey Ulanov, 2012/09/25 18:59:08]

We want that. Presence of the null in that container is used to distinguish the
case when channel creation failed from the case when it was cancelled by
CancelChannelCreation().

+
+  if (async_creation_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &FakeSession::CallStreamChannelCallback, weak_factory_.GetWeakPtr(),
+        name, callback));
+  } else {
+    CallStreamChannelCallback(name, callback);
+  }
+}
+
+void FakeSession::CallStreamChannelCallback(
+    const std::string& name,
+    const StreamChannelCallback& callback) {
+  if (stream_channels_.find(name) != stream_channels_.end())
+    callback.Run(scoped_ptr<net::StreamSocket>(stream_channels_[name]));
 }
 
 void FakeSession::CreateDatagramChannel(
-    const std::string& name, const DatagramChannelCallback& callback) {
-  scoped_ptr<FakeUdpSocket> channel(new FakeUdpSocket());
-  datagram_channels_[name] = channel.get();
-  callback.Run(channel.PassAs<net::Socket>());
+    const std::string& name,
+    const DatagramChannelCallback& callback) {
+  scoped_ptr<FakeUdpSocket> channel;
+  if (error_ == OK)
+    channel.reset(new FakeUdpSocket());
+  datagram_channels_[name] = channel.release();
+
+  if (async_creation_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &FakeSession::CallDatagramChannelCallback, weak_factory_.GetWeakPtr(),
+        name, callback));
+  } else {
+    CallDatagramChannelCallback(name, callback);
+  }
+}
+
+void FakeSession::CallDatagramChannelCallback(
+    const std::string& name,
+    const DatagramChannelCallback& callback) {
+  if (datagram_channels_.find(name) != datagram_channels_.end())
+    callback.Run(scoped_ptr<net::Socket>(datagram_channels_[name]));
 }
 
 void FakeSession::CancelChannelCreation(const std::string& name) {
+  stream_channels_.erase(name);
+  datagram_channels_.erase(name);
 }
 
 }  // namespace protocol
 }  // namespace remoting