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

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

 // Copyright 2015 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.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;
 
 import com.google.android.gms.gcm.GcmListenerService;
 import com.google.ipc.invalidation.ticl.android2.channel.AndroidGcmController;
 
 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.
  */
 public class ChromeGcmListenerService extends GcmListenerService {
     private static final String TAG = "ChromeGcmListener";
 
     @Override
     public void onCreate() {
         ProcessInitializationHandler.getInstance().initializePreNative();
         super.onCreate();
     }
 
     @Override
     public void onMessageReceived(String from, Bundle data) {
         boolean hasCollapseKey = !TextUtils.isEmpty(data.getString("collapse_key"));
         GcmUma.recordDataMessageReceived(getApplicationContext(), hasCollapseKey);
 
         String invalidationSenderId = AndroidGcmController.get(this).getSenderId();
         if (from.equals(invalidationSenderId)) {
             AndroidGcmController.get(this).onMessageReceived(data);
             return;
         }
-        pushMessageReceived(from, data);
+
+        // Dispatch the message to the GCM Driver for native features.
+        scheduleOrDispatchMessageToDriver(getApplicationContext(), from, data);
     }
 
     @Override
     public void onMessageSent(String msgId) {
         Log.d(TAG, "Message sent successfully. Message id: " + msgId);
         GcmUma.recordGcmUpstreamHistogram(getApplicationContext(), GcmUma.UMA_UPSTREAM_SUCCESS);
     }
 
     @Override
     public void onSendError(String msgId, String error) {
         Log.w(TAG, "Error in sending message. Message id: " + msgId + " Error: " + error);
         GcmUma.recordGcmUpstreamHistogram(getApplicationContext(), GcmUma.UMA_UPSTREAM_SEND_FAILED);
     }
 
     @Override
     public void onDeletedMessages() {
         // TODO(johnme): Ask GCM to include the subtype in this event.
         Log.w(TAG, "Push messages were deleted, but we can't tell the Service Worker as we don't"
                 + "know what subtype (app ID) it occurred for.");
         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);
                 }
-            }
-        });
+            });
+        }
     }
 }