Bug 1407046 - Migrate DownloadContentService to JobIntentService; r?sdaswani draft
authorPetru Lingurar <petru.lingurar@softvision.ro>
Wed, 06 Jun 2018 17:59:09 +0300
changeset 805634 5ca0188e753129face1c89ce296d19495e26acbd
parent 805633 29768a32e5c2e59288e0f065b4f1bc219e8ac750
child 805635 cb5c9b7d74d9d818233ade7123bf5652dca30a54
push id112722
push userplingurar@mozilla.com
push dateFri, 08 Jun 2018 06:33:31 +0000
reviewerssdaswani
bugs1407046
milestone62.0a1
Bug 1407046 - Migrate DownloadContentService to JobIntentService; r?sdaswani Broke the big IntentService into four small JobIntentServices because the same JobIntentService class cannot be used with multiple JobIds (https://github.com/aosp-mirror/platform_frameworks_support/blob/b6838fd2d2e834fdd38aab34511d385cb7108f63/compat/src/main/java/android/support/v4/app/JobIntentService.java#L121) Also: - will make the code easier to be migrated to WorkManager in the future - more in line with SRP. It was initially doing too much. Cleaned the code a little, removed the unneededly creation of new Threads for DownloadContentCatalog().persistChanges() / .startLoadFromDisk() as those methods are always called from the background threads of the new JobIntentServices. The new DlcHelper helps reducing duplicated code. MozReview-Commit-ID: G3fsWYOGEbR ***
mobile/android/base/AndroidManifest.xml.in
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/dlc/DlcCleanupService.java
mobile/android/base/java/org/mozilla/gecko/dlc/DlcDownloadService.java
mobile/android/base/java/org/mozilla/gecko/dlc/DlcHelper.java
mobile/android/base/java/org/mozilla/gecko/dlc/DlcStudyService.java
mobile/android/base/java/org/mozilla/gecko/dlc/DlcSyncService.java
mobile/android/base/java/org/mozilla/gecko/dlc/DownloadContentService.java
mobile/android/base/java/org/mozilla/gecko/dlc/StudyAction.java
mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java
mobile/android/base/java/org/mozilla/gecko/dlc/VerifyAction.java
mobile/android/base/java/org/mozilla/gecko/dlc/catalog/DownloadContentCatalog.java
mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
--- a/mobile/android/base/AndroidManifest.xml.in
+++ b/mobile/android/base/AndroidManifest.xml.in
@@ -381,17 +381,36 @@
 
         <service
             android:exported="false"
             android:name="org.mozilla.gecko.notifications.NotificationService">
         </service>
 
         <service
             android:exported="false"
-            android:name="org.mozilla.gecko.dlc.DownloadContentService">
+            android:name="org.mozilla.gecko.dlc.DlcStudyService"
+            android:permission="android.permission.BIND_JOB_SERVICE" >
+        </service>
+
+        <service
+            android:exported="false"
+            android:name="org.mozilla.gecko.dlc.DlcSyncService"
+            android:permission="android.permission.BIND_JOB_SERVICE" >
+        </service>
+
+        <service
+            android:exported="false"
+            android:name="org.mozilla.gecko.dlc.DlcDownloadService"
+            android:permission="android.permission.BIND_JOB_SERVICE" >
+        </service>
+
+        <service
+            android:exported="false"
+            android:name="org.mozilla.gecko.dlc.DlcCleanupService"
+            android:permission="android.permission.BIND_JOB_SERVICE" >
         </service>
 
         <!-- DON'T EXPORT THIS, please! An attacker could delete arbitrary files. -->
         <service
             android:exported="false"
             android:name="org.mozilla.gecko.cleanup.FileCleanupService">
         </service>
 
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -87,17 +87,18 @@ import org.mozilla.gecko.db.BrowserContr
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.db.SuggestedSites;
 import org.mozilla.gecko.delegates.BookmarkStateChangeDelegate;
 import org.mozilla.gecko.delegates.BrowserAppDelegate;
 import org.mozilla.gecko.delegates.OfflineTabStatusDelegate;
 import org.mozilla.gecko.delegates.ScreenshotDelegate;
 import org.mozilla.gecko.distribution.Distribution;
 import org.mozilla.gecko.distribution.DistributionStoreCallback;
-import org.mozilla.gecko.dlc.DownloadContentService;
+import org.mozilla.gecko.dlc.DlcStudyService;
+import org.mozilla.gecko.dlc.DlcSyncService;
 import org.mozilla.gecko.extensions.ExtensionPermissionsHelper;
 import org.mozilla.gecko.firstrun.FirstrunAnimationContainer;
 import org.mozilla.gecko.gfx.DynamicToolbarAnimator;
 import org.mozilla.gecko.gfx.DynamicToolbarAnimator.PinReason;
 import org.mozilla.gecko.home.BrowserSearch;
 import org.mozilla.gecko.home.HomeBanner;
 import org.mozilla.gecko.home.HomeConfig;
 import org.mozilla.gecko.home.HomeConfig.PanelType;
@@ -729,17 +730,17 @@ public class BrowserApp extends GeckoApp
         safeStartingIntent = new SafeIntent(getIntent());
         isInAutomation = IntentUtils.getIsInAutomationFromEnvironment(safeStartingIntent);
 
         GeckoProfile.setIntentArgs(safeStartingIntent.getStringExtra("args"));
 
         if (!isInAutomation && AppConstants.MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE) {
             // Kick off download of app content as early as possible so that in the best case it's
             // available before the user starts using the browser.
-            DownloadContentService.startStudy(this);
+            DlcStudyService.enqueueServiceWork(this);
         }
 
         // This has to be prepared prior to calling GeckoApp.onCreate, because
         // widget code and BrowserToolbar need it, and they're created by the
         // layout, which GeckoApp takes care of.
         final GeckoApplication app = (GeckoApplication) getApplication();
         app.prepareLightweightTheme();
 
@@ -1967,18 +1968,17 @@ public class BrowserApp extends GeckoApp
                     // delaying work (such as upload) a few seconds.  Avoid any potential
                     // startup CPU/thread contention by delaying the pref broadcast.
                     GeckoPreferences.broadcastStumblerPref(BrowserApp.this);
                 }
 
                 if (AppConstants.MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE &&
                         !IntentUtils.getIsInAutomationFromEnvironment(new SafeIntent(getIntent()))) {
                     // TODO: Better scheduling of DLC actions (Bug 1257492)
-                    DownloadContentService.startSync(this);
-                    DownloadContentService.startVerification(this);
+                    DlcSyncService.enqueueServiceWork(this);
                 }
 
                 break;
 
             case "GeckoView:AccessibilityEnabled":
                 mDynamicToolbar.setAccessibilityEnabled(message.getBoolean("enabled"));
                 break;
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DlcCleanupService.java
@@ -0,0 +1,29 @@
+package org.mozilla.gecko.dlc;
+
+import android.content.Context;
+import android.content.Intent;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
+
+import org.mozilla.gecko.background.JobIdsConstants;
+import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
+
+public class DlcCleanupService extends JobIntentService {
+    private static final String LOGTAG = "GeckoDlcCleanupService";
+
+    @Override
+    protected void onHandleWork(@NonNull Intent intent) {
+        if (!DlcHelper.isDlcPossible(LOGTAG)) {
+            return;
+        }
+
+        final DownloadContentCatalog catalog = new DownloadContentCatalog(this);
+
+        new CleanupAction().perform(this, catalog);
+        catalog.persistChanges();
+    }
+
+    public static void enqueueServiceWork(@NonNull final Context context) {
+        enqueueWork(context, DlcCleanupService.class, JobIdsConstants.JOB_ID_DLC_CLEANUP, new Intent());
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DlcDownloadService.java
@@ -0,0 +1,40 @@
+package org.mozilla.gecko.dlc;
+
+import android.content.Context;
+import android.content.Intent;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
+
+import org.mozilla.gecko.EventDispatcher;
+import org.mozilla.gecko.background.JobIdsConstants;
+import org.mozilla.gecko.dlc.catalog.DownloadContent;
+import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
+
+public class DlcDownloadService extends JobIntentService {
+    private static final String LOGTAG = "GeckoDlcDownloadService";
+
+    @Override
+    protected void onHandleWork(@NonNull Intent intent) {
+        if (!DlcHelper.isDlcPossible(LOGTAG)) {
+            return;
+        }
+
+        final BaseAction dwAction = new DownloadAction(new DownloadAction.Callback() {
+            @Override
+            public void onContentDownloaded(DownloadContent content) {
+                if (content.isFont()) {
+                    EventDispatcher.getInstance().dispatch("Fonts:Reload", null);
+                }
+            }
+        });
+
+        final DownloadContentCatalog catalog = new DownloadContentCatalog(this);
+
+        dwAction.perform(this, catalog);
+        catalog.persistChanges();
+    }
+
+    public static void enqueueServiceWork(@NonNull final Context context) {
+        enqueueWork(context, DlcDownloadService.class, JobIdsConstants.JOB_ID_DLC_DOWNLOAD, new Intent());
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DlcHelper.java
@@ -0,0 +1,30 @@
+package org.mozilla.gecko.dlc;
+
+import android.util.Log;
+
+import org.mozilla.gecko.AppConstants;
+import org.mozilla.gecko.util.HardwareUtils;
+
+public class DlcHelper {
+    /**
+     * Checks whether this functionality is available
+     *
+     * @param logTag String used when logging to identify the source of a log message
+     * @return <code>true</code> if the dlc functionality is available<br>
+     *     <code>false</code> otherwise
+     */
+    static boolean isDlcPossible(String logTag) {
+        if (!AppConstants.MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE) {
+            Log.w(logTag, "Download content is not enabled. Stop.");
+            return false;
+        }
+
+        if (!HardwareUtils.isSupportedSystem()) {
+            // This service is running very early before checks in BrowserApp can prevent us from running.
+            Log.w(logTag, "System is not supported. Stop.");
+            return false;
+        }
+
+        return true;
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DlcStudyService.java
@@ -0,0 +1,29 @@
+package org.mozilla.gecko.dlc;
+
+import android.content.Context;
+import android.content.Intent;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
+
+import org.mozilla.gecko.background.JobIdsConstants;
+import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
+
+public class DlcStudyService extends JobIntentService {
+    private static final String LOGTAG = "GeckoDlcStudyService";
+
+    @Override
+    protected void onHandleWork(@NonNull Intent intent) {
+        if (!DlcHelper.isDlcPossible(LOGTAG)) {
+            return;
+        }
+
+        final DownloadContentCatalog catalog = new DownloadContentCatalog(this);
+
+        new StudyAction().perform(this, catalog);
+        catalog.persistChanges();
+    }
+
+    public static void enqueueServiceWork(@NonNull final Context context) {
+        enqueueWork(context, DlcStudyService.class, JobIdsConstants.JOB_ID_DLC_STUDY, new Intent());
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DlcSyncService.java
@@ -0,0 +1,32 @@
+package org.mozilla.gecko.dlc;
+
+import android.content.Context;
+import android.content.Intent;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
+
+import org.mozilla.gecko.background.JobIdsConstants;
+import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
+
+public class DlcSyncService extends JobIntentService {
+    private static final String LOGTAG = "GeckoDlcSyncService";
+
+    @Override
+    protected void onHandleWork(@NonNull Intent intent) {
+        if (!DlcHelper.isDlcPossible(LOGTAG)) {
+            return;
+        }
+
+        final DownloadContentCatalog catalog = new DownloadContentCatalog(this);
+
+        // the following will be executed serially
+        new SyncAction().perform(this, catalog);
+        catalog.persistChanges();
+        new VerifyAction().perform(this, catalog);
+        catalog.persistChanges();
+    }
+
+    public static void enqueueServiceWork(@NonNull final Context context) {
+        enqueueWork(context, DlcSyncService.class, JobIdsConstants.JOB_ID_DLC_SYNCHRONIZE, new Intent());
+    }
+}
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadContentService.java
+++ /dev/null
@@ -1,148 +0,0 @@
-/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.dlc;
-
-import org.mozilla.gecko.AppConstants;
-import org.mozilla.gecko.EventDispatcher;
-import org.mozilla.gecko.dlc.catalog.DownloadContent;
-import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
-import org.mozilla.gecko.util.HardwareUtils;
-
-import android.app.IntentService;
-import android.content.ComponentName;
-import android.content.Context;
-import android.content.Intent;
-import android.util.Log;
-
-/**
- * Service to handle downloadable content that did not ship with the APK.
- */
-public class DownloadContentService extends IntentService {
-    private static final String LOGTAG = "GeckoDLCService";
-
-    /**
-     * Study: Scan the catalog for "new" content available for download.
-     */
-    private static final String ACTION_STUDY_CATALOG = AppConstants.ANDROID_PACKAGE_NAME + ".DLC.STUDY";
-
-    /**
-     * Verify: Validate downloaded content. Does it still exist and does it have the correct checksum?
-     */
-    private static final String ACTION_VERIFY_CONTENT = AppConstants.ANDROID_PACKAGE_NAME + ".DLC.VERIFY";
-
-    /**
-     * Download content that has been scheduled during "study" or "verify".
-     */
-    private static final String ACTION_DOWNLOAD_CONTENT = AppConstants.ANDROID_PACKAGE_NAME + ".DLC.DOWNLOAD";
-
-    /**
-     * Sync: Synchronize catalog from a Kinto instance.
-     */
-    private static final String ACTION_SYNCHRONIZE_CATALOG = AppConstants.ANDROID_PACKAGE_NAME + ".DLC.SYNC";
-
-    /**
-     * CleanupAction: Remove content that is no longer needed (e.g. Removed from the catalog after a sync).
-     */
-    private static final String ACTION_CLEANUP_FILES = AppConstants.ANDROID_PACKAGE_NAME + ".DLC.CLEANUP";
-
-    public static void startStudy(Context context) {
-        Intent intent = new Intent(ACTION_STUDY_CATALOG);
-        intent.setComponent(new ComponentName(context, DownloadContentService.class));
-        context.startService(intent);
-    }
-
-    public static void startVerification(Context context) {
-        Intent intent = new Intent(ACTION_VERIFY_CONTENT);
-        intent.setComponent(new ComponentName(context, DownloadContentService.class));
-        context.startService(intent);
-    }
-
-    public static void startDownloads(Context context) {
-        Intent intent = new Intent(ACTION_DOWNLOAD_CONTENT);
-        intent.setComponent(new ComponentName(context, DownloadContentService.class));
-        context.startService(intent);
-    }
-
-    public static void startSync(Context context) {
-        Intent intent = new Intent(ACTION_SYNCHRONIZE_CATALOG);
-        intent.setComponent(new ComponentName(context, DownloadContentService.class));
-        context.startService(intent);
-    }
-
-    public static void startCleanup(Context context) {
-        Intent intent = new Intent(ACTION_CLEANUP_FILES);
-        intent.setComponent(new ComponentName(context, DownloadContentService.class));
-        context.startService(intent);
-    }
-
-    private DownloadContentCatalog catalog;
-
-    public DownloadContentService() {
-        super(LOGTAG);
-    }
-
-    @Override
-    public void onCreate() {
-        super.onCreate();
-
-        catalog = new DownloadContentCatalog(this);
-    }
-
-    protected void onHandleIntent(Intent intent) {
-        if (!AppConstants.MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE) {
-            Log.w(LOGTAG, "Download content is not enabled. Stop.");
-            return;
-        }
-
-        if (!HardwareUtils.isSupportedSystem()) {
-            // This service is running very early before checks in BrowserApp can prevent us from running.
-            Log.w(LOGTAG, "System is not supported. Stop.");
-            return;
-        }
-
-        if (intent == null) {
-            return;
-        }
-
-        final BaseAction action;
-
-        switch (intent.getAction()) {
-            case ACTION_STUDY_CATALOG:
-                action = new StudyAction();
-                break;
-
-            case ACTION_DOWNLOAD_CONTENT:
-                action = new DownloadAction(new DownloadAction.Callback() {
-                    @Override
-                    public void onContentDownloaded(DownloadContent content) {
-                        if (content.isFont()) {
-                            EventDispatcher.getInstance().dispatch("Fonts:Reload", null);
-                        }
-                    }
-                });
-                break;
-
-            case ACTION_VERIFY_CONTENT:
-                action = new VerifyAction();
-                break;
-
-            case ACTION_SYNCHRONIZE_CATALOG:
-                action = new SyncAction();
-                break;
-
-            case ACTION_CLEANUP_FILES:
-                action = new CleanupAction();
-                break;
-
-            default:
-                Log.e(LOGTAG, "Unknown action: " + intent.getAction());
-                return;
-        }
-
-        action.perform(this, catalog);
-        catalog.persistChanges();
-    }
-}
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/StudyAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/StudyAction.java
@@ -71,11 +71,11 @@ public class StudyAction extends BaseAct
             }
         }
 
         // There are no patterns or all patterns have matched.
         return true;
     }
 
     protected void startDownloads(Context context) {
-        DownloadContentService.startDownloads(context);
+        DlcDownloadService.enqueueServiceWork(context);
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/SyncAction.java
@@ -112,21 +112,21 @@ public class SyncAction extends BaseActi
         if (cleanupRequired) {
             startCleanupAction(context);
         }
 
         Log.v(LOGTAG, "Done");
     }
 
     protected void startStudyAction(Context context) {
-        DownloadContentService.startStudy(context);
+        DlcStudyService.enqueueServiceWork(context);
     }
 
     protected void startCleanupAction(Context context) {
-        DownloadContentService.startCleanup(context);
+        DlcCleanupService.enqueueServiceWork(context);
     }
 
     protected JSONArray fetchRawCatalog(long lastModified)
             throws RecoverableDownloadContentException, UnrecoverableDownloadContentException {
         HttpURLConnection connection = null;
 
         try {
             Uri.Builder builder = Uri.parse(CATALOG_ENDPOINT).buildUpon();
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/VerifyAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/VerifyAction.java
@@ -9,17 +9,17 @@ import android.content.Context;
 import android.util.Log;
 
 import org.mozilla.gecko.dlc.catalog.DownloadContent;
 import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
 
 import java.io.File;
 
 /**
- * Verify: Validate downloaded content. Does it still exist and does it have the correct checksum?
+ * Verify: Validate already downloaded content. Does it still exist and does it have the correct checksum?
  */
 public class VerifyAction extends BaseAction {
     private static final String LOGTAG = "DLCVerifyAction";
 
     @Override
     public void perform(Context context, DownloadContentCatalog catalog) {
         Log.d(LOGTAG, "Verifying catalog..");
 
@@ -53,11 +53,11 @@ public class VerifyAction extends BaseAc
         if (catalog.hasScheduledDownloads()) {
             startDownloads(context);
         }
 
         Log.v(LOGTAG, "Done");
     }
 
     protected void startDownloads(Context context) {
-        DownloadContentService.startDownloads(context);
+        DlcDownloadService.enqueueServiceWork(context);
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/catalog/DownloadContentCatalog.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/catalog/DownloadContentCatalog.java
@@ -169,29 +169,21 @@ public class DownloadContentCatalog {
             markAsPermanentlyFailed(content);
         } else {
             content.rememberFailure(failureType);
             hasCatalogChanged = true;
         }
     }
 
     public void persistChanges() {
-        new Thread(LOGTAG + "-Persist") {
-            public void run() {
-                writeToDisk();
-            }
-        }.start();
+        writeToDisk();
     }
 
     private void startLoadFromDisk() {
-        new Thread(LOGTAG + "-Load") {
-            public void run() {
-                loadFromDisk();
-            }
-        }.start();
+        loadFromDisk();
     }
 
     private void awaitLoadingCatalogLocked() {
         while (!hasLoadedCatalog) {
             try {
                 Log.v(LOGTAG, "Waiting for catalog to be loaded");
 
                 wait();
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
@@ -0,0 +1,16 @@
+package org.mozilla.gecko.background;
+
+/**
+ * Repository for all the IDs used to differentiate between different Android Jobs.<br>
+ * They should be unique across both the app’s own code and the code from any library that the app uses.<br>
+ * To try and mitigate this situations we will use IDs with negative values in our code.
+ *
+ * @see <a href="https://developer.android.com/reference/android/app/job/JobInfo.Builder#JobInfo.Builder(int, android.content.ComponentName)">
+ *     JobId importance</a>
+ */
+public class JobIdsConstants {
+    public static final int JOB_ID_DLC_STUDY = -1;
+    public static final int JOB_ID_DLC_DOWNLOAD = -2;
+    public static final int JOB_ID_DLC_SYNCHRONIZE = -3;
+    public static final int JOB_ID_DLC_CLEANUP = -4;
+}