Move PseudoTCP and channel auth out of LibjingleTransportFactory.

Previously TransportFactory interface was responsible for creation
and initialization of several protocol layers, including PseudoTCP and
authentication (TLS). Simplified it so now it only creates raw datagram
transport channel. PseudoTcpChannelFactory is now responsible for
setting up PseudoTcpAdapter and AuthenticatingChannelFactory takes care
of channel authentication. Also added DatagramChannelFactory for
Datagram channels.

This change will make it possible to replace PseudoTcpChannelFactory
with an object that creates SCTP-based channels.

Also fixed a bug in SslHmacChannelAuthenticator. It wasn't working
properly when deleted from the callback. (base::Callback objects
shouldn't be deleted while being called because when deleted they
also destroy reference parameters values they are holding).

BUG=402993

Review URL: https://codereview.chromium.org/551173004

Cr-Commit-Position: refs/heads/master@{#294474}

remoting/protocol/BUILD.gn

@@ -44,6 +44,7 @@ static_library("protocol") {
     "connection_to_host.h",
     "content_description.cc",
     "content_description.h",
+    "datagram_channel_factory.h",
     "errors.h",
     "host_control_dispatcher.cc",
     "host_control_dispatcher.h",
@@ -98,6 +99,10 @@ static_library("protocol") {
     "protobuf_video_reader.h",
     "protobuf_video_writer.cc",
     "protobuf_video_writer.h",
+    "pseudotcp_channel_factory.cc",
+    "pseudotcp_channel_factory.h",
+    "secure_channel_factory.cc",
+    "secure_channel_factory.h",
     "session.h",
     "session_config.cc",
     "session_config.h",
@@ -106,6 +111,7 @@ static_library("protocol") {
     "socket_util.h",
     "ssl_hmac_channel_authenticator.cc",
     "ssl_hmac_channel_authenticator.h",
+    "stream_channel_factory.h",
     "third_party_authenticator_base.cc",
     "third_party_authenticator_base.h",
     "third_party_client_authenticator.cc",

remoting/protocol/authenticator_test_base.cc

@@ -10,6 +10,7 @@
 #include "base/path_service.h"
 #include "base/test/test_timeouts.h"
 #include "base/timer/timer.h"
+#include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/protocol/authenticator.h"
@@ -157,14 +158,14 @@ void AuthenticatorTestBase::RunChannelAuth(bool expected_fail) {
 }
 
 void AuthenticatorTestBase::OnHostConnected(
-    net::Error error,
+    int error,
     scoped_ptr<net::StreamSocket> socket) {
   host_callback_.OnDone(error);
   host_socket_ = socket.Pass();
 }
 
 void AuthenticatorTestBase::OnClientConnected(
-    net::Error error,
+    int error,
     scoped_ptr<net::StreamSocket> socket) {
   client_callback_.OnDone(error);
   client_socket_ = socket.Pass();

remoting/protocol/authenticator_test_base.h

@@ -9,7 +9,6 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
-#include "net/base/net_errors.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -37,7 +36,7 @@ class AuthenticatorTestBase : public testing::Test {
    public:
     MockChannelDoneCallback();
     ~MockChannelDoneCallback();
-    MOCK_METHOD1(OnDone, void(net::Error error));
+    MOCK_METHOD1(OnDone, void(int error));
   };
 
   static void ContinueAuthExchangeWith(Authenticator* sender,
@@ -49,9 +48,9 @@ class AuthenticatorTestBase : public testing::Test {
   void RunHostInitiatedAuthExchange();
   void RunChannelAuth(bool expected_fail);
 
-  void OnHostConnected(net::Error error,
+  void OnHostConnected(int error,
                        scoped_ptr<net::StreamSocket> socket);
-  void OnClientConnected(net::Error error,
+  void OnClientConnected(int error,
                          scoped_ptr<net::StreamSocket> socket);
 
   base::MessageLoop message_loop_;

remoting/protocol/channel_authenticator.h

@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/callback_forward.h"
-#include "net/base/net_errors.h"
 
 namespace net {
 class StreamSocket;
@@ -23,14 +22,14 @@ namespace protocol {
 // should be used only once for one channel.
 class ChannelAuthenticator {
  public:
-  typedef base::Callback<void(net::Error error, scoped_ptr<net::StreamSocket>)>
+  typedef base::Callback<void(int error, scoped_ptr<net::StreamSocket>)>
       DoneCallback;
 
   virtual ~ChannelAuthenticator() {}
 
-  // Start authentication of the given |socket|. |done_callback| is
-  // called when authentication is finished. Callback may be invoked
-  // before this method returns.
+  // Start authentication of the given |socket|. |done_callback| is called when
+  // authentication is finished. Callback may be invoked before this method
+  // returns, and may delete the calling authenticator.
   virtual void SecureAndAuthenticate(
       scoped_ptr<net::StreamSocket> socket,
       const DoneCallback& done_callback) = 0;

remoting/protocol/channel_dispatcher_base.cc

@@ -6,9 +6,9 @@
 
 #include "base/bind.h"
 #include "net/socket/stream_socket.h"
-#include "remoting/protocol/channel_factory.h"
 #include "remoting/protocol/session.h"
 #include "remoting/protocol/session_config.h"
+#include "remoting/protocol/stream_channel_factory.h"
 
 namespace remoting {
 namespace protocol {

remoting/protocol/channel_dispatcher_base.h

@@ -19,7 +19,7 @@ namespace remoting {
 namespace protocol {
 
 struct ChannelConfig;
-class ChannelFactory;
+class StreamChannelFactory;
 class Session;
 
 // Base class for channel message dispatchers. It's responsible for
@@ -56,7 +56,7 @@ class ChannelDispatcherBase {
   void OnChannelReady(scoped_ptr<net::StreamSocket> socket);
 
   std::string channel_name_;
-  ChannelFactory* channel_factory_;
+  StreamChannelFactory* channel_factory_;
   InitializedCallback initialized_callback_;
   scoped_ptr<net::StreamSocket> channel_;
 

remoting/protocol/channel_multiplexer.cc

@@ -353,7 +353,7 @@ void ChannelMultiplexer::MuxSocket::OnPacketReceived() {
   }
 }
 
-ChannelMultiplexer::ChannelMultiplexer(ChannelFactory* factory,
+ChannelMultiplexer::ChannelMultiplexer(StreamChannelFactory* factory,
                                        const std::string& base_channel_name)
     : base_channel_factory_(factory),
       base_channel_name_(base_channel_name),

remoting/protocol/channel_multiplexer.h

@@ -8,22 +8,22 @@
 #include "base/memory/weak_ptr.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"
+#include "remoting/protocol/stream_channel_factory.h"
 
 namespace remoting {
 namespace protocol {
 
-class ChannelMultiplexer : public ChannelFactory {
+class ChannelMultiplexer : public StreamChannelFactory {
  public:
   static const char kMuxChannelName[];
 
   // |factory| is used to create the channel upon which to multiplex.
-  ChannelMultiplexer(ChannelFactory* factory,
+  ChannelMultiplexer(StreamChannelFactory* factory,
                      const std::string& base_channel_name);
   virtual ~ChannelMultiplexer();
 
-  // ChannelFactory interface.
+  // StreamChannelFactory interface.
   virtual void CreateChannel(const std::string& name,
                              const ChannelCreatedCallback& callback) OVERRIDE;
   virtual void CancelChannelCreation(const std::string& name) OVERRIDE;
@@ -59,7 +59,7 @@ class ChannelMultiplexer : public ChannelFactory {
 
   // Factory used to create |base_channel_|. Set to NULL once creation is
   // finished or failed.
-  ChannelFactory* base_channel_factory_;
+  StreamChannelFactory* base_channel_factory_;
 
   // Name of the underlying channel.
   std::string base_channel_name_;

remoting/protocol/datagram_channel_factory.h

@@ -0,0 +1,45 @@
+// 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.
+
+#ifndef REMOTING_PROTOCOL_DATAGRAM_CHANNEL_FACTORY_H_
+#define REMOTING_PROTOCOL_DATAGRAM_CHANNEL_FACTORY_H_
+
+namespace net {
+class Socket;
+}  // namespace net
+
+namespace remoting {
+namespace protocol {
+
+class DatagramChannelFactory {
+ public:
+  typedef base::Callback<void(scoped_ptr<net::Socket>)>
+      ChannelCreatedCallback;
+
+  DatagramChannelFactory() {}
+
+  // Creates new channels and calls the |callback| when then new channel is
+  // created and connected. The |callback| is called with NULL if channel setup
+  // failed for any reason. Callback may be called synchronously, before the
+  // call returns. All channels must be destroyed, and CancelChannelCreation()
+  // called for any pending channels, before the factory is destroyed.
+  virtual void CreateChannel(const std::string& name,
+                             const ChannelCreatedCallback& callback) = 0;
+
+  // Cancels a pending CreateChannel() operation for the named channel. If the
+  // channel creation already completed then canceling it has no effect. When
+  // shutting down this method must be called for each channel pending creation.
+  virtual void CancelChannelCreation(const std::string& name) = 0;
+
+ protected:
+  virtual ~DatagramChannelFactory() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DatagramChannelFactory);
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_DATAGRAM_CHANNEL_FACTORY_H_

remoting/protocol/fake_authenticator.cc

@@ -7,6 +7,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
 #include "net/socket/stream_socket.h"
 #include "remoting/base/constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -34,31 +35,33 @@ void FakeChannelAuthenticator::SecureAndAuthenticate(
   if (async_) {
     done_callback_ = done_callback;
 
-    scoped_refptr<net::IOBuffer> write_buf = new net::IOBuffer(1);
-    write_buf->data()[0] = 0;
-    int result =
-        socket_->Write(write_buf.get(),
-                       1,
-                       base::Bind(&FakeChannelAuthenticator::OnAuthBytesWritten,
-                                  weak_factory_.GetWeakPtr()));
-    if (result != net::ERR_IO_PENDING) {
-      // This will not call the callback because |did_read_bytes_| is
-      // still set to false.
-      OnAuthBytesWritten(result);
+    if (result_ != net::OK) {
+      // Don't write anything if we are going to reject auth to make test
+      // ordering deterministic.
+      did_write_bytes_ = true;
+    } else {
+      scoped_refptr<net::IOBuffer> write_buf = new net::IOBuffer(1);
+      write_buf->data()[0] = 0;
+      int result = socket_->Write(
+          write_buf.get(), 1,
+          base::Bind(&FakeChannelAuthenticator::OnAuthBytesWritten,
+                     weak_factory_.GetWeakPtr()));
+      if (result != net::ERR_IO_PENDING) {
+        // This will not call the callback because |did_read_bytes_| is
+        // still set to false.
+        OnAuthBytesWritten(result);
+      }
     }
 
     scoped_refptr<net::IOBuffer> read_buf = new net::IOBuffer(1);
-    result =
-        socket_->Read(read_buf.get(),
-                      1,
+    int result =
+        socket_->Read(read_buf.get(), 1,
                       base::Bind(&FakeChannelAuthenticator::OnAuthBytesRead,
                                  weak_factory_.GetWeakPtr()));
     if (result != net::ERR_IO_PENDING)
       OnAuthBytesRead(result);
   } else {
-    if (result_ != net::OK)
-      socket_.reset();
-    done_callback.Run(result_, socket_.Pass());
+    CallDoneCallback();
   }
 }
 
@@ -67,15 +70,23 @@ void FakeChannelAuthenticator::OnAuthBytesWritten(int result) {
   EXPECT_FALSE(did_write_bytes_);
   did_write_bytes_ = true;
   if (did_read_bytes_)
-    done_callback_.Run(result_, socket_.Pass());
+    CallDoneCallback();
 }
 
 void FakeChannelAuthenticator::OnAuthBytesRead(int result) {
   EXPECT_EQ(1, result);
   EXPECT_FALSE(did_read_bytes_);
   did_read_bytes_ = true;
   if (did_write_bytes_)
-    done_callback_.Run(result_, socket_.Pass());
+    CallDoneCallback();
+}
+
+void FakeChannelAuthenticator::CallDoneCallback() {
+  DoneCallback callback = done_callback_;
+  done_callback_.Reset();
+  if (result_ != net::OK)
+    socket_.reset();
+  callback.Run(result_, socket_.Pass());
 }
 
 FakeAuthenticator::FakeAuthenticator(

remoting/protocol/fake_authenticator.h

@@ -24,14 +24,12 @@ class FakeChannelAuthenticator : public ChannelAuthenticator {
       const DoneCallback& done_callback) OVERRIDE;
 
  private:
-  void CallCallback(
-      net::Error error,
-      scoped_ptr<net::StreamSocket> socket);
-
   void OnAuthBytesWritten(int result);
   void OnAuthBytesRead(int result);
 
-  net::Error result_;
+  void CallDoneCallback();
+
+  int result_;
   bool async_;
 
   scoped_ptr<net::StreamSocket> socket_;

remoting/protocol/fake_session.cc

@@ -320,11 +320,11 @@ void FakeSession::set_config(const SessionConfig& config) {
   config_ = config;
 }
 
-ChannelFactory* FakeSession::GetTransportChannelFactory() {
+StreamChannelFactory* FakeSession::GetTransportChannelFactory() {
   return this;
 }
 
-ChannelFactory* FakeSession::GetMultiplexedChannelFactory() {
+StreamChannelFactory* FakeSession::GetMultiplexedChannelFactory() {
   return this;
 }
 

remoting/protocol/fake_session.h

@@ -14,8 +14,8 @@
 #include "net/base/completion_callback.h"
 #include "net/socket/socket.h"
 #include "net/socket/stream_socket.h"
-#include "remoting/protocol/channel_factory.h"
 #include "remoting/protocol/session.h"
+#include "remoting/protocol/stream_channel_factory.h"
 
 namespace base {
 class MessageLoop;
@@ -148,7 +148,7 @@ class FakeUdpSocket : public net::Socket {
 // FakeSession is a dummy protocol::Session that uses FakeSocket for all
 // channels.
 class FakeSession : public Session,
-                    public ChannelFactory {
+                    public StreamChannelFactory {
  public:
   FakeSession();
   virtual ~FakeSession();
@@ -173,11 +173,11 @@ class FakeSession : public Session,
   virtual const CandidateSessionConfig* candidate_config() OVERRIDE;
   virtual const SessionConfig& config() OVERRIDE;
   virtual void set_config(const SessionConfig& config) OVERRIDE;
-  virtual ChannelFactory* GetTransportChannelFactory() OVERRIDE;
-  virtual ChannelFactory* GetMultiplexedChannelFactory() OVERRIDE;
+  virtual StreamChannelFactory* GetTransportChannelFactory() OVERRIDE;
+  virtual StreamChannelFactory* GetMultiplexedChannelFactory() OVERRIDE;
   virtual void Close() OVERRIDE;
 
-  // ChannelFactory interface.
+  // StreamChannelFactory interface.
   virtual void CreateChannel(const std::string& name,
                              const ChannelCreatedCallback& callback) OVERRIDE;
   virtual void CancelChannelCreation(const std::string& name) OVERRIDE;