[GCM] Add connection factory for creating MCS connections

google_apis/gcm/engine/connection_factory_impl.cc

+// Copyright (c) 2013 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 "google_apis/gcm/engine/connection_factory_impl.h"
+
+#include "base/message_loop/message_loop.h"
+#include "google_apis/gcm/engine/connection_handler_impl.h"
+#include "google_apis/gcm/protocol/mcs.pb.h"
+#include "net/base/net_errors.h"
+#include "net/http/http_network_session.h"
+#include "net/http/http_request_headers.h"
+#include "net/proxy/proxy_info.h"
+#include "net/socket/client_socket_handle.h"
+#include "net/socket/client_socket_pool_manager.h"
+#include "net/ssl/ssl_config_service.h"
+
+namespace gcm {
+
+namespace {
+
+// The amount of time a Socket read should wait before timing out.
+const int kReadTimeoutMs = 30000;  // 30 seconds.
+
+// Backoff policy.
+const net::BackoffEntry::Policy kConnectionBackoffPolicy = {
+  // Number of initial errors (in sequence) to ignore before applying
+  // exponential back-off rules.
+  0,
+
+  // Initial delay for exponential back-off in ms.
+  10000,  // 10 seconds.
+
+  // Factor by which the waiting time will be multiplied.
+  2,
+
+  // Fuzzing percentage. ex: 10% will spread requests randomly
+  // between 90%-100% of the calculated time.
+  0.2, // 20%.
+
+  // Maximum amount of time we are willing to delay our request in ms.
+  1000 * 3600 * 4, // 4 hours.
+
+  // Time to keep an entry from being discarded even when it
+  // has no significant state, -1 to never discard.
+  -1,
+
+  // Don't use initial delay unless the last request was an error.
+  false,
+};
+
+// Callback for hostname resolution attempts. Just returns an error if no
+// addresses were resolved.
+int OnResolutionDone(const net::AddressList& address_list,
+                     const net::BoundNetLog& net_log) {
+  if (address_list.empty()) {
+    LOG(ERROR) << "Failed to resolve hostname.";
+    return net::ERR_NAME_RESOLUTION_FAILED;
+  }
+  DVLOG(1) << "MCS Hostname resolution success.";
+  return net::OK;
+}
+
+}  // namespace
+
+ConnectionFactoryImpl::ConnectionFactoryImpl(
+    const GURL& mcs_endpoint,
+    scoped_refptr<net::HttpNetworkSession> network_session,
+    net::NetLog* net_log)
+  : mcs_endpoint_(mcs_endpoint),
+    network_session_(network_session),
+    net_log_(net_log),
+    backoff_entry_(&kConnectionBackoffPolicy),
+    weak_ptr_factory_(this) {
+}
+
+ConnectionFactoryImpl::~ConnectionFactoryImpl() {
+}
+
+ConnectionHandler* ConnectionFactoryImpl::BuildConnectionHandler(
+      const ConnectionHandler::ProtoReceivedCallback& read_callback,
+      const ConnectionHandler::ProtoSentCallback& write_callback) {
+  DCHECK(!connection_handler_.get());

[akalin, 2013/11/02 00:15:43]

i feel like you should do:

if (!connection_handler) {
  NOTREACHED();
  return;
}

so that we don't add ourselves more than once as an observer (unless NCN already
handles that -- check?)

[Nicolas Zea, 2013/11/15 23:37:35]

Assuming you mean if (connection_handler_), done.

+  net::NetworkChangeNotifier::AddIPAddressObserver(this);
+  net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  connection_handler_.reset(
+      new ConnectionHandlerImpl(
+          base::TimeDelta::FromMilliseconds(kReadTimeoutMs),
+          read_callback,
+          write_callback,
+          base::Bind(&ConnectionFactoryImpl::ConnectionHandlerCallback,
+                     weak_ptr_factory_.GetWeakPtr())));
+  return connection_handler_.get();
+}
+
+void ConnectionFactoryImpl::Connect(
+    const mcs_proto::LoginRequest& login_request) {
+  DCHECK(connection_handler_.get());

[akalin, 2013/11/02 00:15:43]

no .get()

[Nicolas Zea, 2013/11/15 23:37:35]

Done.

+  DCHECK(!IsEndpointReachable());
+
+  login_request_ = login_request;
+
+  ConnectImpl();
+}
+
+bool ConnectionFactoryImpl::IsEndpointReachable() const {

[akalin, 2013/11/02 00:15:43]

no .get() (and everywhere else in a boolean context)

[Nicolas Zea, 2013/11/15 23:37:35]

Done.

+  return connection_handler_.get() && connection_handler_->CanSendMessage();
+}
+
+void ConnectionFactoryImpl::OnConnectionTypeChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  // TODO(zea): implement different backoff/retry policies based on connection
+  // type.
+  DVLOG(1) << "Connection type changed to " << type << ", resetting backoff.";
+  backoff_entry_.Reset();
+  // ConnectImpl() should be retrying with backoff already if a connection is
+  // necessary, so no need to call again.
+}
+
+void ConnectionFactoryImpl::OnIPAddressChanged() {
+  DVLOG(1) << "IP Address changed, resetting backoff.";
+  backoff_entry_.Reset();
+  // ConnectImpl() should be retrying with backoff already if a connection is
+  // necessary, so no need to call again.
+}
+
+void ConnectionFactoryImpl::ConnectImpl() {
+  DCHECK(!IsEndpointReachable());
+
+  if (backoff_entry_.ShouldRejectRequest()) {

[akalin, 2013/11/02 00:15:43]

i feel like this logic should be moved into ConnectImpl, just so the logic is
clearer, i.e.:

if (should reject) {
  PostDelayedTask(ConnectImpl);
} else {
  ConnectImpl();
}

[Nicolas Zea, 2013/11/15 23:37:35]

Assuming you mean move to Connect, done.

+    DVLOG(1) << "Delaying MCS endpoint connection for "
+             << backoff_entry_.GetTimeUntilRelease().InSeconds() << " seconds.";
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ConnectionFactoryImpl::ConnectImpl,
+                   weak_ptr_factory_.GetWeakPtr()),
+        backoff_entry_.GetTimeUntilRelease());
+    return;
+  }
+
+  DVLOG(1) << "Attempting connection to MCS endpoint.";
+
+  // TODO(zea): resolve proxies.
+  net::ProxyInfo proxy_info;
+  proxy_info.UseDirect();
+  net::SSLConfig ssl_config;
+  network_session_->ssl_config_service()->GetSSLConfig(&ssl_config);
+
+  // TODO(zea): This should probably be a raw request, but there's no good

