Added code to make sure that the budget service does sane things if the clock rolls backwards.

Added code to make sure that the budget service does sane things if the clock rolls backwards.

BUG=602244
Committed: https://crrev.com/9b0ad479a27c55bb0c215b73d00633ddb5083360
Cr-Commit-Position: refs/heads/master@{#397363}

[harkness, 2016-05-31 13:43:10]

harkness@chromium.org changed reviewers:
+ johnme@chromium.org

[harkness, 2016-06-01 09:45:26]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-01 09:45:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020263002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2020263002/1

[commit-bot: I haz the power, 2016-06-01 10:36:01]

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

[commit-bot: I haz the power, 2016-06-01 10:36:02]

Dry run: This issue passed the CQ dry run.

[johnme, 2016-06-01 18:24:38]

lgtm with nits

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/background_budget_service.cc (right):

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service.cc:135: if
(elapsed.InMicroseconds() < 0)
Conversely, the clock may jump forwards a lot, e.g. if the clock periodically
resets to 1970 then is fixed by ntp, or if the user jumps back and forwards
excessively to cheat at a game.

It would be unfortunate to remove all budget whenever that happens (which may be
daily for some users). If time jumps forward more than ~1 week (1 month?)
without Chrome getting run at all, it's likely that time didn't really advance
that much. Ideally Chrome would centrally detect such jumps in time, though I
haven't yet encountered code for that.

Add a TODO for this?

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/background_budget_service_unittest.cc
(right):

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service_unittest.cc:229: double
budget = 0.0;
Nit: declare this variable as `double budget = service->GetBudget(origin);`
below, rather than giving it a default value that can never be used.

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service_unittest.cc:233: budget
= service->GetBudget(origin);
Nit: add an assert that this budget is different from kTestBudget (since the
comment below says we want to test it matches the budget of the most foward
time)

[harkness, 2016-06-02 10:06:34]

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/background_budget_service.cc (right):

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service.cc:135: if
(elapsed.InMicroseconds() < 0)
On 2016/06/01 18:24:38, johnme wrote:
> Conversely, the clock may jump forwards a lot, e.g. if the clock periodically
> resets to 1970 then is fixed by ntp, or if the user jumps back and forwards
> excessively to cheat at a game.
> 
> It would be unfortunate to remove all budget whenever that happens (which may
be
> daily for some users). If time jumps forward more than ~1 week (1 month?)
> without Chrome getting run at all, it's likely that time didn't really advance
> that much. Ideally Chrome would centrally detect such jumps in time, though I
> haven't yet encountered code for that.
> 
> Add a TODO for this?

Done.

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/background_budget_service_unittest.cc
(right):

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service_unittest.cc:229: double
budget = 0.0;
On 2016/06/01 18:24:38, johnme wrote:
> Nit: declare this variable as `double budget = service->GetBudget(origin);`
> below, rather than giving it a default value that can never be used.

Done.

https://codereview.chromium.org/2020263002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/background_budget_service_unittest.cc:233: budget
= service->GetBudget(origin);
On 2016/06/01 18:24:38, johnme wrote:
> Nit: add an assert that this budget is different from kTestBudget (since the
> comment below says we want to test it matches the budget of the most foward
> time)

Done.

[harkness, 2016-06-02 10:06:44]

The CQ bit was checked by harkness@chromium.org

[harkness, 2016-06-02 10:06:44]

The patchset sent to the CQ was uploaded after l-g-t-m from johnme@chromium.org
Link to the patchset: https://codereview.chromium.org/2020263002/#ps20001
(title: "Code review comments integrated")

[commit-bot: I haz the power, 2016-06-02 10:06:59]

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

[commit-bot: I haz the power, 2016-06-02 11:16:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-02 11:19:13]

Description was changed from

==========
Added code to make sure that the budget service does sane things if the clock
rolls backwards.

BUG=602244
==========

to

==========
Added code to make sure that the budget service does sane things if the clock
rolls backwards.

BUG=602244

Committed: https://crrev.com/9b0ad479a27c55bb0c215b73d00633ddb5083360
Cr-Commit-Position: refs/heads/master@{#397363}
==========

[commit-bot: I haz the power, 2016-06-02 11:19:14]

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