[Android] Adding scheduling through GcmNetworkManager

components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java

Index: components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java
diff --git a/components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java b/components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java
new file mode 100644
index 0000000000000000000000000000000000000000..4b4ab72d78a0ade447a2dc15b61d54c9496a15eb
--- /dev/null
+++ b/components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/BackgroundTaskGcmTaskService.java
@@ -0,0 +1,114 @@
+// Copyright 2017 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.components.background_task_scheduler;
+
+import com.google.android.gms.gcm.GcmNetworkManager;
+import com.google.android.gms.gcm.GcmTaskService;
+import com.google.android.gms.gcm.TaskParams;
+
+import org.chromium.base.ContextUtils;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+/** Delegates calls out to various tasks that need to run in the background. */
+public class BackgroundTaskGcmTaskService extends GcmTaskService {
+    private static final String TAG = "BkgrdTaskGcmTS";
+
+    /** Class that waits for the processing to be done. */
+    private static class Waiter {
+        // Wakelock is only held for 3 minutes by default for GcmTaskService.
+        private static final long MAX_TIMEOUT_SECONDS = 179;
+        private final CountDownLatch mLatch;
+        private long mWaiterTimeoutSeconds;
+        private boolean mIsRescheduleNeeded;
+        private boolean mHasTaskTimedOut;
+
+        public Waiter(long waiterTimeoutSeconds) {
+            mLatch = new CountDownLatch(1);
+            mWaiterTimeoutSeconds = Math.min(waiterTimeoutSeconds, MAX_TIMEOUT_SECONDS);
+        }
+
+        /** Start waiting for the processing to finish. */
+        public void startWaiting() {
+            try {
+                mHasTaskTimedOut = !mLatch.await(mWaiterTimeoutSeconds, TimeUnit.SECONDS);
+            } catch (InterruptedException e) {
+                Log.d(TAG, "Waiter interrupted while waiting.");
+            }
+        }
+
+        /** Called to finish waiting. */
+        public void onWaitDone(boolean needsRescheduling) {
+            mIsRescheduleNeeded = needsRescheduling;
+            mLatch.countDown();
+        }
+
+        /** @return Whether last task timed out. */
+        public boolean hasTaskTimedOut() {
+            return mHasTaskTimedOut;
+        }
+
+        /** @return Whether task needs to be rescheduled. */
+        public boolean isRescheduleNeeded() {
+            return mIsRescheduleNeeded;
+        }
+    }
+
+    private static class TaskFinishedCallbackGcmTaskService
+            implements BackgroundTask.TaskFinishedCallback {
+        private final Waiter mWaiter;
+
+        public TaskFinishedCallbackGcmTaskService(Waiter waiter) {
+            mWaiter = waiter;
+        }
+
+        @Override
+        public void taskFinished(final boolean needsReschedule) {
+            // TODO(fgorski): Does this need to happen on UI thread?

[fgorski, 2017/04/11 19:42:10]

Tommy?

[nyquist, 2017/04/18 21:58:28]

This will set Waiter#mIsRescheduleNeeded, which is checked on the UI thread in
onRunTask(...), so conceptually yes.
But! Good news everyone! AFAIK CountDownLatch has a happens-before guarantee
where actions you take on one thread that calls countDown() will happen-before
returning from await(). So no, this can happen on any thread.

[fgorski, 2017/04/20 05:54:15]

I'll keep it consistent with conceptual presumptions and remove TODO

+            ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+                @Override
+                public void run() {
+                    mWaiter.onWaitDone(needsReschedule);
+                }
+            });
+        }
+    }
+
+    @Override
+    public int onRunTask(TaskParams params) {
+        ThreadUtils.assertOnUiThread();
+        BackgroundTask backgroundTask =
+                BackgroundTaskSchedulerGcmNetworkManager.getBackgroundTaskFromTaskParams(params);
+        if (backgroundTask == null) {
+            Log.w(TAG, "Failed to start task. Could not instantiate class.");
+            return GcmNetworkManager.RESULT_FAILURE;
+        }
+
+        TaskParameters taskParams =
+                BackgroundTaskSchedulerGcmNetworkManager.getTaskParametersFromTaskParams(params);
+        Waiter waiter = new Waiter(Waiter.MAX_TIMEOUT_SECONDS);
+        boolean taskNeedsBackgroundProcessing =
+                backgroundTask.onStartTask(ContextUtils.getApplicationContext(), taskParams,
+                        new TaskFinishedCallbackGcmTaskService(waiter));
+
+        if (taskNeedsBackgroundProcessing) {

[nyquist, 2017/04/18 21:58:28]

I did try looking at making this less indented by:


if (!taskNeedsBackgroundProcessing) return GcmNetworkManager.RESULT_SUCCESS;

waiter.startWaiting();

if (waiter.isRescheduleNeeded()) return GcmNetworkManager.RESULT_RESCHEDULE;


And then continuing with the haskTaskTimedOut() block, but I'm not sure what I
think.
It's definitely easy to read. It's just that this inner-if-thing got a bit long
in the current style.

I don't feel strongly though. Feel free to keep as is.

[fgorski, 2017/04/20 05:54:15]

I split the big if into 3 statements. Let me know if how you feel about that.
(Might be an overkill, but you are right that it is easier to follow the logic
that way).

+            waiter.startWaiting();

[nyquist, 2017/04/18 21:58:28]

I can't see from the documentation at
https://developers.google.com/android/reference/com/google/android/gms/gcm/Gc...
whether onRunTask(..) is guaranteed to never run on the main thread.

I see that they mention that with multiple tasks executing at the same time,
they are never scheduled on the same thread. But they don't mention whether the
main / UI thread is ever in use.

If that is a guarantee they have, could you point to it? And if it's an
assumption we make from looking at the source or otherwise assuming, could you
add a comment?

For JobService, we get the call to onStartJob(...) on the main thread:
https://developer.android.com/reference/android/app/job/JobService.html
"This service executes each incoming job on a Handler running on your
application's main thread. This means that you must offload your execution logic
to another thread/handler/AsyncTask of your choosing."

[fgorski, 2017/04/20 05:54:15]

I'll test it and will get back to you. Perhaps my presumption of Main thread
execution is wrong.
The difference from job scheduler is the way they can preempt you, and probably
that is why they expect main thread to be available to handle stop event. If the
presumption here is wrong we could run into problems, perhaps around the waiter
release. I'll play with that.

[nyquist, 2017/04/25 20:44:15]

Did you figure out which thread this runs on?

[fgorski, 2017/04/26 17:22:35]

Done. Running on blocking ui thread now.

+            if (waiter.isRescheduleNeeded()
+                    || (waiter.hasTaskTimedOut()
+                               && backgroundTask.onStopTask(
+                                          ContextUtils.getApplicationContext(), taskParams))) {
+                return GcmNetworkManager.RESULT_RESCHEDULE;
+            }
+        }
+
+        return GcmNetworkManager.RESULT_SUCCESS;
+    }
+
+    @Override
+    public void onInitializeTasks() {}
+}