Revert 258720 "[GCM] Add port 443 fallback logic and histograms"

This broke linux Clang compile.
gcm_tools_mcs_probe.cc line 294


> [GCM] Add port 443 fallback logic and histograms
> 
> Alternate between the default (5228) endpoint and the fallback (443) endpoint.
> Also introduce histograms for tracking endpoint and proxy success rate.
> 
> BUG=351890
> 
> Review URL: https://codereview.chromium.org/205343003

TBR=zea@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258735 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 12 insertions, 67 deletions

diff --git a/google_apis/gcm/engine/connection_factory_impl.cc b/google_apis/gcm/engine/connection_factory_impl.cc
index deb8102..ebfd367 100644
--- a/google_apis/gcm/engine/connection_factory_impl.cc
+++ b/google_apis/gcm/engine/connection_factory_impl.cc
@@ -41,13 +41,11 @@ bool ShouldRestorePreviousBackoff(const base::TimeTicks& login_time,
 }  // namespace
 
 ConnectionFactoryImpl::ConnectionFactoryImpl(
-    const std::vector<GURL>& mcs_endpoints,
+    const GURL& mcs_endpoint,
     const net::BackoffEntry::Policy& backoff_policy,
     scoped_refptr<net::HttpNetworkSession> network_session,
     net::NetLog* net_log)
