[Blimp Net] Implement Engine Connection Manager.

It handles multiple transports, and accepts new BlimpConnections from these transports as fast as they arrive.

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

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

12 files changed, 196 insertions, 21 deletions

diff --git a/blimp/net/BUILD.gn b/blimp/net/BUILD.gn
index 5ab0ab7..9ab6660 100644
--- a/blimp/net/BUILD.gn
+++ b/blimp/net/BUILD.gn
@@ -62,6 +62,7 @@ source_set("unit_tests") {
     "blimp_message_demultiplexer_unittest.cc",
     "blimp_message_multiplexer_unittest.cc",
     "blimp_message_pump_unittest.cc",
+    "engine_connection_manager_unittest.cc",
     "input_message_unittest.cc",
     "stream_packet_reader_unittest.cc",
     "stream_packet_writer_unittest.cc",
diff --git a/blimp/net/blimp_transport.h b/blimp/net/blimp_transport.h
index dcc68c6..8336912 100644
--- a/blimp/net/blimp_transport.h
+++ b/blimp/net/blimp_transport.h
@@ -5,6 +5,8 @@
 #ifndef BLIMP_NET_BLIMP_TRANSPORT_H_
 #define BLIMP_NET_BLIMP_TRANSPORT_H_
 
+#include <string>
+
 #include "base/memory/scoped_ptr.h"
 #include "net/base/completion_callback.h"
 
@@ -31,6 +33,9 @@ class BlimpTransport {
 
   // Returns the connection object after a successful Connect().
   virtual scoped_ptr<BlimpConnection> TakeConnection() = 0;
+
+  // Gets transport name, e.g. "TCP", "SSL", "mock", etc.
+  virtual const std::string GetName() const = 0;
 };
 
 }  // namespace blimp
