[GCM] Add port 443 fallback logic and histograms

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 "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "google_apis/gcm/engine/connection_handler_impl.h"
@@ skipping 23 matching lines @@
 bool ShouldRestorePreviousBackoff(const base::TimeTicks& login_time,
                                   const base::TimeTicks& now_ticks) {
   return !login_time.is_null() &&
       now_ticks - login_time <=
           base::TimeDelta::FromSeconds(kConnectionResetWindowSecs);
 }
 
 }  // namespace
 
 ConnectionFactoryImpl::ConnectionFactoryImpl(
-    const GURL& mcs_endpoint,
+    const std::vector<GURL>& mcs_endpoints,
     const net::BackoffEntry::Policy& backoff_policy,
     scoped_refptr<net::HttpNetworkSession> network_session,
     net::NetLog* net_log)
-  : mcs_endpoint_(mcs_endpoint),
+  : mcs_endpoints_(mcs_endpoints),
+    next_endpoint_(0),
+    last_successful_endpoint_(0),
     backoff_policy_(backoff_policy),
     network_session_(network_session),
     bound_net_log_(
         net::BoundNetLog::Make(net_log, net::NetLog::SOURCE_SOCKET)),
     pac_request_(NULL),
     connecting_(false),
     logging_in_(false),
     weak_ptr_factory_(this) {
+  DCHECK_GE(mcs_endpoints_.size(), 1U);
 }
 
 ConnectionFactoryImpl::~ConnectionFactoryImpl() {
   if (pac_request_) {
     network_session_->proxy_service()->CancelPacRequest(pac_request_);
     pac_request_ = NULL;
   }
 }
 
 void ConnectionFactoryImpl::Initialize(
@@ skipping 110 matching lines @@
   // necessary, so no need to call again.
 }
 
 void ConnectionFactoryImpl::OnIPAddressChanged() {
   DVLOG(1) << "IP Address changed, resetting backoff.";
   backoff_entry_->Reset();
   // Connect(..) should be retrying with backoff already if a connection is
   // necessary, so no need to call again.
 }
 
+GURL ConnectionFactoryImpl::GetCurrentEndpoint() const {
+  if (IsEndpointReachable())
+    return mcs_endpoints_[last_successful_endpoint_];
+  return mcs_endpoints_[next_endpoint_];
+}
+
 void ConnectionFactoryImpl::ConnectImpl() {
   DCHECK(connecting_);
   DCHECK(!socket_handle_.socket());
 
   int status = network_session_->proxy_service()->ResolveProxy(
-      mcs_endpoint_,
+      GetCurrentEndpoint(),
       &proxy_info_,
       base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone,
                  weak_ptr_factory_.GetWeakPtr()),
       &pac_request_,
       bound_net_log_);
   if (status != net::ERR_IO_PENDING)
     OnProxyResolveDone(status);
 }
 
 void ConnectionFactoryImpl::InitHandler() {
@@ skipping 24 matching lines @@
     // not reconsidering a proxy) or returns ERR_IO_PENDING if it is considering
     // another proxy.
     DCHECK_NE(result, net::OK);
     if (result == net::ERR_IO_PENDING)
       return;  // Proxy reconsideration pending. Return.
     LOG(ERROR) << "Failed to connect to MCS endpoint with error " << result;
     UMA_HISTOGRAM_BOOLEAN("GCM.ConnectionSuccessRate", false);
     CloseSocket();
     backoff_entry_->InformOfRequest(false);
     UMA_HISTOGRAM_SPARSE_SLOWLY("GCM.ConnectionFailureErrorCode", result);
+
+    // If there are other endpoints available, use the next endpoint on the
+    // subsequent retry.
+    next_endpoint_++;

[jianli, 2014/03/20 23:10:21]

What if GetCurrentEndpoint() uses last_successful_endpoint_ and it failed? We're
indeed not trying next_endpoint_ but simply moving to the next one.

[Nicolas Zea, 2014/03/20 23:53:30]

While connecting, GetCurrentEndpoint() will not return
last_successful_endpoint_, due to IsEndpointReachable() being dependent on
!connecting_. Therefore, all connections are based on next_endpoint_.

[jianli, 2014/03/21 00:13:18]

Could you please add a comment in GetCurrentEndpoint?

[Nicolas Zea, 2014/03/21 00:35:10]

Done.

+    if (next_endpoint_ >= mcs_endpoints_.size())
+      next_endpoint_ = 0;
     Connect();
     return;
   }
 
   UMA_HISTOGRAM_BOOLEAN("GCM.ConnectionSuccessRate", true);
