Revert of Move PseudoTCP and channel auth out of LibjingleTransportFa…

…ctory. (patchset chromium#5 id:100001 of https://codereview.chromium.org/551173004/)

Reason for revert:
Failed to compile on android builder.

http://build.chromium.org/p/chromium.webkit/builders/Android%20Builder/builds/46485

In file included from ../../remoting/protocol/pseudotcp_channel_factory.h:11:0,
                 from ../../remoting/protocol/pseudotcp_channel_factory.cc:5:
../../remoting/protocol/stream_channel_factory.h:34:41:error: 'string' in namespace 'std' does not name a type
   virtual void CreateChannel(const std::string& name,
                                         ^
../../remoting/protocol/stream_channel_factory.h:40:49:error: 'string' in namespace 'std' does not name a type
   virtual void CancelChannelCreation(const std::string& name) = 0;
...

Original issue's description:
> 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
>
> Committed: https://crrev.com/28d886c967e016a5d5812be43cd5916f577c2e10
> Cr-Commit-Position: refs/heads/master@{#294474}

TBR=wez@chromium.org,sergeyu@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=402993

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

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

remoting/protocol/BUILD.gn

@@ -44,7 +44,6 @@ 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",
@@ -99,10 +98,6 @@ 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",
@@ -111,7 +106,6 @@ 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,7 +10,6 @@
 #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"
@@ -158,14 +157,14 @@ void AuthenticatorTestBase::RunChannelAuth(bool expected_fail) {
 }
 
 void AuthenticatorTestBase::OnHostConnected(
-    int error,
+    net::Error error,
     scoped_ptr<net::StreamSocket> socket) {
   host_callback_.OnDone(error);
   host_socket_ = socket.Pass();
 }
 
 void AuthenticatorTestBase::OnClientConnected(
-    int error,
+    net::Error error,
     scoped_ptr<net::StreamSocket> socket) {
   client_callback_.OnDone(error);
   client_socket_ = socket.Pass();

remoting/protocol/authenticator_test_base.h

@@ -9,6 +9,7 @@
 
 #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"
 
@@ -36,7 +37,7 @@ class AuthenticatorTestBase : public testing::Test {
    public:
     MockChannelDoneCallback();
     ~MockChannelDoneCallback();
-    MOCK_METHOD1(OnDone, void(int error));
+    MOCK_METHOD1(OnDone, void(net::Error error));
   };
 
   static void ContinueAuthExchangeWith(Authenticator* sender,
@@ -48,9 +49,9 @@ class AuthenticatorTestBase : public testing::Test {
   void RunHostInitiatedAuthExchange();
   void RunChannelAuth(bool expected_fail);
 
-  void OnHostConnected(int error,
+  void OnHostConnected(net::Error error,
                        scoped_ptr<net::StreamSocket> socket);
-  void OnClientConnected(int error,
+  void OnClientConnected(net::Error error,
                          scoped_ptr<net::StreamSocket> socket);
 
   base::MessageLoop message_loop_;

remoting/protocol/channel_authenticator.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/callback_forward.h"
+#include "net/base/net_errors.h"
 
 namespace net {
 class StreamSocket;
@@ -22,14 +23,14 @@ namespace protocol {
 // should be used only once for one channel.
 class ChannelAuthenticator {
  public:
-  typedef base::Callback<void(int error, scoped_ptr<net::StreamSocket>)>
+  typedef base::Callback<void(net::Error 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, and may delete the calling authenticator.
+  // Start authentication of the given |socket|. |done_callback| is
+  // called when authentication is finished. Callback may be invoked
+  // before this method returns.
   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 StreamChannelFactory;
+class ChannelFactory;
 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_;
-  StreamChannelFactory* channel_factory_;
+  ChannelFactory* channel_factory_;
   InitializedCallback initialized_callback_;
   scoped_ptr<net::StreamSocket> channel_;
 

remoting/protocol/stream_channel_factory.h → remoting/protocol/channel_factory.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef REMOTING_PROTOCOL_STREAM_CHANNEL_FACTORY_H_
-#define REMOTING_PROTOCOL_STREAM_CHANNEL_FACTORY_H_
+#ifndef REMOTING_PROTOCOL_CHANNEL_FACTORY_H_
+#define REMOTING_PROTOCOL_CHANNEL_FACTORY_H_
 
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
@@ -17,20 +17,22 @@ class StreamSocket;
 namespace remoting {
 namespace protocol {
 
-class StreamChannelFactory : public base::NonThreadSafe {
+class ChannelFactory : public base::NonThreadSafe {
  public:
   // TODO(sergeyu): Specify connection error code when channel
   // connection fails.
   typedef base::Callback<void(scoped_ptr<net::StreamSocket>)>
       ChannelCreatedCallback;
 
-  StreamChannelFactory() {}
+  ChannelFactory() {}
 
-  // Creates new channels and calls the |callback| when then new channel is
-  // created and connected. The |callback| is called with NULL if connection
-  // 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.
+  // Creates new channels for this connection. The specified callback is called
+  // when then new channel is created and connected. The callback is called with
+  // NULL if connection failed for any reason. Callback may be called
+  // synchronously, before the call returns. All channels must be destroyed
+  // before the factory is destroyed and CancelChannelCreation() must be called
+  // to cancel creation of channels for which the |callback| hasn't been called
+  // yet.
   virtual void CreateChannel(const std::string& name,
                              const ChannelCreatedCallback& callback) = 0;
 
@@ -40,13 +42,13 @@ class StreamChannelFactory : public base::NonThreadSafe {
   virtual void CancelChannelCreation(const std::string& name) = 0;
 
  protected:
-  virtual ~StreamChannelFactory() {}
+  virtual ~ChannelFactory() {}
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(StreamChannelFactory);
+  DISALLOW_COPY_AND_ASSIGN(ChannelFactory);
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
-#endif  // REMOTING_PROTOCOL_STREAM_CHANNEL_FACTORY_H_
+#endif  // REMOTING_PROTOCOL_CHANNEL_FACTORY_H_

remoting/protocol/channel_multiplexer.cc

@@ -353,7 +353,7 @@ void ChannelMultiplexer::MuxSocket::OnPacketReceived() {
   }
 }
 
-ChannelMultiplexer::ChannelMultiplexer(StreamChannelFactory* factory,
+ChannelMultiplexer::ChannelMultiplexer(ChannelFactory* 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 StreamChannelFactory {
+class ChannelMultiplexer : public ChannelFactory {
  public:
   static const char kMuxChannelName[];
 
   // |factory| is used to create the channel upon which to multiplex.
-  ChannelMultiplexer(StreamChannelFactory* factory,
+  ChannelMultiplexer(ChannelFactory* factory,
                      const std::string& base_channel_name);
   virtual ~ChannelMultiplexer();
 
-  // StreamChannelFactory interface.
+  // ChannelFactory 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 StreamChannelFactory {
 
   // Factory used to create |base_channel_|. Set to NULL once creation is
   // finished or failed.
-  StreamChannelFactory* base_channel_factory_;
+  ChannelFactory* base_channel_factory_;
 
   // Name of the underlying channel.
   std::string base_channel_name_;

remoting/protocol/fake_authenticator.cc

@@ -7,7 +7,6 @@
 #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"
@@ -35,33 +34,31 @@ void FakeChannelAuthenticator::SecureAndAuthenticate(
   if (async_) {
     done_callback_ = done_callback;
 
-    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> 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);
-    int result =
-        socket_->Read(read_buf.get(), 1,
+    result =
+        socket_->Read(read_buf.get(),
+                      1,
                       base::Bind(&FakeChannelAuthenticator::OnAuthBytesRead,
                                  weak_factory_.GetWeakPtr()));
     if (result != net::ERR_IO_PENDING)
       OnAuthBytesRead(result);
   } else {
-    CallDoneCallback();
+    if (result_ != net::OK)
+      socket_.reset();
+    done_callback.Run(result_, socket_.Pass());
   }
 }
 
@@ -70,23 +67,15 @@ void FakeChannelAuthenticator::OnAuthBytesWritten(int result) {
   EXPECT_FALSE(did_write_bytes_);
   did_write_bytes_ = true;
   if (did_read_bytes_)
-    CallDoneCallback();
+    done_callback_.Run(result_, socket_.Pass());
 }
 
 void FakeChannelAuthenticator::OnAuthBytesRead(int result) {
   EXPECT_EQ(1, result);
   EXPECT_FALSE(did_read_bytes_);
   did_read_bytes_ = true;
   if (did_write_bytes_)
-    CallDoneCallback();
-}
-
-void FakeChannelAuthenticator::CallDoneCallback() {
-  DoneCallback callback = done_callback_;
-  done_callback_.Reset();
-  if (result_ != net::OK)
-    socket_.reset();
-  callback.Run(result_, socket_.Pass());
+    done_callback_.Run(result_, socket_.Pass());
 }
 
 FakeAuthenticator::FakeAuthenticator(

remoting/protocol/fake_authenticator.h

@@ -24,12 +24,14 @@ 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);
 
-  void CallDoneCallback();
-
-  int result_;
+  net::Error result_;
   bool async_;
 
   scoped_ptr<net::StreamSocket> socket_;