diff --git a/blimp/net/connection_handler.h b/blimp/net/connection_handler.h
index a9725a2..b9d37f6 100644
--- a/blimp/net/connection_handler.h
+++ b/blimp/net/connection_handler.h
@@ -9,7 +9,7 @@ namespace blimp {
 
 class BlimpConnection;
 
-// Interface for objects that can take possesion of BlimpConnections.
+// Interface for objects that can take possession of BlimpConnections.
 class ConnectionHandler {
  public:
   virtual ~ConnectionHandler() {}
diff --git a/blimp/net/engine_connection_manager.cc b/blimp/net/engine_connection_manager.cc
index b38ed0f..549caa3 100644
--- a/blimp/net/engine_connection_manager.cc
+++ b/blimp/net/engine_connection_manager.cc
@@ -5,23 +5,37 @@
 #include "blimp/net/engine_connection_manager.h"
 
 #include "base/logging.h"
+#include "blimp/net/blimp_connection.h"
+#include "blimp/net/blimp_transport.h"
+#include "net/base/net_errors.h"
 
 namespace blimp {
 
 EngineConnectionManager::EngineConnectionManager(
-    ConnectionHandler* engine_browser_session) {
-  NOTIMPLEMENTED();
-}
+    ConnectionHandler* connection_handler)
+    : connection_handler_(connection_handler) {}
 
 EngineConnectionManager::~EngineConnectionManager() {}
 
-void EngineConnectionManager::StartListening() {
-  NOTIMPLEMENTED();
+void EngineConnectionManager::AddTransport(
+    scoped_ptr<BlimpTransport> transport) {
+  BlimpTransport* transport_ptr = transport.get();
+  transports_.push_back(std::move(transport));
+  Connect(transport_ptr);
+}
+
+void EngineConnectionManager::Connect(BlimpTransport* transport) {
+  transport->Connect(base::Bind(&EngineConnectionManager::OnConnectResult,
+                                base::Unretained(this),
+                                base::Unretained(transport)));
 }
 
-void EngineConnectionManager::HandleConnection(
-    scoped_ptr<BlimpConnection> connection) {
-  NOTIMPLEMENTED();
+void EngineConnectionManager::OnConnectResult(BlimpTransport* transport,
+                                              int result) {
+  // Expects engine transport to be reliably, thus |result| is always net::OK.
+  CHECK(result == net::OK) << "Transport failure:" << transport->GetName();
+  connection_handler_->HandleConnection(transport->TakeConnection());
+  Connect(transport);
 }
 
 }  // namespace blimp
diff --git a/blimp/net/engine_connection_manager.h b/blimp/net/engine_connection_manager.h
index def115d..1a0a491 100644
--- a/blimp/net/engine_connection_manager.h
+++ b/blimp/net/engine_connection_manager.h
@@ -5,35 +5,46 @@
 #ifndef BLIMP_NET_ENGINE_CONNECTION_MANAGER_H_
 #define BLIMP_NET_ENGINE_CONNECTION_MANAGER_H_
 
+#include <vector>
+
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "blimp/net/blimp_connection.h"
-#include "blimp/net/connection_error_observer.h"
+#include "base/memory/scoped_ptr.h"
+#include "blimp/net/blimp_net_export.h"
 #include "blimp/net/connection_handler.h"
 
 namespace blimp {
 
+class BlimpConnection;
+class BlimpTransport;
+
 // Coordinates the channel creation and authentication workflows for
 // incoming (Engine) network connections.
 //
 // TODO(kmarshall): Add rate limiting and abuse handling logic.
-class EngineConnectionManager : public ConnectionHandler,
-                                public ConnectionErrorObserver {
+class BLIMP_NET_EXPORT EngineConnectionManager {
  public:
   // Caller is responsible for ensuring that |connection_handler| outlives
   // |this|.
   explicit EngineConnectionManager(ConnectionHandler* connection_handler);
 
-  ~EngineConnectionManager() override;
-
-  // Accepts new BlimpConnections from |transport_| as fast as they arrive.
-  void StartListening();
+  ~EngineConnectionManager();
 
-  // ConnectionHandler implementation.
-  // Handles a new connection, authenticates it, and passes it on to the
-  // underlying ConnectionHandler.
-  void HandleConnection(scoped_ptr<BlimpConnection> connection) override;
+  // Adds a transport and accepts new BlimpConnections from it as fast as they
+  // arrive.
+  void AddTransport(scoped_ptr<BlimpTransport> transport);
 
  private:
+  // Invokes transport->Connect to listen for a connection.
+  void Connect(BlimpTransport* transport);
+
+  // Callback invoked by |transport| to indicate that it has a connection
+  // ready to be authenticated.
+  void OnConnectResult(BlimpTransport* transport, int result);
+
+  ConnectionHandler* connection_handler_;
+  std::vector<scoped_ptr<BlimpTransport>> transports_;
+
   DISALLOW_COPY_AND_ASSIGN(EngineConnectionManager);
 };
 
diff --git a/blimp/net/engine_connection_manager_unittest.cc b/blimp/net/engine_connection_manager_unittest.cc
new file mode 100644
index 0000000..df9de9d
--- /dev/null
+++ b/blimp/net/engine_connection_manager_unittest.cc
@@ -0,0 +1,88 @@
+// Copyright 2015 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 <stddef.h>
+#include <string>
+
+#include "base/callback_helpers.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "blimp/net/blimp_connection.h"
+#include "blimp/net/blimp_transport.h"
+#include "blimp/net/common.h"
+#include "blimp/net/connection_error_observer.h"
+#include "blimp/net/engine_connection_manager.h"
+#include "blimp/net/test_common.h"
+#include "net/base/completion_callback.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+using testing::Eq;
+using testing::Return;
+using testing::SaveArg;
+
+namespace blimp {
+
+class EngineConnectionManagerTest : public testing::Test {
+ public:
+  EngineConnectionManagerTest()
+      : manager_(new EngineConnectionManager(&connection_handler_)) {}
+
+  ~EngineConnectionManagerTest() override {}
+
+  scoped_ptr<BlimpConnection> CreateConnection() {
+    return make_scoped_ptr(
+        new BlimpConnection(make_scoped_ptr(new MockPacketReader),
+                            make_scoped_ptr(new MockPacketWriter)));
+  }
+
+ protected:
+  base::MessageLoopForIO message_loop_;
+  testing::StrictMock<MockConnectionHandler> connection_handler_;
+  scoped_ptr<EngineConnectionManager> manager_;
+};
+
+TEST_F(EngineConnectionManagerTest, ConnectionSucceeds) {
+  scoped_ptr<testing::StrictMock<MockTransport>> transport1(
+      new testing::StrictMock<MockTransport>);
+  scoped_ptr<testing::StrictMock<MockTransport>> transport2(
+      new testing::StrictMock<MockTransport>);
+
+  scoped_ptr<BlimpConnection> connection1 = CreateConnection();
+  net::CompletionCallback connect_cb_1;
+  EXPECT_CALL(*transport1, Connect(_))
+      .Times(2)
+      .WillRepeatedly(SaveArg<0>(&connect_cb_1));
+  EXPECT_CALL(connection_handler_, HandleConnectionPtr(Eq(connection1.get())));
+  EXPECT_CALL(*transport1, TakeConnectionPtr())
+      .WillOnce(Return(connection1.release()));
+
+  scoped_ptr<BlimpConnection> connection2 = CreateConnection();
+  net::CompletionCallback connect_cb_2;
+  EXPECT_CALL(*transport2, Connect(_))
+      .Times(2)
+      .WillRepeatedly(SaveArg<0>(&connect_cb_2));
+  EXPECT_CALL(connection_handler_, HandleConnectionPtr(Eq(connection2.get())));
+  EXPECT_CALL(*transport2, TakeConnectionPtr())
+      .WillOnce(Return(connection2.release()));
+
+  ASSERT_TRUE(connect_cb_1.is_null());
+  manager_->AddTransport(std::move(transport1));
+  ASSERT_FALSE(connect_cb_1.is_null());
+
+  ASSERT_TRUE(connect_cb_2.is_null());
+  manager_->AddTransport(std::move(transport2));
+  ASSERT_FALSE(connect_cb_2.is_null());
+
+  base::ResetAndReturn(&connect_cb_1).Run(net::OK);
+  base::ResetAndReturn(&connect_cb_2).Run(net::OK);
+  ASSERT_FALSE(connect_cb_1.is_null());
+  ASSERT_FALSE(connect_cb_2.is_null());
+}
+
+}  // namespace blimp
diff --git a/blimp/net/tcp_client_transport.cc b/blimp/net/tcp_client_transport.cc
index e5f5b37..9dbb8803 100644
--- a/blimp/net/tcp_client_transport.cc
+++ b/blimp/net/tcp_client_transport.cc
@@ -49,6 +49,10 @@ scoped_ptr<BlimpConnection> TCPClientTransport::TakeConnection() {
   return make_scoped_ptr(new StreamSocketConnection(std::move(socket_)));
 }
 
+const std::string TCPClientTransport::GetName() const {
+  return "TCP";
+}
+
 void TCPClientTransport::OnTCPConnectComplete(int result) {
   DCHECK_NE(net::ERR_IO_PENDING, result);
   DCHECK(socket_);
diff --git a/blimp/net/tcp_client_transport.h b/blimp/net/tcp_client_transport.h
index bbac1ba..3242883 100644
--- a/blimp/net/tcp_client_transport.h
+++ b/blimp/net/tcp_client_transport.h
@@ -32,6 +32,7 @@ class BLIMP_NET_EXPORT TCPClientTransport : public BlimpTransport {
   // BlimpTransport implementation.
   void Connect(const net::CompletionCallback& callback) override;
   scoped_ptr<BlimpConnection> TakeConnection() override;
+  const std::string GetName() const override;
 
  private:
   void OnTCPConnectComplete(int result);
diff --git a/blimp/net/tcp_engine_transport.cc b/blimp/net/tcp_engine_transport.cc
index e48509b..473a1bb 100644
--- a/blimp/net/tcp_engine_transport.cc
+++ b/blimp/net/tcp_engine_transport.cc
@@ -61,6 +61,10 @@ scoped_ptr<BlimpConnection> TCPEngineTransport::TakeConnection() {
       new StreamSocketConnection(std::move(accepted_socket_)));
 }
 
+const std::string TCPEngineTransport::GetName() const {
+  return "TCP";
+}
+
 int TCPEngineTransport::GetLocalAddressForTesting(
     net::IPEndPoint* address) const {
   DCHECK(server_socket_);
diff --git a/blimp/net/tcp_engine_transport.h b/blimp/net/tcp_engine_transport.h
index 5bb92dd..bf8b7cf 100644
--- a/blimp/net/tcp_engine_transport.h
+++ b/blimp/net/tcp_engine_transport.h
@@ -33,6 +33,7 @@ class BLIMP_NET_EXPORT TCPEngineTransport : public BlimpTransport {
   // BlimpTransport implementation.
   void Connect(const net::CompletionCallback& callback) override;
   scoped_ptr<BlimpConnection> TakeConnection() override;
+  const std::string GetName() const override;
 
   int GetLocalAddressForTesting(net::IPEndPoint* address) const;
 
diff --git a/blimp/net/test_common.cc b/blimp/net/test_common.cc
index 24f4ae3..06eddac 100644
--- a/blimp/net/test_common.cc
+++ b/blimp/net/test_common.cc
@@ -8,6 +8,7 @@
 
 #include "base/sys_byteorder.h"
 #include "blimp/common/proto/blimp_message.pb.h"
+#include "blimp/net/blimp_connection.h"
 #include "blimp/net/common.h"
 #include "net/base/io_buffer.h"
 
@@ -17,6 +18,27 @@ MockStreamSocket::MockStreamSocket() {}
 
 MockStreamSocket::~MockStreamSocket() {}
 
+MockTransport::MockTransport() {}
+
+MockTransport::~MockTransport() {}
+
+scoped_ptr<BlimpConnection> MockTransport::TakeConnection() {
+  return make_scoped_ptr(TakeConnectionPtr());
+}
+
+const std::string MockTransport::GetName() const {
+  return "mock";
+}
+
+MockConnectionHandler::MockConnectionHandler() {}
+
+MockConnectionHandler::~MockConnectionHandler() {}
+
+void MockConnectionHandler::HandleConnection(
+    scoped_ptr<BlimpConnection> connection) {
+  HandleConnectionPtr(connection.get());
+}
+
 MockPacketReader::MockPacketReader() {}
 
 MockPacketReader::~MockPacketReader() {}
diff --git a/blimp/net/test_common.h b/blimp/net/test_common.h
index 119f8ee..ca766a6 100644
--- a/blimp/net/test_common.h
+++ b/blimp/net/test_common.h
@@ -7,9 +7,12 @@
 
 #include <string>
 
+#include "base/memory/scoped_ptr.h"
 #include "blimp/common/proto/blimp_message.pb.h"
 #include "blimp/net/blimp_message_processor.h"
+#include "blimp/net/blimp_transport.h"
 #include "blimp/net/connection_error_observer.h"
+#include "blimp/net/connection_handler.h"
 #include "blimp/net/packet_reader.h"
 #include "blimp/net/packet_writer.h"
 #include "net/socket/stream_socket.h"
@@ -109,6 +112,27 @@ class MockStreamSocket : public net::StreamSocket {
   MOCK_CONST_METHOD0(GetTotalReceivedBytes, int64_t());
 };
 
+class MockTransport : public BlimpTransport {
+ public:
+  MockTransport();
+  ~MockTransport() override;
+
+  MOCK_METHOD1(Connect, void(const net::CompletionCallback& callback));
+  MOCK_METHOD0(TakeConnectionPtr, BlimpConnection*());
+
+  scoped_ptr<BlimpConnection> TakeConnection() override;
+  const std::string GetName() const override;
+};
+
+class MockConnectionHandler : public ConnectionHandler {
+ public:
+  MockConnectionHandler();
+  ~MockConnectionHandler() override;
+
+  MOCK_METHOD1(HandleConnectionPtr, void(BlimpConnection* connection));
+  void HandleConnection(scoped_ptr<BlimpConnection> connection) override;
+};
+
 class MockPacketReader : public PacketReader {
  public:
   MockPacketReader();