[GCM] Suppress connection attempts when there is no valid connection.

[GCM] Suppress connection attempts when there is no valid connection.

If the local machine's network is unavailable, there's no poin in attempting to
make connections. This change simplifies the connection listening logic and
drops connection attempts while it is known that the network is unavailable.

BUG=376556
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278662

[Nicolas Zea, 2014-06-19 18:48:07]

PTAL. I also suspect changing to the NetworkChangeObserver, rather than ip
address change, will fix Dimitry's problem with hitting backoff on network
resume (we were likely attempting to connect too early).

[fgorski, 2014-06-19 20:01:50]

lgtm

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:153: bool
waiting_for_network_change_;
nit: I understand that name comes from OnNetworkChanged, but perhaps following
the naming withing the notifier is_offline_ should be considered?

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl_unittest.cc (right):

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:418:
factory()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_WIFI);
nit: can we try other stuff than wifi here? CONNECTION_4G or ethernet? Just to
get more coverage of that enum.

[Nicolas Zea, 2014-06-19 21:50:04]

Comments addressed, committing.

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:153: bool
waiting_for_network_change_;
On 2014/06/19 20:01:49, fgorski wrote:
> nit: I understand that name comes from OnNetworkChanged, but perhaps following
> the naming withing the notifier is_offline_ should be considered?

Well, we're waiting for the network to come back online/reconnect, and I'd like
the variable to stay true while we're waiting (rather than having to check
!is_offline_). Changed to waiting_for_network_online_.

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl_unittest.cc (right):

https://codereview.chromium.org/336473003/diff/20001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:418:
factory()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_WIFI);
On 2014/06/19 20:01:49, fgorski wrote:
> nit: can we try other stuff than wifi here? CONNECTION_4G or ethernet? Just to
> get more coverage of that enum.

Well, the code doesn't care what type of connection it is at the moment. But
went ahead and switched this to ethernet and another to 4g.

[Nicolas Zea, 2014-06-19 21:50:06]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-06-19 21:54:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zea@chromium.org/336473003/40001

[commit-bot: I haz the power, 2014-06-20 10:20:04]

Change committed as 278662