[Blimp Net] Implements ClientConnectionManager.

It manages mutliple transports and uses them one by one in the order they were added to connect to the engine.

BUG=

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

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

5 files changed, 177 insertions, 28 deletions

diff --git a/blimp/net/BUILD.gn b/blimp/net/BUILD.gn
index f20c77c..943c34a 100644
--- a/blimp/net/BUILD.gn
+++ b/blimp/net/BUILD.gn
@@ -82,6 +82,7 @@ source_set("unit_tests") {
     "blimp_message_multiplexer_unittest.cc",
     "blimp_message_output_buffer_unittest.cc",
     "blimp_message_pump_unittest.cc",
+    "client_connection_manager_unittest.cc",
     "engine_connection_manager_unittest.cc",
     "input_message_unittest.cc",
     "stream_packet_reader_unittest.cc",
diff --git a/blimp/net/client_connection_manager.cc b/blimp/net/client_connection_manager.cc
index 659f6a4..c8238e4 100644
--- a/blimp/net/client_connection_manager.cc
+++ b/blimp/net/client_connection_manager.cc
@@ -5,29 +5,55 @@
 #include "blimp/net/client_connection_manager.h"
 
 #include "base/logging.h"
+#include "blimp/net/blimp_connection.h"
+#include "blimp/net/blimp_transport.h"
+#include "blimp/net/connection_handler.h"
+#include "net/base/net_errors.h"
 
 namespace blimp {
 
 ClientConnectionManager::ClientConnectionManager(
-    ConnectionHandler* connection_handler) {
-  NOTIMPLEMENTED();
+    ConnectionHandler* connection_handler)
+    : connection_handler_(connection_handler) {
+  DCHECK(connection_handler_);
 }
 
-ClientConnectionManager::~ClientConnectionManager() {
-  NOTIMPLEMENTED();
+ClientConnectionManager::~ClientConnectionManager() {}
+
+void ClientConnectionManager::AddTransport(
+    scoped_ptr<BlimpTransport> transport) {
+  DCHECK(transport);
+  transports_.push_back(std::move(transport));
 }
 
 void ClientConnectionManager::Connect() {
-  NOTIMPLEMENTED();
+  // A |transport| added first is used first. When it fails to connect,
+  // the next transport is used.
+  DCHECK(!transports_.empty());
+  Connect(0);
 }
 
-void ClientConnectionManager::HandleConnection(
-    scoped_ptr<BlimpConnection> connection) {
-  NOTIMPLEMENTED();
+void ClientConnectionManager::Connect(int transport_index) {
+  if (static_cast<size_t>(transport_index) < transports_.size()) {
+    transports_[transport_index]->Connect(
+        base::Bind(&ClientConnectionManager::OnConnectResult,
+                   base::Unretained(this), transport_index));
+  } else {
+    // TODO(haibinlu): add an error reporting path out for this.
+    LOG(WARNING) << "All transports failed to connect";
+  }
 }
 
-void ClientConnectionManager::OnConnectionError(int error) {
-  NOTIMPLEMENTED();
+void ClientConnectionManager::OnConnectResult(int transport_index, int result) {
+  DCHECK_NE(result, net::ERR_IO_PENDING);
+  const auto& transport = transports_[transport_index];
+  if (result == net::OK) {
+    connection_handler_->HandleConnection(transport->TakeConnection());
+  } else {
+    DVLOG(1) << "Transport " << transport->GetName()
+             << " failed to connect:" << net::ErrorToString(result);
+    Connect(transport_index + 1);
+  }
 }
 
 }  // namespace blimp
diff --git a/blimp/net/client_connection_manager.h b/blimp/net/client_connection_manager.h
index ed9c9a9..f259b62 100644
--- a/blimp/net/client_connection_manager.h
+++ b/blimp/net/client_connection_manager.h
@@ -5,37 +5,53 @@
 #ifndef BLIMP_NET_CLIENT_CONNECTION_MANAGER_H_
 #define BLIMP_NET_CLIENT_CONNECTION_MANAGER_H_
 
+#include <vector>
+
 #include "base/macros.h"
-#include "blimp/net/blimp_connection.h"
-#include "blimp/net/connection_error_observer.h"
-#include "blimp/net/connection_handler.h"
+#include "base/memory/scoped_ptr.h"
+#include "blimp/net/blimp_net_export.h"
 
 namespace blimp {
 
+class BlimpTransport;
+class ConnectionHandler;
+
 // Coordinates the channel creation and authentication workflows for
 // outgoing (Client) network connections.
-// Attempts to reconnect if an authenticated connection is
-// disconnected.
-class ClientConnectionManager : public ConnectionHandler,
-                                ConnectionErrorObserver {
-  // Caller is responsible for ensuring that |client_browser_session|
+//
+// TODO(haibinlu): cope with network changes that may potentially affect the
+// endpoint that we're trying to connect to.
+class BLIMP_NET_EXPORT ClientConnectionManager {
+ public:
+  // Caller is responsible for ensuring that |connection_handler|
   // outlives |this|.
   explicit ClientConnectionManager(ConnectionHandler* connection_handler);
 
-  ~ClientConnectionManager() override;
+  ~ClientConnectionManager();
+
+  // Adds a transport. All transports are expected to be added before invoking
+  // |Connect|.
+  void AddTransport(scoped_ptr<BlimpTransport> transport);
 
+  // Attempts to create a connection using any of the BlimpTransports in
+  // |transports_|.
+  // This will result in the handler being called back at-most-once.
+  //
+  // This is also a placeholder for automatic reconnection logic for common
+  // cases such as network switches, online/offline changes.
   void Connect();
 
-  // ConnectionHandler implementation.
-  // Handles a new connection and authenticates it before passing it on to
-  // the underlying ConnectionHandler.
-  void HandleConnection(scoped_ptr<BlimpConnection> connection) override;
+ private:
+  // Tries to connect using the BlimpTransport specified at |transport_index|.
+  void Connect(int transport_index);
 
-  // ConnectionErrorObserver implementation.
-  // Used to implement reconnection logic on unexpected disconnections.
-  void OnConnectionError(int error) override;
+  // Callback invoked by transports_[transport_index] to indicate that it has a
+  // connection ready to be authenticated or there is an error.
+  void OnConnectResult(int transport_index, int result);
+
+  ConnectionHandler* connection_handler_;
+  std::vector<scoped_ptr<BlimpTransport>> transports_;
 
- private:
   DISALLOW_COPY_AND_ASSIGN(ClientConnectionManager);
 };
 
diff --git a/blimp/net/client_connection_manager_unittest.cc b/blimp/net/client_connection_manager_unittest.cc
new file mode 100644
index 0000000..31d6a2bff
--- /dev/null
+++ b/blimp/net/client_connection_manager_unittest.cc
@@ -0,0 +1,104 @@
+// 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 "blimp/net/blimp_connection.h"
+#include "blimp/net/blimp_transport.h"
+#include "blimp/net/client_connection_manager.h"
+#include "blimp/net/test_common.h"
+#include "net/base/completion_callback.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 ClientConnectionManagerTest : public testing::Test {
+ public:
+  ClientConnectionManagerTest()
+      : manager_(new ClientConnectionManager(&connection_handler_)),
+        transport1_(new testing::StrictMock<MockTransport>),
+        transport2_(new testing::StrictMock<MockTransport>),
+        connection_(
+            new BlimpConnection(make_scoped_ptr(new MockPacketReader),
+                                make_scoped_ptr(new MockPacketWriter))) {}
+
+  ~ClientConnectionManagerTest() override {}
+
+ protected:
+  base::MessageLoop message_loop_;
+  testing::StrictMock<MockConnectionHandler> connection_handler_;
+  scoped_ptr<ClientConnectionManager> manager_;
+  scoped_ptr<testing::StrictMock<MockTransport>> transport1_;
+  scoped_ptr<testing::StrictMock<MockTransport>> transport2_;
+  scoped_ptr<BlimpConnection> connection_;
+};
+
+// The 1st transport connects, and the 2nd transport is not used.
+TEST_F(ClientConnectionManagerTest, FirstTransportConnects) {
+  net::CompletionCallback connect_cb_1;
+  EXPECT_CALL(*transport1_, Connect(_)).WillOnce(SaveArg<0>(&connect_cb_1));
+  EXPECT_CALL(connection_handler_, HandleConnectionPtr(Eq(connection_.get())));
+  EXPECT_CALL(*transport1_, TakeConnectionPtr())
+      .WillOnce(Return(connection_.release()));
+
+  ASSERT_TRUE(connect_cb_1.is_null());
+  manager_->AddTransport(std::move(transport1_));
+  manager_->AddTransport(std::move(transport2_));
+  manager_->Connect();
+  ASSERT_FALSE(connect_cb_1.is_null());
+  base::ResetAndReturn(&connect_cb_1).Run(net::OK);
+}
+
+// The 1st transport fails to connect, and the 2nd transport connects.
+TEST_F(ClientConnectionManagerTest, SecondTransportConnects) {
+  net::CompletionCallback connect_cb_1;
+  EXPECT_CALL(*transport1_, Connect(_)).WillOnce(SaveArg<0>(&connect_cb_1));
+  net::CompletionCallback connect_cb_2;
+  EXPECT_CALL(*transport2_, Connect(_)).WillOnce(SaveArg<0>(&connect_cb_2));
+  EXPECT_CALL(connection_handler_, HandleConnectionPtr(Eq(connection_.get())));
+  EXPECT_CALL(*transport2_, TakeConnectionPtr())
+      .WillOnce(Return(connection_.release()));
+
+  ASSERT_TRUE(connect_cb_1.is_null());
+  ASSERT_TRUE(connect_cb_2.is_null());
+  manager_->AddTransport(std::move(transport1_));
+  manager_->AddTransport(std::move(transport2_));
+  manager_->Connect();
+  ASSERT_FALSE(connect_cb_1.is_null());
+  base::ResetAndReturn(&connect_cb_1).Run(net::ERR_FAILED);
+  ASSERT_FALSE(connect_cb_2.is_null());
+  base::ResetAndReturn(&connect_cb_2).Run(net::OK);
+}
+
+// Both transports fail to connect.
+TEST_F(ClientConnectionManagerTest, BothTransportsFailToConnect) {
+  net::CompletionCallback connect_cb_1;
+  EXPECT_CALL(*transport1_, Connect(_)).WillOnce(SaveArg<0>(&connect_cb_1));
+  net::CompletionCallback connect_cb_2;
+  EXPECT_CALL(*transport2_, Connect(_)).WillOnce(SaveArg<0>(&connect_cb_2));
+
+  ASSERT_TRUE(connect_cb_1.is_null());
+  ASSERT_TRUE(connect_cb_2.is_null());
+  manager_->AddTransport(std::move(transport1_));
+  manager_->AddTransport(std::move(transport2_));
+  manager_->Connect();
+  ASSERT_FALSE(connect_cb_1.is_null());
+  ASSERT_TRUE(connect_cb_2.is_null());
+  base::ResetAndReturn(&connect_cb_1).Run(net::ERR_FAILED);
+  ASSERT_FALSE(connect_cb_2.is_null());
+  base::ResetAndReturn(&connect_cb_2).Run(net::ERR_FAILED);
+}
+
+}  // namespace blimp
diff --git a/blimp/net/engine_connection_manager.cc b/blimp/net/engine_connection_manager.cc
index 549caa3..053508e 100644
--- a/blimp/net/engine_connection_manager.cc
+++ b/blimp/net/engine_connection_manager.cc
@@ -13,7 +13,9 @@ namespace blimp {
 
 EngineConnectionManager::EngineConnectionManager(
     ConnectionHandler* connection_handler)
-    : connection_handler_(connection_handler) {}
+    : connection_handler_(connection_handler) {
+  DCHECK(connection_handler_);
+}
 
 EngineConnectionManager::~EngineConnectionManager() {}