[akalin, 2013/11/02 00:15:43]

yeah, I think this should be a raw request. having the mcs_endpoint_ URL start
with https doesn't suffice?

[Nicolas Zea, 2013/11/15 23:37:35]

Raw connect takes a host/port pair, and prepends a "http://" onto them to create
a GURL, so ssl is never enabled. That said, it only appears to be used in the
jingle code, so that could maybe be changed?

+  // way to force SSL for a raw request yet. Instead, pretend to be an https
+  // request (which provides hostname resolution as well).
+  int status = net::InitSocketHandleForHttpRequest(
+      mcs_endpoint_,
+      net::HttpRequestHeaders(),
+      0,                                           // No load flags.
+      net::MEDIUM,                                 // Normal priority.
+      network_session_.get(),
+      proxy_info,
+      false,                                       // Don't force SPDY over SSL.
+      false,                                       // Don't want SPDY over NPN.
+      ssl_config,
+      ssl_config,
+      net::kPrivacyModeDisabled,
+      net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_SOCKET),
+      &socket_handle_,
+      base::Bind(&OnResolutionDone),
+      base::Bind(&ConnectionFactoryImpl::OnConnectDone,
+                 weak_ptr_factory_.GetWeakPtr()));
+  if (status != net::ERR_IO_PENDING)
+    OnConnectDone(status);
+}
+
+void ConnectionFactoryImpl::OnConnectDone(int result) {
+  if (result != net::OK) {
+    LOG(ERROR) << "Failed to connect to MCS endpoint with error " << result;
+    backoff_entry_.InformOfRequest(false);
+    ConnectImpl();
+    return;
+  }
+
+  DVLOG(1) << "MCS endpoint connection success.";
+
+  // Reset the backoff.
+  backoff_entry_.Reset();
+
+  // Login to the server.
+  connection_handler_->Init(login_request_, socket_handle_.PassSocket());
+}
+
+void ConnectionFactoryImpl::ConnectionHandlerCallback(int result) {
+  // TODO(zea): Consider how to handle errors that may require some sort of
+  // user intervention (login page, etc.).
+  LOG(ERROR) << "Connection reset with error " << result;
+  backoff_entry_.InformOfRequest(false);
+  ConnectImpl();
+}
+
+}  // namespace gcm