Revert of Fix crashing bug in EngineAuthenticationHandler on token mismatch. (patchset #1 id:1 of https://codereview.chromium.org/1712083002/ )

Reason for revert:
This CL works-around an underlying bug in BlimpMessagePump, by implementing non-standard ProcessMessage() behaviour based on implicit knowledge of the calling BlimpMessagePump's lifetime. The underlying fix has been landed, so the workaround can now be removed.

Original issue's description:
> Fix crashing bug in EngineAuthenticationHandler on token mismatch.
>
> * Modify logic to only invoke the read callback on authentication
>   success. Previously, the callback was invoked even if the connection
>   was terminated due to auth failure. This lead to a segfault because
>   the callback was referencing the message pump that belonged to the
>   now-destroyed connection.
> * Restructure auth logic for clarity - success case at the end of the
>   function, with error cases returning early.
>
> R=haibinlu@chromium.org
> BUG=
>
> Committed: https://crrev.com/aec59869aca3417b913e4f57321ca730dcdcb20d
> Cr-Commit-Position: refs/heads/master@{#376499}

R=haibinlu@chromium.org,kmarshall@chromium.org
BUG=588195

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

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

2 files changed, 13 insertions, 22 deletions

diff --git a/blimp/net/engine_authentication_handler.cc b/blimp/net/engine_authentication_handler.cc
index 39037b5..41813fc 100644
--- a/blimp/net/engine_authentication_handler.cc
+++ b/blimp/net/engine_authentication_handler.cc
@@ -4,8 +4,6 @@
 
 #include "blimp/net/engine_authentication_handler.h"
 
-#include <string>
-
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/timer/timer.h"
@@ -110,30 +108,23 @@ void Authenticator::OnConnectionError(int error) {
 
 void Authenticator::ProcessMessage(scoped_ptr<BlimpMessage> message,
                                    const net::CompletionCallback& callback) {
-  if (message->type() != BlimpMessage::PROTOCOL_CONTROL ||
-      message->protocol_control().type() !=
+  if (message->type() == BlimpMessage::PROTOCOL_CONTROL &&
+      message->protocol_control().type() ==
           ProtocolControlMessage::START_CONNECTION) {
+    bool token_match =
+        client_token_ ==
+        message->protocol_control().start_connection().client_token();
+    DVLOG(1) << "Authentication challenge received: "
+             << message->protocol_control().start_connection().client_token()
+             << ", and token "
+             << (token_match ? " matches" : " does not match");
+    OnConnectionAuthenticated(token_match);
+  } else {
     DVLOG(1) << "Expected START_CONNECTION message, got " << *message
              << " instead.";
     OnConnectionAuthenticated(false);
-    return;
-  }
-
-  bool token_matches =
-      client_token_ ==
-      message->protocol_control().start_connection().client_token();
-  DVLOG(1) << "Authentication challenge received: "
-           << message->protocol_control().start_connection().client_token()
-           << ", and token "
-           << (token_matches ? " matches" : " does not match");
-  if (!token_matches) {
-    OnConnectionAuthenticated(false);
-    return;
   }
 
-  // Authentication succeeded!
-  // Run |callback| to signal the pump to read more messages.
-  OnConnectionAuthenticated(true);
   callback.Run(net::OK);
 }
 
diff --git a/blimp/net/engine_authentication_handler_unittest.cc b/blimp/net/engine_authentication_handler_unittest.cc
index 9dc52b3..dd8943c 100644
--- a/blimp/net/engine_authentication_handler_unittest.cc
+++ b/blimp/net/engine_authentication_handler_unittest.cc
@@ -84,7 +84,7 @@ TEST_F(EngineAuthenticationHandlerTest, AuthenticationFailed) {
   net::TestCompletionCallback process_message_cb;
   incoming_message_processor_->ProcessMessage(std::move(blimp_message),
                                               process_message_cb.callback());
-  EXPECT_FALSE(process_message_cb.have_result());
+  EXPECT_EQ(net::OK, process_message_cb.WaitForResult());
 }
 
 TEST_F(EngineAuthenticationHandlerTest, WrongMessageReceived) {
@@ -96,7 +96,7 @@ TEST_F(EngineAuthenticationHandlerTest, WrongMessageReceived) {
   net::TestCompletionCallback process_message_cb;
   incoming_message_processor_->ProcessMessage(std::move(blimp_message),
                                               process_message_cb.callback());
-  EXPECT_FALSE(process_message_cb.have_result());
+  EXPECT_EQ(net::OK, process_message_cb.WaitForResult());
 }
 
 TEST_F(EngineAuthenticationHandlerTest, ConnectionError) {