[GCM] Add support for canary connection attempts

[GCM] Add support for canary connection attempts

Canary connection attempts occur on network changes, and are capable of
bypassing backoff (without resetting it). As such they happen immediately
if no connection attempt is already in progress, but if they fail backoff
will be increased accordingly.

BUG=355126
R=asvitkine@chromium.org, jianli@chromium.org

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

[Nicolas Zea, 2014-03-26 22:07:58]

PTAL

[jianli, 2014-03-26 22:27:43]

lgtm

[Alexei Svitkine (slow), 2014-03-27 21:00:51]

histograms lgtm

[Nicolas Zea, 2014-03-28 02:31:37]

Jian Li/Filip, please take another look, I've refactored due to an issue I
noticed, and updated tests to provide better coverage. connecting_ and
waiting_for_backoff_ are now independent of each other, and the tests now cover
the connection handler state.

[jianli, 2014-03-28 04:26:12]

https://codereview.chromium.org/213693002/diff/60001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/213693002/diff/60001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:97: if (connecting_ ||
waiting_for_backoff_)
Now it becomes a bit harder to follow all the logic with different flags plus
checking IsEndpointReachable. Can we manage all these through some kind of enum
states, like: NOT_CONNECTED, CONNECTING, CONNECTED, BACKING_OFF.

[Nicolas Zea, 2014-03-28 17:53:42]

https://codereview.chromium.org/213693002/diff/60001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/213693002/diff/60001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:97: if (connecting_ ||
waiting_for_backoff_)
On 2014/03/28 04:26:13, jianli wrote:
> Now it becomes a bit harder to follow all the logic with different flags plus
> checking IsEndpointReachable. Can we manage all these through some kind of
enum
> states, like: NOT_CONNECTED, CONNECTING, CONNECTED, BACKING_OFF.

The problem is that, because of canaries, these two states are independent of
each other. The factory can be connecting while a backoff expiration is still
pending. And similarly the canary can succeed, making IsEndpointReachable true,
but the backoff is still pending (and its request will be silently dropped when
it finishes). So trying to combine them into a single variable seems more
error-prone to me.

[jianli, 2014-03-28 19:19:32]

lgtm

[Nicolas Zea, 2014-03-28 21:07:16]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:14:15]

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

[commit-bot: I haz the power, 2014-03-28 23:45:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 23:45:29]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Nicolas Zea, 2014-03-28 23:56:19]

Committed patchset #3 manually as r260308 (presubmit successful).