[GCM] Updating GCM Status Syncer to the new Experiment Status pb

[GCM] Updating GCM Status Syncer to the new Experiment Status pb

BUG=421210
Committed: https://crrev.com/3689f8d171c79fb6695eae6c65b6fd517f5a09c0
Cr-Commit-Position: refs/heads/master@{#298771}

[fgorski, 2014-10-08 00:21:57]

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

[fgorski, 2014-10-08 00:21:57]

PTAL

This is the suggested protobuf update.

[Nicolas Zea, 2014-10-08 01:02:20]

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

https://codereview.chromium.org/635093002/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.cc:106: for (int i = 0; i <
response_proto.experiment_size(); ++i) {
we're only ever going to get back the experiments we request right? Do we need
to iterate in that case? We can update this code if/when we support more
experiments.

Checking has_gcm_channel seems fine though as a safety measure.

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

https://codereview.chromium.org/635093002/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request_unittest.cc:72: GCMStatus
status, int poll_interval_seconds) {
should we also be setting something based on poll_interval_seconds?

[fgorski, 2014-10-08 16:43:43]

Comments addressed.

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

https://codereview.chromium.org/635093002/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request.cc:106: for (int i = 0; i <
response_proto.experiment_size(); ++i) {
On 2014/10/08 01:02:20, Nicolas Zea wrote:
> we're only ever going to get back the experiments we request right? Do we need
> to iterate in that case? We can update this code if/when we support more
> experiments.
> 
> Checking has_gcm_channel seems fine though as a safety measure.

Done.

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

https://codereview.chromium.org/635093002/diff/1/components/gcm_driver/gcm_ch...
components/gcm_driver/gcm_channel_status_request_unittest.cc:72: GCMStatus
status, int poll_interval_seconds) {
On 2014/10/08 01:02:20, Nicolas Zea wrote:
> should we also be setting something based on poll_interval_seconds?

It's done in the code below. I just double checked to confirm:
poll_interval_seconds is at the same level in response as before the change, so
I don't need to be making an update there.

[Nicolas Zea, 2014-10-08 16:52:15]

lgtm

[fgorski, 2014-10-08 18:25:15]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-10-08 18:26:18]

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

[commit-bot: I haz the power, 2014-10-09 04:39:39]

Committed patchset #2 (id:20001) as 536e697a762587ced76d942b3278a4e0de5862ec

[commit-bot: I haz the power, 2014-10-09 04:40:40]

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