diff options
author | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-24 19:48:24 +0000 |
---|---|---|
committer | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-24 19:48:24 +0000 |
commit | a4ba958842e3062a878156ae946cabb04663de4c (patch) | |
tree | aabcf53f0f16aa22aa8c208d58fcfe04bc58340c /google_apis | |
parent | 087dcc0522811a00ae927f2083162b3a9db4ccec (diff) | |
download | chromium_src-a4ba958842e3062a878156ae946cabb04663de4c.zip chromium_src-a4ba958842e3062a878156ae946cabb04663de4c.tar.gz chromium_src-a4ba958842e3062a878156ae946cabb04663de4c.tar.bz2 |
[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.
Relanding due to compile issue.
Original review: https://codereview.chromium.org/205343003/
BUG=351890
TBR=jianli@chromium.org, fgorski@chromium.org, asvitkine@chromium.org
Review URL: https://codereview.chromium.org/206873006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258997 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'google_apis')
-rw-r--r-- | google_apis/gcm/engine/connection_factory_impl.cc | 36 | ||||
-rw-r--r-- | google_apis/gcm/engine/connection_factory_impl.h | 15 | ||||
-rw-r--r-- | google_apis/gcm/engine/connection_factory_impl_unittest.cc | 15 | ||||
-rw-r--r-- | google_apis/gcm/gcm_client_impl.cc | 13 | ||||
-rw-r--r-- | google_apis/gcm/tools/mcs_probe.cc | 7 |
5 files changed, 72 insertions, 14 deletions
diff --git a/google_apis/gcm/engine/connection_factory_impl.cc b/google_apis/gcm/engine/connection_factory_impl.cc index ebfd367..deb8102 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,20 @@ 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( - mcs_endpoint_, + GetCurrentEndpoint(), &proxy_info_, base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone, weak_ptr_factory_.GetWeakPtr()), @@ -234,13 +245,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_++; + 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."; @@ -300,7 +326,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 +400,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_, diff --git a/google_apis/gcm/engine/connection_factory_impl.h b/google_apis/gcm/engine/connection_factory_impl.h index 967c6c1..28c2941 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 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); @@ -54,6 +54,11 @@ 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 @@ -89,8 +94,12 @@ class GCM_EXPORT ConnectionFactoryImpl : void CloseSocket(); - // The MCS endpoint to make connections to. - const GURL mcs_endpoint_; + // 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 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 82c3f18..eebe14a 100644 --- a/google_apis/gcm/engine/connection_factory_impl_unittest.cc +++ b/google_apis/gcm/engine/connection_factory_impl_unittest.cc @@ -19,6 +19,7 @@ namespace gcm { namespace { const char kMCSEndpoint[] = "http://my.server"; +const char kMCSEndpoint2[] = "http://my.alt.server"; const int kBackoffDelayMs = 1; const int kBackoffMultiplier = 2; @@ -50,6 +51,13 @@ 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) { @@ -132,7 +140,7 @@ class TestConnectionFactoryImpl : public ConnectionFactoryImpl { TestConnectionFactoryImpl::TestConnectionFactoryImpl( const base::Closure& finished_callback) - : ConnectionFactoryImpl(GURL(kMCSEndpoint), + : ConnectionFactoryImpl(BuildEndpoints(), net::BackoffEntry::Policy(), NULL, NULL), @@ -254,9 +262,11 @@ 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. +// A connection failure should result in backoff, and attempting the fallback +// endpoint next. TEST_F(ConnectionFactoryImplTest, ConnectFail) { factory()->Initialize( ConnectionFactory::BuildLoginRequestCallback(), @@ -265,6 +275,7 @@ 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 ad02a4d..325226f 100644 --- a/google_apis/gcm/gcm_client_impl.cc +++ b/google_apis/gcm/gcm_client_impl.cc @@ -67,7 +67,13 @@ enum MessageType { SEND_ERROR, // Error sending a message. }; -const char kMCSEndpoint[] = "https://mtalk.google.com:5228"; +// 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 int kMaxRegistrationRetries = 5; const char kMessageTypeDataMessage[] = "gcm"; const char kMessageTypeDeletedMessagesKey[] = "deleted_messages"; @@ -189,8 +195,11 @@ 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( - GURL(kMCSEndpoint), + endpoints, kDefaultBackoffPolicy, network_session_, net_log_.net_log())); diff --git a/google_apis/gcm/tools/mcs_probe.cc b/google_apis/gcm/tools/mcs_probe.cc index 48815df..157a73f 100644 --- a/google_apis/gcm/tools/mcs_probe.cc +++ b/google_apis/gcm/tools/mcs_probe.cc @@ -290,9 +290,12 @@ void MCSProbe::Start() { file_thread_.Start(); InitializeNetworkState(); BuildNetworkSession(); + std::vector<GURL> endpoints(1, + GURL("https://" + + net::HostPortPair(server_host_, + server_port_).ToString())); connection_factory_.reset( - new ConnectionFactoryImpl(GURL("https://" + net::HostPortPair( - server_host_, server_port_).ToString()), + new ConnectionFactoryImpl(endpoints, kDefaultBackoffPolicy, network_session_, &net_log_)); |