Implementation and fixes for Blimp client/engine E2E communication.

blimp/net/engine_authentication_handler.cc

Index: blimp/net/engine_authentication_handler.cc
diff --git a/blimp/net/engine_authentication_handler.cc b/blimp/net/engine_authentication_handler.cc
index e49f5ea9c049027745eea67580678107952f9feb..0470e4c21af32f11925b29972787f818850a3637 100644
--- a/blimp/net/engine_authentication_handler.cc
+++ b/blimp/net/engine_authentication_handler.cc
@@ -64,6 +64,7 @@ Authenticator::Authenticator(
     base::WeakPtr<ConnectionHandler> connection_handler)
     : connection_(std::move(connection)),
       connection_handler_(connection_handler) {
+  DVLOG(1) << "Authenticator object created.";
   connection_->SetConnectionErrorObserver(this);
   connection_->SetIncomingMessageProcessor(this);
   timeout_timer_.Start(
@@ -74,13 +75,13 @@ Authenticator::Authenticator(
 Authenticator::~Authenticator() {}
 
 void Authenticator::OnConnectionAuthenticated(bool authenticated) {
-  connection_->SetIncomingMessageProcessor(nullptr);
-  connection_->SetConnectionErrorObserver(nullptr);
-
+  DVLOG(1) << "OnConnectionAuthenticated result=" << authenticated;
   if (authenticated && connection_handler_) {
+    // We expect |connection_handler_| to provide |connection_| with its own
+    // ConnectionErrorObserver and IncomingMessageProcessor objects,

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

I prefer to set nullptr as before, rather than rely on the next connection
handler to set it synchronously. And we do not really require "HandleConnection"
to synchronously attach error observer and incoming message processor. 

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

Removed ErrorObserver from here.

I'm not sure I understand the reasoning for unsetting them before setting them
again?

+    // so there is no need to unset them beforehand.
     connection_handler_->HandleConnection(std::move(connection_));
   }
-
   delete this;
 }
 
@@ -98,10 +99,12 @@ void Authenticator::OnConnectionError(int error) {
 void Authenticator::ProcessMessage(scoped_ptr<BlimpMessage> message,
                                    const net::CompletionCallback& callback) {
   if (message->type() == BlimpMessage::PROTOCOL_CONTROL) {
-    // TODO(haibinlu): check client token.

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

keep TODO since you are not verifying the client token.

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

Done.

+    DVLOG(1) << "Authentication challenge received: "
+             << message->protocol_control().start_connection().client_token();
     OnConnectionAuthenticated(true);
   } else {
-    DVLOG(1) << "The first message is not START_CONNECTION";
+    DVLOG(1) << "The first message is not START_CONNECTION; got type "
+             << message->type() << " instead.";
     OnConnectionAuthenticated(false);
   }