Route through a JobService when receiving a message for the GCM Driver

chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java

Index: chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java b/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
index d1a1660be5a4a59c13edd1a4acf117f01a0f9ee0..ecd80fc3a6371f9001d62856934526e1fbb64019 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/services/gcm/ChromeGcmListenerService.java
@@ -4,6 +4,11 @@
 
 package org.chromium.chrome.browser.services.gcm;
 
+import android.app.job.JobInfo;
+import android.app.job.JobScheduler;
+import android.content.ComponentName;
+import android.content.Context;
+import android.os.Build;
 import android.os.Bundle;
 import android.text.TextUtils;
 
@@ -14,9 +19,11 @@ import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
+import org.chromium.chrome.browser.JobSchedulerConstants;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.init.ProcessInitializationHandler;
 import org.chromium.components.gcm_driver.GCMDriver;
+import org.chromium.components.gcm_driver.GCMMessage;
 
 /**
  * Receives Downstream messages and status of upstream messages from GCM.
@@ -40,7 +47,9 @@ public class ChromeGcmListenerService extends GcmListenerService {
             AndroidGcmController.get(this).onMessageReceived(data);
             return;
         }
-        pushMessageReceived(from, data);
+
+        // Dispatch the message to the GCM Driver for native features.
+        scheduleOrDispatchMessageToDriver(getApplicationContext(), from, data);
     }
 
     @Override
@@ -63,28 +72,53 @@ public class ChromeGcmListenerService extends GcmListenerService {
         GcmUma.recordDeletedMessages(getApplicationContext());
     }
 
-    private void pushMessageReceived(final String from, final Bundle data) {
-        final String bundleSubtype = "subtype";
-        if (!data.containsKey(bundleSubtype)) {
-            Log.w(TAG, "Received push message with no subtype");
-            return;
+    /**
+     * To be called when a GCM message is ready to be dispatched. Will initialise the native code
+     * of the browser process, and forward the message to the GCM Driver.

[awdf, 2017/02/16 21:26:48]

nit: could add "Must be called on the UI thread."

[Peter Beverloo, 2017/02/17 15:43:59]

Done.

+     */
+    @SuppressFBWarnings("DM_EXIT")
+    public static void dispatchMessageToDriver(Context applicationContext, GCMMessage message) {

[awdf, 2017/02/16 21:26:48]

nit: can you make this method default (package) protected instead of public?
it's only called from this package

[Peter Beverloo, 2017/02/17 15:43:59]

Done.

+        ThreadUtils.assertOnUiThread();
+
+        try {
+            ChromeBrowserInitializer.getInstance(applicationContext).handleSynchronousStartup();
+            GCMDriver.dispatchMessage(message);
+
+        } catch (ProcessInitException e) {
+            Log.e(TAG, "ProcessInitException while starting the browser process");
+
+            // Since the library failed to initialize nothing in the application can work, so kill
+            // the whole application as opposed to just this service.
+            System.exit(-1);
         }
-        final String appId = data.getString(bundleSubtype);
-        ThreadUtils.runOnUiThread(new Runnable() {
-            @Override
-            @SuppressFBWarnings("DM_EXIT")
-            public void run() {
-                try {
-                    ChromeBrowserInitializer.getInstance(getApplicationContext())
-                        .handleSynchronousStartup();
-                    GCMDriver.onMessageReceived(appId, from, data);
-                } catch (ProcessInitException e) {
-                    Log.e(TAG, "ProcessInitException while starting the browser process");
-                    // Since the library failed to initialize nothing in the application
-                    // can work, so kill the whole application not just the activity.
-                    System.exit(-1);
+    }
+
+    /**
+     * Either schedules |message| to be dispatched through the Job Scheduler, which we use on
+     * Android N and beyond, or immediately dispatches the message on other versions of Android.
+     */
+    public static void scheduleOrDispatchMessageToDriver(

[awdf, 2017/02/16 21:26:48]

nit: place this method above dispatchMessageToDriver?

[Peter Beverloo, 2017/02/17 15:43:59]

Done. Why?

[awdf, 2017/02/18 07:51:41]

because this one calls dispatchMessageToDrive so logically comes between the
previous method & dispatchMessageToDrive (at least, this bugged me while
reviewing & scrolling, if I edited the code I probably wouldn't notice as I'd
just ctrl-click the method name in my IDE to jump about.. so this is a really
micro nit!)

+            final Context context, String senderId, Bundle extras) {
+        final GCMMessage message = new GCMMessage(senderId, extras);
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {

[awdf, 2017/02/16 21:26:48]

nit: I'm wondering if we should have a common shouldUseJobScheduler method for
all job services in case we decide to change this to M or O in the future

if we end up with only one job service this is moot though

[Peter Beverloo, 2017/02/17 15:43:59]

I think that'll depend on the feature, doesn't it? Don't feel strongly either
way, it could be a good way to centralize UMA too.

+            // TODO(peter): Add UMA for measuring latency introduced by the JobScheduler.
+            JobInfo job = new JobInfo
+                                  .Builder(JobSchedulerConstants.GCM_DRIVER_JOB_ID,
+                                          new ComponentName(context, GCMJobService.class))
+                                  .setExtras(message.toPersistableBundle())
+                                  .setOverrideDeadline(0)
+                                  .build();
+            JobScheduler scheduler =
+                    (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE);
+            scheduler.schedule(job);
+        } else {
+            ThreadUtils.runOnUiThread(new Runnable() {
+                @Override
+                public void run() {
+                    ChromeGcmListenerService.dispatchMessageToDriver(context, message);
                 }
-            }
-        });
+            });
+        }
     }
 }