Implementation and fixes for Blimp client/engine E2E communication.

blimp/net/client_connection_manager.cc

Index: blimp/net/client_connection_manager.cc
diff --git a/blimp/net/client_connection_manager.cc b/blimp/net/client_connection_manager.cc
index 06266965d6e1e9b5fda8ef9993069ee4bb1caf44..2dd15024e789e14fdd23cd33505933674dd4ce67 100644
--- a/blimp/net/client_connection_manager.cc
+++ b/blimp/net/client_connection_manager.cc
@@ -18,7 +18,7 @@ namespace blimp {
 
 ClientConnectionManager::ClientConnectionManager(
     ConnectionHandler* connection_handler)
-    : connection_handler_(connection_handler) {
+    : connection_handler_(connection_handler), weak_factory_(this) {
   DCHECK(connection_handler_);
 }
 
@@ -38,6 +38,7 @@ void ClientConnectionManager::Connect() {
 }
 
 void ClientConnectionManager::Connect(int transport_index) {
+  DVLOG(1) << "ClientConnectionManager::Connect(" << transport_index << ")";
   if (static_cast<size_t>(transport_index) < transports_.size()) {
     transports_[transport_index]->Connect(
         base::Bind(&ClientConnectionManager::OnConnectResult,
@@ -53,8 +54,8 @@ void ClientConnectionManager::OnConnectResult(int transport_index, int result) {
   const auto& transport = transports_[transport_index];
   if (result == net::OK) {
     scoped_ptr<BlimpConnection> connection = transport->TakeConnection();
-    SendAuthenticationMessage(connection.get());
-    connection_handler_->HandleConnection(std::move(connection));
+    connection->SetConnectionErrorObserver(this);

[haibinlu, 2015/12/29 00:51:46]

ClientConnectionManager will then rely on error observer to re-connection on
connection error. but the next connection handler will overwrite the error
observer.

[Kevin M, 2015/12/30 23:08:49]

Good point. ClientConnectionManager will retain the observer.

+    SendAuthenticationMessage(std::move(connection));
   } else {
     DVLOG(1) << "Transport " << transport->GetName()
              << " failed to connect:" << net::ErrorToString(result);
@@ -63,12 +64,33 @@ void ClientConnectionManager::OnConnectResult(int transport_index, int result) {
 }
 
 void ClientConnectionManager::SendAuthenticationMessage(
-    BlimpConnection* connection) {
+    scoped_ptr<BlimpConnection> connection) {
   // TODO(haibinlu): get client token.

[haibinlu, 2015/12/29 00:51:46]

remove this TODO

[Kevin M, 2015/12/30 23:08:49]

Done.

-  const char* client_token = "";
+  DVLOG(1) << "Sending authentication message.";
   connection->GetOutgoingMessageProcessor()->ProcessMessage(
-      CreateStartConnectionMessage(client_token, kProtocolVersion),
-      net::CompletionCallback());
+      CreateStartConnectionMessage(client_token_, kProtocolVersion),
+      base::Bind(&ClientConnectionManager::AuthenticationMessageSent,

[haibinlu, 2015/12/29 00:51:46]

OnAuthenticationMessageSent?

[Kevin M, 2015/12/30 23:08:49]

Done.

+                 weak_factory_.GetWeakPtr(),
+                 base::Passed(std::move(connection))));

[haibinlu, 2015/12/29 00:51:46]

No need for std::move with base::Passed

[Kevin M, 2015/12/30 23:08:49]

Passed requires a move-only type as its input, which you only get with Pass() or
std::move().

+}
+
+void ClientConnectionManager::AuthenticationMessageSent(
+    scoped_ptr<BlimpConnection> connection,
+    int result) {
+  DVLOG(1) << "AuthenticationMessageSent, result=" << result;
+  if (result != net::OK) {
+    // If a write error occurred, just throw away |connection|.
+    // We don't need to propagate the error code here because the connection
+    // will already have done so via the ErrorObserver object.
+    return;
+  }
+  connection_handler_->HandleConnection(std::move(connection));
+}
+
+void ClientConnectionManager::OnConnectionError(int error) {
+  // TODO(kmarshall): Replace this with actual reconnection logic.
+  VLOG(0) << "Connection dropped, error=" << error;
+  Connect();
 }
 
 }  // namespace blimp