[GCM] Add port 443 fallback logic and histograms

google_apis/gcm/engine/connection_factory_impl.cc

Index: google_apis/gcm/engine/connection_factory_impl.cc
diff --git a/google_apis/gcm/engine/connection_factory_impl.cc b/google_apis/gcm/engine/connection_factory_impl.cc
index ebfd367c9a85c571da3b3914477dab303e1fc673..631fcae5491962eb201dfdb5ca654ebdb84efdaa 100644
--- a/google_apis/gcm/engine/connection_factory_impl.cc
+++ b/google_apis/gcm/engine/connection_factory_impl.cc
@@ -41,11 +41,13 @@ bool ShouldRestorePreviousBackoff(const base::TimeTicks& login_time,
 }  // 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_(
@@ -54,6 +56,7 @@ ConnectionFactoryImpl::ConnectionFactoryImpl(
     connecting_(false),
     logging_in_(false),
     weak_ptr_factory_(this) {
+  DCHECK_GE(mcs_endpoints_.size(), 1U);
 }
 
 ConnectionFactoryImpl::~ConnectionFactoryImpl() {
@@ -184,12 +187,18 @@ void ConnectionFactoryImpl::OnIPAddressChanged() {
   // 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()),
@@ -234,13 +243,28 @@ void ConnectionFactoryImpl::OnConnectDone(int result) {
     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.";
@@ -300,7 +324,7 @@ void ConnectionFactoryImpl::OnProxyResolveDone(int status) {
   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,
@@ -374,7 +398,7 @@ int ConnectionFactoryImpl::ReconsiderProxyAfterError(int error) {
   }
 
   int status = network_session_->proxy_service()->ReconsiderProxyAfterError(
-      mcs_endpoint_, &proxy_info_,
+      GetCurrentEndpoint(), &proxy_info_,
       base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone,
                  weak_ptr_factory_.GetWeakPtr()),
       &pac_request_,