Refactor BlimpConnection to TCPConnection

blimp/net/engine_connection_manager.cc

Index: blimp/net/engine_connection_manager.cc
diff --git a/blimp/net/engine_connection_manager.cc b/blimp/net/engine_connection_manager.cc
index 8e74fbdf7d05f3695926714ab4514ed503579575..88d8234ccbcc0f5ae0fb34bd731d6babab9771d1 100644
--- a/blimp/net/engine_connection_manager.cc
+++ b/blimp/net/engine_connection_manager.cc
@@ -11,37 +11,49 @@
 #include "blimp/net/blimp_connection.h"
 #include "blimp/net/blimp_transport.h"
 #include "blimp/net/message_port.h"
+#include "blimp/net/tcp_engine_transport.h"
+#include "net/base/ip_address.h"
 #include "net/base/net_errors.h"
 
 namespace blimp {
 
 EngineConnectionManager::EngineConnectionManager(
-    ConnectionHandler* connection_handler)
-    : connection_handler_(connection_handler) {
+    ConnectionHandler* connection_handler,
+    net::NetLog* net_log)
+    : connection_handler_(connection_handler), net_log_(net_log) {
   DCHECK(connection_handler_);
 }
 
 EngineConnectionManager::~EngineConnectionManager() {}
 
-void EngineConnectionManager::AddTransport(
-    std::unique_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::ConnectTransport(

[scf, 2016/11/10 21:59:09]

Rename to CreateAndConnectTransport?

[perumaal, 2016/11/11 01:50:55]

Good point.

+    net::IPEndPoint* ip_endpoint,
+    EngineTransportType transport_type) {
+  switch (transport_type) {
+    case EngineTransportType::TCP: {
+      transport_ = base::MakeUnique<TCPEngineTransport>(*ip_endpoint, net_log_);
+      break;
+    }
+
+    case EngineTransportType::GRPC: {
+      NOTIMPLEMENTED();
+      // TODO(perumaal): Unimplemented as yet.
+      // transport_ =
+      //      base::MakeUnique<GrpcEngineTransport>(ip_endpoint.ToString());
+      break;
+    }
+  }
+
+  transport_->Connect(base::Bind(&EngineConnectionManager::OnConnectResult,
+                                 base::Unretained(this),
+                                 base::Unretained(transport_.get())));

[Wez, 2016/11/09 23:25:49]

We should fix GetLocalAddress to trigger the Bind(), if not already bound,
rather than relying on the caller to know that they must Connect() first.

[Wez, 2016/11/10 01:25:06]

Scratch that; of course we need to kick-off Connect() calls here. D'oh!

[perumaal, 2016/11/10 02:05:05]

Yeah, another reason I tied the GetLocalAddress just after the Connect call.

+  transport_->GetLocalAddress(ip_endpoint);
 }
 
 void EngineConnectionManager::OnConnectResult(BlimpTransport* transport,
                                               int result) {
   CHECK_EQ(net::OK, result) << "Transport failure:" << transport->GetName();
-  connection_handler_->HandleConnection(
-      base::MakeUnique<BlimpConnection>(transport->TakeMessagePort()));
-  Connect(transport);
+  connection_handler_->HandleConnection(transport->MakeConnection());
 }
 
 }  // namespace blimp