Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(206)

Side by Side Diff: google_apis/gcm/engine/connection_factory_impl.cc

Issue 2624653003: Provide correct guard for GCM connection tracking of login failures. (Closed)
Patch Set: nits Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « google_apis/gcm/engine/connection_factory_impl.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "google_apis/gcm/engine/connection_factory_impl.h" 5 #include "google_apis/gcm/engine/connection_factory_impl.h"
6 6
7 #include <string> 7 #include <string>
8 8
9 #include "base/location.h" 9 #include "base/location.h"
10 #include "base/memory/ptr_util.h" 10 #include "base/memory/ptr_util.h"
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
59 last_successful_endpoint_(0), 59 last_successful_endpoint_(0),
60 backoff_policy_(backoff_policy), 60 backoff_policy_(backoff_policy),
61 gcm_network_session_(gcm_network_session), 61 gcm_network_session_(gcm_network_session),
62 http_network_session_(http_network_session), 62 http_network_session_(http_network_session),
63 net_log_( 63 net_log_(
64 net::NetLogWithSource::Make(net_log, net::NetLogSourceType::SOCKET)), 64 net::NetLogWithSource::Make(net_log, net::NetLogSourceType::SOCKET)),
65 pac_request_(NULL), 65 pac_request_(NULL),
66 connecting_(false), 66 connecting_(false),
67 waiting_for_backoff_(false), 67 waiting_for_backoff_(false),
68 waiting_for_network_online_(false), 68 waiting_for_network_online_(false),
69 logging_in_(false), 69 handshake_in_progress_(false),
70 recorder_(recorder), 70 recorder_(recorder),
71 listener_(NULL), 71 listener_(NULL),
72 weak_ptr_factory_(this) { 72 weak_ptr_factory_(this) {
73 DCHECK_GE(mcs_endpoints_.size(), 1U); 73 DCHECK_GE(mcs_endpoints_.size(), 1U);
74 DCHECK(!http_network_session_ || 74 DCHECK(!http_network_session_ ||
75 (gcm_network_session_ != http_network_session_)); 75 (gcm_network_session_ != http_network_session_));
76 } 76 }
77 77
78 ConnectionFactoryImpl::~ConnectionFactoryImpl() { 78 ConnectionFactoryImpl::~ConnectionFactoryImpl() {
79 CloseSocket(); 79 CloseSocket();
(...skipping 44 matching lines...) Expand 10 before | Expand all | Expand 10 after
124 ConnectWithBackoff(); 124 ConnectWithBackoff();
125 } 125 }
126 126
127 ConnectionEventTracker* ConnectionFactoryImpl::GetEventTrackerForTesting() { 127 ConnectionEventTracker* ConnectionFactoryImpl::GetEventTrackerForTesting() {
128 return &event_tracker_; 128 return &event_tracker_;
129 } 129 }
130 130
131 void ConnectionFactoryImpl::ConnectWithBackoff() { 131 void ConnectionFactoryImpl::ConnectWithBackoff() {
132 // If a canary managed to connect while a backoff expiration was pending, 132 // If a canary managed to connect while a backoff expiration was pending,
133 // just cleanup the internal state. 133 // just cleanup the internal state.
134 if (connecting_ || logging_in_ || IsEndpointReachable()) { 134 if (connecting_ || handshake_in_progress_ || IsEndpointReachable()) {
135 waiting_for_backoff_ = false; 135 waiting_for_backoff_ = false;
136 return; 136 return;
137 } 137 }
138 138
139 if (backoff_entry_->ShouldRejectRequest()) { 139 if (backoff_entry_->ShouldRejectRequest()) {
140 DVLOG(1) << "Delaying MCS endpoint connection for " 140 DVLOG(1) << "Delaying MCS endpoint connection for "
141 << backoff_entry_->GetTimeUntilRelease().InMilliseconds() 141 << backoff_entry_->GetTimeUntilRelease().InMilliseconds()
142 << " milliseconds."; 142 << " milliseconds.";
143 waiting_for_backoff_ = true; 143 waiting_for_backoff_ = true;
144 recorder_->RecordConnectionDelayedDueToBackoff( 144 recorder_->RecordConnectionDelayedDueToBackoff(
(...skipping 15 matching lines...) Expand all
160 ConnectImpl(); 160 ConnectImpl();
161 } 161 }
162 162
163 bool ConnectionFactoryImpl::IsEndpointReachable() const { 163 bool ConnectionFactoryImpl::IsEndpointReachable() const {
164 return connection_handler_ && connection_handler_->CanSendMessage(); 164 return connection_handler_ && connection_handler_->CanSendMessage();
165 } 165 }
166 166
167 std::string ConnectionFactoryImpl::GetConnectionStateString() const { 167 std::string ConnectionFactoryImpl::GetConnectionStateString() const {
168 if (IsEndpointReachable()) 168 if (IsEndpointReachable())
169 return "CONNECTED"; 169 return "CONNECTED";
170 if (logging_in_) 170 if (handshake_in_progress_)
171 return "LOGGING IN"; 171 return "HANDSHAKE IN PROGRESS";
172 if (connecting_) 172 if (connecting_)
173 return "CONNECTING"; 173 return "CONNECTING";
174 if (waiting_for_backoff_) 174 if (waiting_for_backoff_)
175 return "WAITING FOR BACKOFF"; 175 return "WAITING FOR BACKOFF";
176 if (waiting_for_network_online_) 176 if (waiting_for_network_online_)
177 return "WAITING FOR NETWORK CHANGE"; 177 return "WAITING FOR NETWORK CHANGE";
178 return "NOT CONNECTED"; 178 return "NOT CONNECTED";
179 } 179 }
180 180
181 void ConnectionFactoryImpl::SignalConnectionReset( 181 void ConnectionFactoryImpl::SignalConnectionReset(
(...skipping 20 matching lines...) Expand all
202 if (!last_login_time_.is_null()) { 202 if (!last_login_time_.is_null()) {
203 UMA_HISTOGRAM_CUSTOM_TIMES("GCM.ConnectionUpTime", 203 UMA_HISTOGRAM_CUSTOM_TIMES("GCM.ConnectionUpTime",
204 NowTicks() - last_login_time_, 204 NowTicks() - last_login_time_,
205 base::TimeDelta::FromSeconds(1), 205 base::TimeDelta::FromSeconds(1),
206 base::TimeDelta::FromHours(24), 206 base::TimeDelta::FromHours(24),
207 50); 207 50);
208 // |last_login_time_| will be reset below, before attempting the new 208 // |last_login_time_| will be reset below, before attempting the new
209 // connection. 209 // connection.
210 } 210 }
211 211
212 if (logging_in_) 212 if (reason == LOGIN_FAILURE)
213 event_tracker_.ConnectionLoginFailed(); 213 event_tracker_.ConnectionLoginFailed();
214 event_tracker_.EndConnectionAttempt(); 214 event_tracker_.EndConnectionAttempt();
215 215
216 CloseSocket(); 216 CloseSocket();
217 DCHECK(!IsEndpointReachable()); 217 DCHECK(!IsEndpointReachable());
218 218
219 // TODO(zea): if the network is offline, don't attempt to connect. 219 // TODO(zea): if the network is offline, don't attempt to connect.
220 // See crbug.com/396687 220 // See crbug.com/396687
221 221
222 // Network changes get special treatment as they can trigger a one-off canary 222 // Network changes get special treatment as they can trigger a one-off canary
223 // request that bypasses backoff (but does nothing if a connection is in 223 // request that bypasses backoff (but does nothing if a connection is in
224 // progress). Other connection reset events can be ignored as a connection 224 // progress). Other connection reset events can be ignored as a connection
225 // is already awaiting backoff expiration. 225 // is already awaiting backoff expiration.
226 if (waiting_for_backoff_ && reason != NETWORK_CHANGE) { 226 if (waiting_for_backoff_ && reason != NETWORK_CHANGE) {
227 DVLOG(1) << "Backoff expiration pending, ignoring reset."; 227 DVLOG(1) << "Backoff expiration pending, ignoring reset.";
228 return; 228 return;
229 } 229 }
230 230
231 if (reason == NETWORK_CHANGE) { 231 if (reason == NETWORK_CHANGE) {
232 // Canary attempts bypass backoff without resetting it. These will have no 232 // Canary attempts bypass backoff without resetting it. These will have no
233 // effect if we're already in the process of connecting. 233 // effect if we're already in the process of connecting.
234 ConnectImpl(); 234 ConnectImpl();
235 return; 235 return;
236 } else if (logging_in_) { 236 } else if (handshake_in_progress_) {
237 // Failures prior to login completion just reuse the existing backoff entry. 237 // Failures prior to handshake completion reuse the existing backoff entry.
238 logging_in_ = false; 238 handshake_in_progress_ = false;
239 backoff_entry_->InformOfRequest(false); 239 backoff_entry_->InformOfRequest(false);
240 } else if (reason == LOGIN_FAILURE || 240 } else if (reason == LOGIN_FAILURE ||
241 ShouldRestorePreviousBackoff(last_login_time_, NowTicks())) { 241 ShouldRestorePreviousBackoff(last_login_time_, NowTicks())) {
242 // Failures due to login, or within the reset window of a login, restore 242 // Failures due to login, or within the reset window of a login, restore
243 // the backoff entry that was saved off at login completion time. 243 // the backoff entry that was saved off at login completion time.
244 backoff_entry_.swap(previous_backoff_); 244 backoff_entry_.swap(previous_backoff_);
245 backoff_entry_->InformOfRequest(false); 245 backoff_entry_->InformOfRequest(false);
246 } else { 246 } else {
247 // We shouldn't be in backoff in thise case. 247 // We shouldn't be in backoff in thise case.
248 DCHECK_EQ(0, backoff_entry_->failure_count()); 248 DCHECK_EQ(0, backoff_entry_->failure_count());
(...skipping 156 matching lines...) Expand 10 before | Expand all | Expand 10 after
405 ReportSuccessfulProxyConnection(); 405 ReportSuccessfulProxyConnection();
406 recorder_->RecordConnectionSuccess(); 406 recorder_->RecordConnectionSuccess();
407 407
408 // Reset the endpoint back to the default. 408 // Reset the endpoint back to the default.
409 // TODO(zea): consider prioritizing endpoints more intelligently based on 409 // TODO(zea): consider prioritizing endpoints more intelligently based on
410 // which ones succeed most for this client? Although that will affect 410 // which ones succeed most for this client? Although that will affect
411 // measuring the success rate of the default endpoint vs fallback. 411 // measuring the success rate of the default endpoint vs fallback.
412 last_successful_endpoint_ = next_endpoint_; 412 last_successful_endpoint_ = next_endpoint_;
413 next_endpoint_ = 0; 413 next_endpoint_ = 0;
414 connecting_ = false; 414 connecting_ = false;
415 logging_in_ = true; 415 handshake_in_progress_ = true;
416 DVLOG(1) << "MCS endpoint socket connection success, starting login."; 416 DVLOG(1) << "MCS endpoint socket connection success, starting login.";
417 InitHandler(); 417 InitHandler();
418 } 418 }
419 419
420 void ConnectionFactoryImpl::ConnectionHandlerCallback(int result) { 420 void ConnectionFactoryImpl::ConnectionHandlerCallback(int result) {
421 DCHECK(!connecting_); 421 DCHECK(!connecting_);
422 if (result != net::OK) { 422 if (result != net::OK) {
423 // TODO(zea): Consider how to handle errors that may require some sort of 423 // TODO(zea): Consider how to handle errors that may require some sort of
424 // user intervention (login page, etc.). 424 // user intervention (login page, etc.).
425 UMA_HISTOGRAM_SPARSE_SLOWLY("GCM.ConnectionDisconnectErrorCode", result); 425 UMA_HISTOGRAM_SPARSE_SLOWLY("GCM.ConnectionDisconnectErrorCode", result);
426 SignalConnectionReset(SOCKET_FAILURE); 426 SignalConnectionReset(SOCKET_FAILURE);
427 return; 427 return;
428 } 428 }
429 429
430 // Handshake complete, reset backoff. If the login failed with an error, 430 // Handshake complete, reset backoff. If the login failed with an error,
431 // the client should invoke SignalConnectionReset(LOGIN_FAILURE), which will 431 // the client should invoke SignalConnectionReset(LOGIN_FAILURE), which will
432 // restore the previous backoff. 432 // restore the previous backoff.
433 DVLOG(1) << "Handshake complete."; 433 DVLOG(1) << "Handshake complete.";
434 last_login_time_ = NowTicks(); 434 last_login_time_ = NowTicks();
435 previous_backoff_.swap(backoff_entry_); 435 previous_backoff_.swap(backoff_entry_);
436 backoff_entry_->Reset(); 436 backoff_entry_->Reset();
437 logging_in_ = false; 437 handshake_in_progress_ = false;
438 438
439 event_tracker_.ConnectionAttemptSucceeded(); 439 event_tracker_.ConnectionAttemptSucceeded();
440 440
441 if (listener_) 441 if (listener_)
442 listener_->OnConnected(GetCurrentEndpoint(), GetPeerIP()); 442 listener_->OnConnected(GetCurrentEndpoint(), GetPeerIP());
443 } 443 }
444 444
445 // This has largely been copied from 445 // This has largely been copied from
446 // HttpStreamFactoryImpl::Job::DoResolveProxyComplete. This should be 446 // HttpStreamFactoryImpl::Job::DoResolveProxyComplete. This should be
447 // refactored into some common place. 447 // refactored into some common place.
(...skipping 144 matching lines...) Expand 10 before | Expand all | Expand 10 after
592 592
593 void ConnectionFactoryImpl::RebuildNetworkSessionAuthCache() { 593 void ConnectionFactoryImpl::RebuildNetworkSessionAuthCache() {
594 if (!http_network_session_ || !http_network_session_->http_auth_cache()) 594 if (!http_network_session_ || !http_network_session_->http_auth_cache())
595 return; 595 return;
596 596
597 gcm_network_session_->http_auth_cache()->UpdateAllFrom( 597 gcm_network_session_->http_auth_cache()->UpdateAllFrom(
598 *http_network_session_->http_auth_cache()); 598 *http_network_session_->http_auth_cache());
599 } 599 }
600 600
601 } // namespace gcm 601 } // namespace gcm
OLDNEW
« no previous file with comments | « google_apis/gcm/engine/connection_factory_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698