[Android] BackgroundOfflinerTask using wall clock for timings UMA |
||
Issue descriptionIn TaskExtrasPacker#packTimeInBundle uses SystemClock.currentTimeMillis() to store the time the task was scheduled, which is then used by BackgroundOfflinerTask to calculate how long before the device was online again, but we probably don't want to use wall clock for interval timings - see https://developer.android.com/reference/android/os/SystemClock.html SystemClock.elapsedRealTime() is probably a better choice here.
,
Mar 23 2017
(Unless the intention is for timings to span device reboots, which SystemClock.elapsedRealTime() is not suitable for, but right now it's recording the UMA with a max value of 1 hour so this doesn't seem to be the intention).
,
Mar 23 2017
Thanks for the problem report! We do want our tasks to persist across a reboot, which is why I originally chose to use currentTimeMillis. Normally, the times will be less than an hour, but it could well go over an hour, perhaps we should update our UMA. However, we should weigh working across reboots against potential problems caused by daylight savings time changes. If the time moves forwards by an hour, it could cause a task to fire sooner than anticipated, this does not present a huge problem for us given that it happens only once a year. If time moves backwards by an hour, our task could become due immediately. Given that we normally want a 5 minute wait, the task could fire sooner than 5 minutes, but the consequences of the task firing sooner are fairly minimal, we don't need a strong guarantee. So, weighing losing a request on a reboot against once a year the task might fire up to 5 minutes too soon or an hour too late, and that would be in the middle of the night, I think we should prefer not losing requests. So, I'm resolving the issue as "Works as intended", feel free to reopen if I have missed anything or you disagree with my logic here.
,
Mar 24 2017
Cool, so long as you're aware you might get some weird spikes this weekend when the clocks change in Europe! |
||
►
Sign in to add a comment |
||
Comment 1 by awdf@chromium.org
, Mar 22 2017