[GCM] Start GCMChannelStatusSyncer when GCM is disabled

[GCM] Start GCMChannelStatusSyncer when GCM is disabled

We need to start GCMChannelStatusSyncer when GCM is disabled in order
to poll the server to find out when it is reenabled.

Also if server returns empty response, we should not treat it as error
and trigger the backoff logic.

BUG=423415
TEST=new tests added

Committed: https://crrev.com/9e9dd3b7798b3500d70941af04b1325ef9a0b544
Cr-Commit-Position: refs/heads/master@{#299518}

R=fgorski@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/74eadb07fbefe7b883b8d8056dd3e029ff45b088

[jianli, 2014-10-14 17:26:02]

jianli@chromium.org changed reviewers:
+ fgorski@chromium.org

[fgorski, 2014-10-14 17:40:37]

lgtm with nits

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
File components/gcm_driver/gcm_channel_status_request.cc (right):

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.cc:109: LOG(ERROR) << "GCM
channel response failed to be parse as proto.";
nit: parse -> parsed

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
File components/gcm_driver/gcm_channel_status_request.h (right):

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.h:31: // |enabled|: indicates
if GCM should be enabled or not.
nit: or disabled.

Why? One could read that statement as: Should I enable GCM? yes/no, as opposed
to: Is GCM allowed (enabled) to run? yes/no

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.h:32: //
|poll_interval_seconds|: the interval in seconds to start next polling
nit: next poll request

[jianli, 2014-10-14 17:45:36]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2014-10-14 17:46:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/653843003/20001

[commit-bot: I haz the power, 2014-10-14 18:57:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-14 18:59:14]

Patchset 2 (id:??) landed as
https://crrev.com/9e9dd3b7798b3500d70941af04b1325ef9a0b544
Cr-Commit-Position: refs/heads/master@{#299518}

[jianli, 2014-10-14 23:13:10]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/657703002/ by jianli@chromium.org.

The reason for reverting is: Reverted due to that the fix is not complete. Will
fix one more issue we find and reland the patch..

[jianli, 2014-10-14 23:21:47]

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
File components/gcm_driver/gcm_channel_status_request.cc (right):

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.cc:109: LOG(ERROR) << "GCM
channel response failed to be parse as proto.";
On 2014/10/14 17:40:37, fgorski wrote:
> nit: parse -> parsed

Done.

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
File components/gcm_driver/gcm_channel_status_request.h (right):

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.h:31: // |enabled|: indicates
if GCM should be enabled or not.
On 2014/10/14 17:40:37, fgorski wrote:
> nit: or disabled.
> 
> Why? One could read that statement as: Should I enable GCM? yes/no, as opposed
> to: Is GCM allowed (enabled) to run? yes/no

Updated the comment.

https://codereview.chromium.org/653843003/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.h:32: //
|poll_interval_seconds|: the interval in seconds to start next polling
On 2014/10/14 17:40:37, fgorski wrote:
> nit: next poll request

Done.

[fgorski, 2014-10-15 17:01:52]

One comment and awaiting response from James to my email.

https://codereview.chromium.org/653843003/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_channel_status_syncer.cc (right):

https://codereview.chromium.org/653843003/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_channel_status_syncer.cc:105: poll_interval_seconds_ =
minutes * 60;
we should make sure it is >0, right?

[fgorski, 2014-10-15 21:17:20]

lgtm

https://codereview.chromium.org/653843003/diff/60001/components/gcm_driver/gc...
File components/gcm_driver/gcm_channel_status_syncer.cc (right):

https://codereview.chromium.org/653843003/diff/60001/components/gcm_driver/gc...
components/gcm_driver/gcm_channel_status_syncer.cc:119: }
nit: DVLOG here that it was not set and we are ignoring the custom poll
interval. One more way to remind the testers that the min interval is 5 minutes.

[fgorski, 2014-10-15 22:44:16]

still lgtm

[jianli, 2014-10-15 22:46:46]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2014-10-15 22:50:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/653843003/100001

[commit-bot: I haz the power, 2014-10-16 02:01:20]

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

[commit-bot: I haz the power, 2014-10-16 02:01:21]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jianli, 2014-10-16 05:42:00]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2014-10-16 05:42:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/653843003/100001

[commit-bot: I haz the power, 2014-10-16 05:48:13]

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

[commit-bot: I haz the power, 2014-10-16 05:48:14]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jianli, 2014-10-16 05:53:52]

The CQ bit was unchecked by jianli@chromium.org

[commit-bot: I haz the power, 2014-10-16 05:55:23]

Patchset 6 (id:??) landed as
https://crrev.com/74eadb07fbefe7b883b8d8056dd3e029ff45b088
Cr-Commit-Position: refs/heads/master@{#299854}

[jianli, 2014-10-16 05:55:27]

Committed patchset #6 (id:100001) manually as
74eadb07fbefe7b883b8d8056dd3e029ff45b088 (presubmit successful).