+  UMA_HISTOGRAM_COUNTS("GCM.ConnectionEndpoint", next_endpoint_);
+  UMA_HISTOGRAM_BOOLEAN("GCM.ConnectedViaProxy",
+                        !(proxy_info_.is_empty() || proxy_info_.is_direct()));
   ReportSuccessfulProxyConnection();
 
+  // Reset the endpoint back to the default.
+  // TODO(zea): consider prioritizing endpoints more intelligently based on
+  // which ones succeed most for this client? Although that will affect
+  // measuring the success rate of the default endpoint vs fallback.
+  last_successful_endpoint_ = next_endpoint_;

[jianli, 2014/03/20 23:10:21]

What if GetCurrentEndpoint() uses last_successful_endpoint_?

[Nicolas Zea, 2014/03/20 23:53:30]

See above.

+  next_endpoint_ = 0;
   connecting_ = false;
   logging_in_ = true;
   DVLOG(1) << "MCS endpoint socket connection success, starting login.";
   InitHandler();
 }
 
 void ConnectionFactoryImpl::ConnectionHandlerCallback(int result) {
   DCHECK(!connecting_);
   if (result != net::OK) {
     // TODO(zea): Consider how to handle errors that may require some sort of
@@ skipping 39 matching lines @@
     // Failed to resolve proxy. Retry later.
     OnConnectDone(status);
     return;
   }
 
   DVLOG(1) << "Resolved proxy with PAC:" << proxy_info_.ToPacString();
 
   net::SSLConfig ssl_config;
   network_session_->ssl_config_service()->GetSSLConfig(&ssl_config);
   status = net::InitSocketHandleForTlsConnect(
-      net::HostPortPair::FromURL(mcs_endpoint_),
+      net::HostPortPair::FromURL(GetCurrentEndpoint()),
       network_session_.get(),
       proxy_info_,
       ssl_config,
       ssl_config,
       net::kPrivacyModeDisabled,
       bound_net_log_,
       &socket_handle_,
       base::Bind(&ConnectionFactoryImpl::OnConnectDone,
                  weak_ptr_factory_.GetWeakPtr()));
   if (status != net::ERR_IO_PENDING)
@@ skipping 53 matching lines @@
   }
 
   net::SSLConfig ssl_config;
   network_session_->ssl_config_service()->GetSSLConfig(&ssl_config);
   if (proxy_info_.is_https() && ssl_config.send_client_cert) {
     network_session_->ssl_client_auth_cache()->Remove(
         proxy_info_.proxy_server().host_port_pair());
   }
 
   int status = network_session_->proxy_service()->ReconsiderProxyAfterError(
-      mcs_endpoint_, &proxy_info_,
+      GetCurrentEndpoint(), &proxy_info_,
       base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone,
                  weak_ptr_factory_.GetWeakPtr()),
       &pac_request_,
       bound_net_log_);
   if (status == net::OK || status == net::ERR_IO_PENDING) {
     CloseSocket();
   } else {
     // If ReconsiderProxyAfterError() failed synchronously, it means
     // there was nothing left to fall-back to, so fail the transaction
     // with the last connection error we got.
@@ skipping 22 matching lines @@
   // the destroyed socket.
   if (connection_handler_)
     connection_handler_->Reset();
 
   if (socket_handle_.socket() && socket_handle_.socket()->IsConnected())
     socket_handle_.socket()->Disconnect();
   socket_handle_.Reset();
 }
 
 }  // namespace gcm