Switch OfflinePages UMA to use currentTimeMillis, persist task

Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
Committed: https://crrev.com/4667f21d36db6c8bb92a7f4b2dc4eced06f1bea4
Cr-Commit-Position: refs/heads/master@{#397877}

[Pete Williamson, 2016-06-03 21:57:09]

petewil@chromium.org changed reviewers:
+ dewittj@chromium.org, dougarnett@chromium.org

[Pete Williamson, 2016-06-03 21:57:10]

Splitting out the time and persistence part of this changelist.

[dewittj, 2016-06-03 22:02:47]

Thanks for pulling this out!

lgtm with nits

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java
(left):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java:5:
package org.chromium.chrome.browser.offlinepages;
nit: mention offline pages in CL description

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:208:
public static void recordWakeupUMA(Context context, long millisTaskStart) {
taskScheduledTimeMillis might be a better name

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java:14:
public static final String DATE_TAG = "Date";
maybe use more descriptive names here, such as TASK_SCHEDULED_TIME_TAG etc.

[dougarnett, 2016-06-03 22:45:50]

lgtm with naming comments

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:208:
public static void recordWakeupUMA(Context context, long millisTaskStart) {
On 2016/06/03 22:02:46, dewittj wrote:
> taskScheduledTimeMillis might be a better name

+1

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:230:
long millisNow = System.currentTimeMillis();
nowMillis

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java:14:
public static final String DATE_TAG = "Date";
On 2016/06/03 22:02:46, dewittj wrote:
> maybe use more descriptive names here, such as TASK_SCHEDULED_TIME_TAG etc.

+1 - think I suggested just SCHEDULED_TIME_TAG or REQUESTED_TIME_TAG in other CL
review

[Pete Williamson, 2016-06-03 23:33:36]

Description was changed from

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
==========

to

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
==========

[Pete Williamson, 2016-06-03 23:42:11]

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java
(left):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java:5:
package org.chromium.chrome.browser.offlinepages;
On 2016/06/03 22:02:46, dewittj wrote:
> nit: mention offline pages in CL description

Done.

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:208:
public static void recordWakeupUMA(Context context, long millisTaskStart) {
On 2016/06/03 22:02:46, dewittj wrote:
> taskScheduledTimeMillis might be a better name

Done.

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtils.java:230:
long millisNow = System.currentTimeMillis();
On 2016/06/03 22:45:50, dougarnett wrote:
> nowMillis

Done.

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java
(right):

https://codereview.chromium.org/2037213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPacker.java:14:
public static final String DATE_TAG = "Date";
On 2016/06/03 22:45:50, dougarnett wrote:
> On 2016/06/03 22:02:46, dewittj wrote:
> > maybe use more descriptive names here, such as TASK_SCHEDULED_TIME_TAG etc.
> 
> +1 - think I suggested just SCHEDULED_TIME_TAG or REQUESTED_TIME_TAG in other
CL
> review

Oops, I thought I deleted this file from the change (since it requires things
from the other test changelist).
I re-deleted this file.

[Pete Williamson, 2016-06-03 23:42:17]

The CQ bit was checked by petewil@chromium.org

[Pete Williamson, 2016-06-03 23:42:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dougarnett@chromium.org, dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/2037213002/#ps20001
(title: "CR fixes per DewittJ and DougArnett")

[commit-bot: I haz the power, 2016-06-03 23:43:07]

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

[commit-bot: I haz the power, 2016-06-04 01:05:17]

Description was changed from

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
==========

to

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
==========

[commit-bot: I haz the power, 2016-06-04 01:05:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-04 01:06:51]

Description was changed from

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325
==========

to

==========
Switch to currentTimeMillis, persist task

Since we are persisting our GCM Network Manager task across reboots,
we need to switch to a type of time that will work reliably across
reboots.

We consisered SystemClock.elapsedRealtime, but we think that bad UMA
samples will be more likely from reboots than the user adjusting
wall clock time.

BUG=612325

Committed: https://crrev.com/4667f21d36db6c8bb92a7f4b2dc4eced06f1bea4
Cr-Commit-Position: refs/heads/master@{#397877}
==========

[commit-bot: I haz the power, 2016-06-04 01:06:52]

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