[Blimp Net] Add EngineAuthHandler.

blimp/net/engine_auth_handler.cc

+// 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.
+
+#include "blimp/net/engine_auth_handler.h"
+
+#include "base/callback_helpers.h"
+#include "base/logging.h"
+#include "blimp/common/proto/blimp_message.pb.h"
+#include "blimp/net/blimp_connection.h"
+#include "blimp/net/blimp_transport.h"
+#include "net/base/net_errors.h"
+
+namespace blimp {
+
+namespace {
+// Expect authentication to be done in 10 seconds.
+const double kAuthTimeoutDurationInSeconds = 10;
+}  // namespace
+
+EngineAuthHandler::EngineAuthHandler(ConnectionHandler* connection_handler)
+    : connection_handler_(connection_handler) {
+  DCHECK(connection_handler_);
+}
+
+EngineAuthHandler::~EngineAuthHandler() {}
+
+void EngineAuthHandler::HandleConnection(
+    scoped_ptr<BlimpConnection> connection) {
+  scoped_ptr<Authenticator> authenticator(
+      new Authenticator(std::move(connection)));
+  authenticator->Start(base::Bind(&EngineAuthHandler::OnAuthResult,
+                                  base::Unretained(this),
+                                  base::Unretained(authenticator.get())));

[Wez, 2015/12/04 01:35:38]

Why do we have a separate Start() call, rather than passing the OnAuthResult
callback directly to the Authenticator constructor?

[haibinlu, 2015/12/04 02:50:40]

removed

+  pending_auths_.push_back(std::move(authenticator));
+}
+
+void EngineAuthHandler::OnAuthResult(Authenticator* authenticator,
+                                     scoped_ptr<BlimpConnection> connection,
+                                     bool authenticated) {
+  if (authenticated) {
+    connection_handler_->HandleConnection(std::move(connection));
+  }
+
+  RemovePendingAuthenticator(authenticator);
+}
+
+void EngineAuthHandler::RemovePendingAuthenticator(
+    Authenticator* authenticator) {
+  auto it = pending_auths_.begin();
+  for (; it != pending_auths_.end(); ++it) {
+    if (it->get() == authenticator)
+      break;
+  }
+  DCHECK(it != pending_auths_.end());
+  pending_auths_.erase(it);
+}
+
+EngineAuthHandler::Authenticator::Authenticator(
+    scoped_ptr<BlimpConnection> connection)
+    : connection_(std::move(connection)) {
+  DCHECK(connection_);
+}
+
+EngineAuthHandler::Authenticator::~Authenticator() {}
+
+void EngineAuthHandler::Authenticator::Start(
+    const AuthCallback& auth_callback) {
+  auth_callback_ = auth_callback;
+  connection_->SetConnectionErrorObserver(this);
+  connection_->SetIncomingMessageProcessor(this);
+  timer_.Start(FROM_HERE,
+               base::TimeDelta::FromSecondsD(kAuthTimeoutDurationInSeconds),
+               this, &EngineAuthHandler::Authenticator::FailAuthetication);
+}
+
+void EngineAuthHandler::Authenticator::OnConnectionError(int error) {
+  DVLOG(1) << "Connection error before authenticated";
+  InvokeAuthCallback(false);
+}
+
+void EngineAuthHandler::Authenticator::ProcessMessage(
+    scoped_ptr<BlimpMessage> message,
+    const net::CompletionCallback& callback) {
+  if (message->type() == BlimpMessage::PROTOCOL_CONTROL) {

[Wez, 2015/12/04 01:35:38]

If you called SetIncomingMessageProcessor(null) on the connection here then that
should be sufficient to avoid further messages being pumped until a new
processor is attached to the connection, right?

[haibinlu, 2015/12/04 02:50:40]

 Added, though need to restore the ability to SetIncomingMessageProcessor to
null in BlimpConnection.

+    // TODO(haibinlu): check client token.
+    InvokeAuthCallback(true);
+    callback.Run(net::OK);

[Wez, 2015/12/04 01:35:38]

Why are you only acknowledging the message in case of authentication success?

[haibinlu, 2015/12/04 02:50:40]

Done.

+  } else {
+    InvokeAuthCallback(false);
+  }
+}
+
+void EngineAuthHandler::Authenticator::InvokeAuthCallback(bool authenticated) {
+  DCHECK(!auth_callback_.is_null());
+  timer_.Stop();
+  base::ResetAndReturn(&auth_callback_)
+      .Run(std::move(connection_), authenticated);
+}
+
+void EngineAuthHandler::Authenticator::FailAuthetication() {
+  DVLOG(1) << "Connection authentication timeout";

[Wez, 2015/12/04 01:35:38]

If this is specific to timeout, it should be called OnAuthenticationTimeout,
surely? Or just bind the timer directly to InvokeAuthCallback(false)?

+  InvokeAuthCallback(false);
+}
+
+}  // namespace blimp