Refactor BlimpConnection to TCPConnection

blimp/net/engine_connection_manager.h

 // 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.
 
 #ifndef BLIMP_NET_ENGINE_CONNECTION_MANAGER_H_
 #define BLIMP_NET_ENGINE_CONNECTION_MANAGER_H_
 
 #include <memory>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "blimp/net/blimp_net_export.h"
 #include "blimp/net/connection_handler.h"
+#include "net/base/ip_address.h"

[Garrett Casto, 2016/10/25 18:33:50]

Doesn't look like this is used.

[perumaal, 2016/10/25 23:02:39]

How do I check for unnecessary includes using git cl?

+#include "net/base/ip_endpoint.h"
+#include "net/base/net_errors.h"

[Garrett Casto, 2016/10/25 18:33:50]

Or this.

[perumaal, 2016/10/25 23:02:39]

Done.

+#include "net/log/net_log.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 BLIMP_NET_EXPORT EngineConnectionManager {
  public:
+  enum class EngineTransportType { TCP, GRPC };
+
   // Caller is responsible for ensuring that |connection_handler| outlives
   // |this|.
-  explicit EngineConnectionManager(ConnectionHandler* connection_handler);
+  explicit EngineConnectionManager(ConnectionHandler* connection_handler,
+                                   net::NetLog* net_log);
 
   ~EngineConnectionManager();
 
   // Adds a transport and accepts new BlimpConnections from it as fast as they
   // arrive.
-  void AddTransport(std::unique_ptr<BlimpTransport> transport);
+  void ConnectTransport(const net::IPEndPoint& ip_endpoint,

[Kevin M, 2016/10/25 18:49:23]

Suggestion: just take a std::unique_ptr<BlimpTransport> supplied by the caller
rather than using an enum. The lookup is unnecessary and it's a natural
interface for introducing loose coupling.

[perumaal, 2016/10/25 23:02:39]

I would really like the EngineConnectionManager to be the factory. This
decouples the callers from having to worry about the specifics of tcp vs grpc or
duplicate this logic.

Also, please note that this is going away once gRPC is settled (and the sole
connection facility).

+                        EngineTransportType transport_type);
 
  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<std::unique_ptr<BlimpTransport>> transports_;
+  net::NetLog* net_log_;
+  std::unique_ptr<BlimpTransport> transport_;
 
   DISALLOW_COPY_AND_ASSIGN(EngineConnectionManager);
 };
 
 }  // namespace blimp
 
 #endif  // BLIMP_NET_ENGINE_CONNECTION_MANAGER_H_