Skip to content

Commit

Permalink
[GCM] Ignore network status when reconnecting again
Browse files Browse the repository at this point in the history
It appears the network change notifications may not be enough to suppress
connection attemtps. Clients are getting stuck waiting for a notification,
despite being online. Ignore the network status for now until a better solution
can be created (this brings things back to their old behavior).

BUG=396687

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286402 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
zea@chromium.org committed Jul 30, 2014
1 parent 606bb3b commit 0a0c8ee
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
9 changes: 5 additions & 4 deletions google_apis/gcm/engine/connection_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ void ConnectionFactoryImpl::SignalConnectionReset(
CloseSocket();
DCHECK(!IsEndpointReachable());

if (waiting_for_network_online_)
return;
// TODO(zea): if the network is offline, don't attempt to connect.
// See crbug.com/396687

// Network changes get special treatment as they can trigger a one-off canary
// request that bypasses backoff (but does nothing if a connection is in
Expand Down Expand Up @@ -245,6 +245,7 @@ void ConnectionFactoryImpl::OnNetworkChanged(
waiting_for_network_online_ = true;

// Will do nothing due to |waiting_for_network_online_ == true|.
// TODO(zea): make the above statement actually true. See crbug.com/396687
SignalConnectionReset(NETWORK_CHANGE);
return;
}
Expand Down Expand Up @@ -278,8 +279,8 @@ void ConnectionFactoryImpl::ConnectImpl() {
DCHECK(!IsEndpointReachable());
DCHECK(!socket_handle_.socket());

if (waiting_for_network_online_)
return;
// TODO(zea): if the network is offline, don't attempt to connect.
// See crbug.com/396687

connecting_ = true;
GURL current_endpoint = GetCurrentEndpoint();
Expand Down
3 changes: 2 additions & 1 deletion google_apis/gcm/engine/connection_factory_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,8 @@ TEST_F(ConnectionFactoryImplTest, SignalResetRestoresBackoff) {

// When the network is disconnected, close the socket and suppress further
// connection attempts until the network returns.
TEST_F(ConnectionFactoryImplTest, SuppressConnectWhenNoNetwork) {
// Disabled while crbug.com/396687 is being investigated.
TEST_F(ConnectionFactoryImplTest, DISABLED_SuppressConnectWhenNoNetwork) {
factory()->SetConnectResult(net::OK);
factory()->Connect();
EXPECT_TRUE(factory()->NextRetryAttempt().is_null());
Expand Down

0 comments on commit 0a0c8ee

Please sign in to comment.