Wireframe scheduler implementation

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java

Index: chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java
new file mode 100644
index 0000000000000000000000000000000000000000..0195202c90e5066c191b5beb616ed62ba19637d6
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundScheduler.java
@@ -0,0 +1,61 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.offlinepages;
+
+import android.content.Context;
+import android.os.Bundle;
+
+import com.google.android.gms.gcm.GcmNetworkManager;
+import com.google.android.gms.gcm.OneoffTask;
+import com.google.android.gms.gcm.Task;
+
+import org.chromium.chrome.browser.ChromeBackgroundService;
+
+import java.util.Date;
+
+/**
+ * The background scheduler class is for setting GCM Network Manager tasks.
+ */
+public class BackgroundScheduler {
+    /** Bundle key for the timestamp in milliseconds when the request started. */
+    private static final String DATE_TAG = "Date";
+    private static final long ONE_WEEK_IN_SECONDS = 60 * 60 * 24 * 7;
+
+    /**
+     * For the given Triggering conditions, start a new GCM Network Manager request.
+     */
+    public static void schedule(Context context) {
+        // Get the GCM Network Scheduler.
+        GcmNetworkManager gcmNetworkManager = GcmNetworkManager.getInstance(context);
+
+        // TODO(petewil): Today this puts timestamp, add triggering conditions into bundle.

[Bernhard Bauer, 2016/05/26 11:17:44]

Nit: I can't parse this sentence :)

[Pete Williamson, 2016/05/26 17:02:45]

OK, try again, I rephrased it,hopefully with better grammar.

+        // Triggering conditions will include network state and charging requirements, maybe
+        // also battery percentage.
+        Bundle taskExtras = new Bundle();
+        Date now = new Date();

[Bernhard Bauer, 2016/05/26 11:17:44]

I don't exactly know what you are going to use this for, but note that wall
clock time might change. It looks like your task is not persisting across
reboots, so if you just want to get elapsed time when the task is run, you could
use a guaranteed monotonic clock (e.g. SystemClock.elapsedRealtime).

[Pete Williamson, 2016/05/26 17:02:45]

This is to mark the starting point of an elapsed time calculation we will make
when the task runs.  I'm definitely open to using a different kind of time
class, but I'm not sure which is best here, and I'd welcome any feedback.

[Bernhard Bauer, 2016/05/27 12:36:15]

OK, yeah, I would use SystemClock.elapsedRealtime() for that.

[Pete Williamson, 2016/05/27 17:57:58]

Done.  I'll need to be careful to check for negative elapsed time in case the
device reboots, then.

[Bernhard Bauer, 2016/05/31 14:15:26]

AFAICT, your tasks are currently not persisted across reboots, so that couldn't
happen.

[Pete Williamson, 2016/05/31 14:30:15]

FYI: Our tasks are persisted across reboots.  We put the date into the bundle
that we give the GCM network manager to give back to us when it wakes up Chrome.
 There is a chance Chrome will have gotten kicked out of memory before it wakes
up.  The android device could reboot before the network comes back.  So, we are
using the bundle to persist the task start time, even across reboots (Assuming
the GCM Network Manager still remembers to wake us up across reboots.)

[Bernhard Bauer, 2016/05/31 14:57:31]

Uh, the default for one-off tasks is to be _not_ persisted (see
https://developers.google.com/android/reference/com/google/android/gms/gcm/Ta...).

+        taskExtras.putLong(DATE_TAG, now.getTime());

[Bernhard Bauer, 2016/05/26 11:17:44]

Hm... this creates the bundle, but BackgroundSchedulerBridge pulls information
out of it (or at least is supposed to). It would probably make more sense to do
both in the same class?

[Pete Williamson, 2016/05/26 17:02:45]

We want to keep BackgroundSchedulerBridge at thin as possible because it is hard
to test across the JNI boundary, so we wanted to keep that out of
BackgroundSchedulerBridge.  BackgroundTask (Now BackgroundOfflinerTask) will
extract this date in the future to make an elapsed time computation for UMA
purposes.)

[Bernhard Bauer, 2016/05/27 12:36:15]

Yeah, it doesn't have to be in BackgroundSchedulerBridge, but it might be a good
idea to have bundle encoding and decoding in the same class (once you actually
do that).

FTR, if you want to ease testing, you could actually move the native methods
into a non-static class, where you could override them in a subclass for
testing. Otherwise even with a thin BackgroundSchedulerBridge it will be
difficult to stub them out in a test (you'll just have moved the problem from
avoiding a nativeFoo() call to avoiding a static BackgroundSchedulerBridge.foo()
call).

[Pete Williamson, 2016/05/27 17:57:58]

Good idea.  I'm working on a test changelist right now, and I'll make a
non-static class that I can mock which calls out to the static bridge class. 
The non-static class might be a good place for the packing/unpacking logic. 
TODO added.

+
+        // Create a task.

[Bernhard Bauer, 2016/05/26 11:17:44]

Nit: some of these comments are bordering on no informational content that can't
be gathered from the code :)

[Pete Williamson, 2016/05/26 17:02:45]

Comment removed.  Yep, sorry, that's my personal style clashing with the style
guidelines.  I've always thought that it is faster to read comments to see what
is going on than read the code, so you can scan past the code faster looking for
relevant parts.

+        Task task = new OneoffTask.Builder()
+                .setService(ChromeBackgroundService.class)
+                .setExecutionWindow(0, ONE_WEEK_IN_SECONDS)
+                .setTag(OfflinePageUtils.TASK_TAG)
+                .setUpdateCurrent(true)
+                .setRequiredNetwork(Task.NETWORK_STATE_CONNECTED)
+                .setRequiresCharging(false)
+                .setExtras(taskExtras)
+                .build();
+
+        // Schedule the task.
+        gcmNetworkManager.schedule(task);
+    }
+
+    /**
+     * Cancel any outstanding GCM Network Manager requests.
+     */
+    public static void unschedule(Context context) {
+        // TODO(petewil): Take our task off the task queue.
+    }
+}