Move PseudoTCP and channel auth out of LibjingleTransportFactory.

remoting/protocol/authenticated_channel_factory.cc

Index: remoting/protocol/authenticated_channel_factory.cc
diff --git a/remoting/protocol/authenticated_channel_factory.cc b/remoting/protocol/authenticated_channel_factory.cc
new file mode 100644
index 0000000000000000000000000000000000000000..aee455fbe537000a8b608bdbd9b46f988c09f2c7
--- /dev/null
+++ b/remoting/protocol/authenticated_channel_factory.cc
@@ -0,0 +1,91 @@
+// Copyright 2014 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/authenticated_channel_factory.h"
+
+#include "base/bind.h"
+#include "net/socket/stream_socket.h"
+#include "remoting/protocol/authenticator.h"
+#include "remoting/protocol/channel_authenticator.h"
+
+namespace remoting {
+namespace protocol {
+
+AuthenticatedChannelFactory::AuthenticatedChannelFactory(
+    ChannelFactory* channel_factory,
+    Authenticator* authenticator)
+    : channel_factory_(channel_factory),
+      authenticator_(authenticator) {
+  DCHECK_EQ(authenticator_->state(), Authenticator::ACCEPTED);
+}
+
+AuthenticatedChannelFactory::~AuthenticatedChannelFactory() {
+  // CancelChannelCreation() is expected to be called before destruction.

[Wez, 2014/09/10 02:29:25]

Is that a part of the ChannelFactory interface contract? If so then consider
removing this comment.

[Sergey Ulanov, 2014/09/10 21:50:57]

I think it's still useful to make it clear what this DCHECK is for.

[Wez, 2014/09/10 22:19:41]

You have an equivalent check in the PseudoTcpChannelFactory, though, and don't
have a comment there. ;)

[Sergey Ulanov, 2014/09/10 22:49:30]

Done.

+  DCHECK(channels_.empty());
+}
+
+void AuthenticatedChannelFactory::CreateChannel(
+    const std::string& name,
+    const ChannelCreatedCallback& callback) {
+  DCHECK(!callback.is_null());
+  channel_factory_->CreateChannel(
+      name,
+      base::Bind(&AuthenticatedChannelFactory::OnBaseChannelCreated,
+                 base::Unretained(this), name, callback));
+}
+
+void AuthenticatedChannelFactory::CancelChannelCreation(
+    const std::string& name) {
+  ChannelsMap::iterator it = channels_.find(name);
+  if (it == channels_.end()) {
+    channel_factory_->CancelChannelCreation(name);
+  } else {
+    delete it->second;
+    channels_.erase(it);
+  }
+}
+
+void AuthenticatedChannelFactory::OnBaseChannelCreated(
+    const std::string& name,
+    const ChannelCreatedCallback& callback,
+    scoped_ptr<net::StreamSocket> socket) {
+  if (!socket) {
+    callback.Run(socket.Pass());

[Wez, 2014/09/10 02:29:25]

It reads oddly to socket.Pass() only if socket is NULL...
scoped_ptr<net::StreamSocket>() would be more obvious, I think. ;)

[Sergey Ulanov, 2014/09/10 21:50:57]

yes, but it's more to type :(. Done.

+    return;
+  }
+
+  scoped_ptr<ChannelAuthenticator> channel_authenticator =
+      authenticator_->CreateChannelAuthenticator();
+  ChannelAuthenticator* channel_authenticator_ptr =
+      channel_authenticator.release();

[Wez, 2014/09/10 02:29:25]

nit: Can you just .release() the CreateChannelAuthenticator() result directly,
without the |channel_authenticator| variable?

[Sergey Ulanov, 2014/09/10 21:50:57]

Done.

+  channels_[name] = channel_authenticator_ptr;
+  channel_authenticator_ptr->SecureAndAuthenticate(
+      socket.Pass(),
+      base::Bind(&AuthenticatedChannelFactory::OnAuthenticated,
+                 base::Unretained(this), name, callback));
+}
+
+void AuthenticatedChannelFactory::OnAuthenticated(
+    const std::string& name,
+    const ChannelCreatedCallback& callback,
+    net::Error error,
+    scoped_ptr<net::StreamSocket> socket) {
+  scoped_ptr<ChannelAuthenticator> authenticator;
+
+  ChannelsMap::iterator it = channels_.find(name);
+  DCHECK(it != channels_.end());
+  authenticator.reset(it->second);

[Wez, 2014/09/10 02:29:25]

Why not just delete it->second, like you do in the cancel case?

[Sergey Ulanov, 2014/09/10 21:50:57]

Done.

+  channels_.erase(it);
+
+  DCHECK(error == net::OK || socket);

[Wez, 2014/09/10 02:29:25]

Do you mean (socket || error != net::OK), i.e. that either we got a socket, or
an error must have occurred?

[Sergey Ulanov, 2014/09/10 21:50:57]

Yes. Replaced it with a stricter DCHECK and fixed a bug in
FakeChannelAuthenticator.

+  if (error != net::OK) {
+    socket.reset();
+  }
+
+  callback.Run(socket.Pass());
+

[Wez, 2014/09/10 02:29:26]

nit: Lose this blank line.

[Sergey Ulanov, 2014/09/10 21:50:57]

Done.

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