[GCM] Minimum change for periodic checkin

[GCM] Minimum change for periodic checkin

* enforces checkin every couple of days.
* persists/load the last checkin time from gcm store.

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

[fgorski, 2014-04-02 21:44:00]

In the interest of minimizing merge traction based on recommendation from both
of you.

Please check it out.

Rest of periodic checkin will be a kick start of G-services work
https://codereview.chromium.org/215363007/

[Nicolas Zea, 2014-04-02 21:53:54]

LGTM

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
google_apis/gcm/engine/gcm_store_impl.cc:472: 
nit: remove extra newline

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
File google_apis/gcm/gcm_client_impl.cc (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:77: const int64 kDefaultCheckinInterval =
172800LL;  // seconds - 2 days.
nit: prefer 60 * 60 * 24 * 2 for readability

[jianli, 2014-04-02 22:00:18]

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
File google_apis/gcm/engine/gcm_store_impl.h (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
google_apis/gcm/engine/gcm_store_impl.h:11: #include "base/time/time.h"
nit: not needed

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
File google_apis/gcm/gcm_client_impl.cc (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:222: } else {
nit: revert these two parts

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:320: ResetState();
When the updated checkin info is different, we call ResetState which seems not
to be implemented.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:337: +
base::TimeDelta::FromSeconds(kDefaultCheckinInterval)
nit: + and - should be placed at the end of previous line.

[fgorski, 2014-04-02 22:37:20]

PTAL

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
File google_apis/gcm/engine/gcm_store_impl.cc (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
google_apis/gcm/engine/gcm_store_impl.cc:472: 
On 2014/04/02 21:53:55, Nicolas Zea wrote:
> nit: remove extra newline

Done.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
File google_apis/gcm/engine/gcm_store_impl.h (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/engine/gcm_s...
google_apis/gcm/engine/gcm_store_impl.h:11: #include "base/time/time.h"
On 2014/04/02 22:00:18, jianli wrote:
> nit: not needed

Done.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
File google_apis/gcm/gcm_client_impl.cc (right):

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:77: const int64 kDefaultCheckinInterval =
172800LL;  // seconds - 2 days.
On 2014/04/02 21:53:55, Nicolas Zea wrote:
> nit: prefer 60 * 60 * 24 * 2 for readability

Done.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:222: } else {
On 2014/04/02 22:00:18, jianli wrote:
> nit: revert these two parts

Done.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:320: ResetState();
On 2014/04/02 22:00:18, jianli wrote:
> When the updated checkin info is different, we call ResetState which seems not
> to be implemented.

Done.

https://codereview.chromium.org/222913004/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl.cc:337: +
base::TimeDelta::FromSeconds(kDefaultCheckinInterval)
On 2014/04/02 22:00:18, jianli wrote:
> nit: + and - should be placed at the end of previous line.

Done.

[jianli, 2014-04-02 23:02:51]

lgtm

https://codereview.chromium.org/222913004/diff/20001/google_apis/gcm/gcm_clie...
File google_apis/gcm/gcm_client_impl.cc (right):

https://codereview.chromium.org/222913004/diff/20001/google_apis/gcm/gcm_clie...
google_apis/gcm/gcm_client_impl.cc:318:
DCHECK_EQ(device_checkin_info_.android_id, checkin_info.android_id);
Please comment on the fact that the checkin info is not expected to be updated.

[fgorski, 2014-04-02 23:06:21]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-04-02 23:09:45]

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

[commit-bot: I haz the power, 2014-04-03 08:27:24]

Change committed as 261324