-  : mcs_endpoints_(mcs_endpoints),
-    next_endpoint_(0),
-    last_successful_endpoint_(0),
+  : mcs_endpoint_(mcs_endpoint),
     backoff_policy_(backoff_policy),
     network_session_(network_session),
     bound_net_log_(
@@ -56,7 +54,6 @@ ConnectionFactoryImpl::ConnectionFactoryImpl(
     connecting_(false),
     logging_in_(false),
     weak_ptr_factory_(this) {
-  DCHECK_GE(mcs_endpoints_.size(), 1U);
 }
 
 ConnectionFactoryImpl::~ConnectionFactoryImpl() {
@@ -187,20 +184,12 @@ void ConnectionFactoryImpl::OnIPAddressChanged() {
   // necessary, so no need to call again.
 }
 
-GURL ConnectionFactoryImpl::GetCurrentEndpoint() const {
-  // Note that IsEndpointReachable() returns false anytime connecting_ is true,
-  // so while connecting this always uses |next_endpoint_|.
-  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(
-      GetCurrentEndpoint(),
+      mcs_endpoint_,
       &proxy_info_,
       base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone,
                  weak_ptr_factory_.GetWeakPtr()),
@@ -245,28 +234,13 @@ 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_++;
-    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_;
-  next_endpoint_ = 0;
   connecting_ = false;
   logging_in_ = true;
   DVLOG(1) << "MCS endpoint socket connection success, starting login.";
@@ -326,7 +300,7 @@ void ConnectionFactoryImpl::OnProxyResolveDone(int status) {
   net::SSLConfig ssl_config;
   network_session_->ssl_config_service()->GetSSLConfig(&ssl_config);
   status = net::InitSocketHandleForTlsConnect(
-      net::HostPortPair::FromURL(GetCurrentEndpoint()),
+      net::HostPortPair::FromURL(mcs_endpoint_),
       network_session_.get(),
       proxy_info_,
       ssl_config,
@@ -400,7 +374,7 @@ int ConnectionFactoryImpl::ReconsiderProxyAfterError(int error) {
   }
 
   int status = network_session_->proxy_service()->ReconsiderProxyAfterError(
-      GetCurrentEndpoint(), &proxy_info_,
+      mcs_endpoint_, &proxy_info_,
       base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone,
                  weak_ptr_factory_.GetWeakPtr()),
       &pac_request_,
diff --git a/google_apis/gcm/engine/connection_factory_impl.h b/google_apis/gcm/engine/connection_factory_impl.h
index 28c2941..967c6c1 100644
--- a/google_apis/gcm/engine/connection_factory_impl.h
+++ b/google_apis/gcm/engine/connection_factory_impl.h
@@ -32,7 +32,7 @@ class GCM_EXPORT ConnectionFactoryImpl :
     public net::NetworkChangeNotifier::IPAddressObserver {
  public:
   ConnectionFactoryImpl(
-      const std::vector<GURL>& mcs_endpoints,
+      const GURL& mcs_endpoint,
       const net::BackoffEntry::Policy& backoff_policy,
       scoped_refptr<net::HttpNetworkSession> network_session,
       net::NetLog* net_log);
@@ -54,11 +54,6 @@ class GCM_EXPORT ConnectionFactoryImpl :
       net::NetworkChangeNotifier::ConnectionType type) OVERRIDE;
   virtual void OnIPAddressChanged() OVERRIDE;
 
-  // Returns the server to which the factory is currently connected, or if
-  // a connection is currently pending, the server to which the next connection
-  // attempt will be made.
-  GURL GetCurrentEndpoint() const;
-
  protected:
   // Implementation of Connect(..). If not in backoff, uses |login_request_|
   // in attempting a connection/handshake. On connection/handshake failure, goes
@@ -94,12 +89,8 @@ class GCM_EXPORT ConnectionFactoryImpl :
 
   void CloseSocket();
 
-  // The MCS endpoints to make connections to, sorted in order of priority.
-  const std::vector<GURL> mcs_endpoints_;
-  // Index to the endpoint for which a connection should be attempted next.
-  size_t next_endpoint_;
-  // Index to the endpoint that was last successfully connected.
-  size_t last_successful_endpoint_;
+  // The MCS endpoint to make connections to.
+  const GURL mcs_endpoint_;
 
   // The backoff policy to use.
   const net::BackoffEntry::Policy backoff_policy_;
diff --git a/google_apis/gcm/engine/connection_factory_impl_unittest.cc b/google_apis/gcm/engine/connection_factory_impl_unittest.cc
index eebe14a..82c3f18 100644
--- a/google_apis/gcm/engine/connection_factory_impl_unittest.cc
+++ b/google_apis/gcm/engine/connection_factory_impl_unittest.cc
@@ -19,7 +19,6 @@ namespace gcm {
 namespace {
 
 const char kMCSEndpoint[] = "http://my.server";
-const char kMCSEndpoint2[] = "http://my.alt.server";
 
 const int kBackoffDelayMs = 1;
 const int kBackoffMultiplier = 2;
@@ -51,13 +50,6 @@ const net::BackoffEntry::Policy kTestBackoffPolicy = {
   false,
 };
 
-std::vector<GURL> BuildEndpoints() {
-  std::vector<GURL> endpoints;
-  endpoints.push_back(GURL(kMCSEndpoint));
-  endpoints.push_back(GURL(kMCSEndpoint2));
-  return endpoints;
-}
-
 // Helper for calculating total expected exponential backoff delay given an
 // arbitrary number of failed attempts. See BackoffEntry::CalculateReleaseTime.
 double CalculateBackoff(int num_attempts) {
@@ -140,7 +132,7 @@ class TestConnectionFactoryImpl : public ConnectionFactoryImpl {
 
 TestConnectionFactoryImpl::TestConnectionFactoryImpl(
     const base::Closure& finished_callback)
-    : ConnectionFactoryImpl(BuildEndpoints(),
+    : ConnectionFactoryImpl(GURL(kMCSEndpoint),
                             net::BackoffEntry::Policy(),
                             NULL,
                             NULL),
@@ -262,11 +254,9 @@ TEST_F(ConnectionFactoryImplTest, ConnectSuccess) {
   factory()->SetConnectResult(net::OK);
   factory()->Connect();
   EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
-  EXPECT_EQ(factory()->GetCurrentEndpoint(), BuildEndpoints()[0]);
 }
 
-// A connection failure should result in backoff, and attempting the fallback
-// endpoint next.
+// A connection failure should result in backoff.
 TEST_F(ConnectionFactoryImplTest, ConnectFail) {
   factory()->Initialize(
       ConnectionFactory::BuildLoginRequestCallback(),
@@ -275,7 +265,6 @@ TEST_F(ConnectionFactoryImplTest, ConnectFail) {
   factory()->SetConnectResult(net::ERR_CONNECTION_FAILED);
   factory()->Connect();
   EXPECT_FALSE(factory()->NextRetryAttempt().is_null());
-  EXPECT_EQ(factory()->GetCurrentEndpoint(), BuildEndpoints()[1]);
 }
 
 // A connection success after a failure should reset backoff.
diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc
index 325226f..ad02a4d 100644
--- a/google_apis/gcm/gcm_client_impl.cc
+++ b/google_apis/gcm/gcm_client_impl.cc
@@ -67,13 +67,7 @@ enum MessageType {
   SEND_ERROR,        // Error sending a message.
 };
 
-// MCS endpoints. SSL Key pinning is done automatically due to the *.google.com
-// pinning rule.
-// Note: modifying the endpoints will affect the ability to compare the
-// GCM.CurrentEnpoint histogram across versions.
-const char kMCSEndpointMain[] = "https://mtalk.google.com:5228";
-const char kMCSEndpointFallback[] = "https://mtalk.google.com:443";
-
+const char kMCSEndpoint[] = "https://mtalk.google.com:5228";
 const int kMaxRegistrationRetries = 5;
 const char kMessageTypeDataMessage[] = "gcm";
 const char kMessageTypeDeletedMessagesKey[] = "deleted_messages";
@@ -195,11 +189,8 @@ void GCMClientImpl::InitializeMCSClient(
             GetNetworkSessionParams();
     DCHECK(network_session_params);
     network_session_ = new net::HttpNetworkSession(*network_session_params);
-    std::vector<GURL> endpoints;
-    endpoints.push_back(GURL(kMCSEndpointMain));
-    endpoints.push_back(GURL(kMCSEndpointFallback));
     connection_factory_.reset(new ConnectionFactoryImpl(
-        endpoints,
+        GURL(kMCSEndpoint),
         kDefaultBackoffPolicy,
         network_session_,
         net_log_.net_log()));