Bug 1407046 - Migrate TelemetryUploadService to JobIntentService; r?JanH draft
authorPetru Lingurar <plingurar@mozilla.com>
Tue, 12 Jun 2018 06:49:52 +0300
changeset 806836 12137ee83e82130257e506e5a42cfdbfcbf0f51c
parent 806835 4fd35d478965d499cdee3ce6c18e67c3f8fcd0c2
child 806837 c40b5211f33bbbae2e12f0fbd86acacba5086042
push id112967
push userbmo:petru.lingurar@softvision.ro
push dateTue, 12 Jun 2018 04:13:23 +0000
reviewersJanH
bugs1407046
milestone62.0a1
Bug 1407046 - Migrate TelemetryUploadService to JobIntentService; r?JanH MozReview-Commit-ID: 8UGDzgmY81y
mobile/android/base/AndroidManifest.xml.in
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
mobile/android/base/java/org/mozilla/gecko/telemetry/schedulers/TelemetryUploadAllPingsImmediatelyScheduler.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
@@ -436,16 +436,17 @@
             android:exported="false">
             <intent-filter>
                 <action android:name="android.intent.action.MY_PACKAGE_REPLACED"></action>
             </intent-filter>
         </receiver>
 
         <service
           android:name="org.mozilla.gecko.telemetry.TelemetryUploadService"
+          android:permission="android.permission.BIND_JOB_SERVICE"
           android:exported="false"/>
 
         <service
             android:name="org.mozilla.gecko.customtabs.GeckoCustomTabsService"
             android:exported="true">
             <intent-filter>
                 <action android:name="android.support.customtabs.action.CustomTabsService" />
             </intent-filter>
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -1,25 +1,22 @@
 /* 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.telemetry;
 
-import android.app.IntentService;
 import android.content.Context;
 import android.content.Intent;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import android.util.Log;
-import ch.boye.httpclientandroidlib.HttpHeaders;
-import ch.boye.httpclientandroidlib.HttpResponse;
-import ch.boye.httpclientandroidlib.client.ClientProtocolException;
-import ch.boye.httpclientandroidlib.client.methods.HttpRequestBase;
-import ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
+
 import org.mozilla.gecko.GeckoProfile;
-import org.mozilla.gecko.Telemetry;
+import org.mozilla.gecko.background.JobIdsConstants;
 import org.mozilla.gecko.preferences.GeckoPreferences;
 import org.mozilla.gecko.restrictions.Restrictable;
 import org.mozilla.gecko.restrictions.Restrictions;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.net.BaseResource;
 import org.mozilla.gecko.sync.net.BaseResourceDelegate;
 import org.mozilla.gecko.sync.net.Resource;
 import org.mozilla.gecko.telemetry.stores.TelemetryPingStore;
@@ -31,72 +28,83 @@ import java.io.IOException;
 import java.net.URISyntaxException;
 import java.security.GeneralSecurityException;
 import java.util.Calendar;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
+import ch.boye.httpclientandroidlib.HttpHeaders;
+import ch.boye.httpclientandroidlib.HttpResponse;
+import ch.boye.httpclientandroidlib.client.ClientProtocolException;
+import ch.boye.httpclientandroidlib.client.methods.HttpRequestBase;
+import ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
+
 /**
  * The service that handles retrieving a list of telemetry pings to upload from the given
  * {@link TelemetryPingStore}, uploading those payloads to the associated server, and reporting
  * back to the Store which uploads were a success.
  */
-public class TelemetryUploadService extends IntentService {
+public class TelemetryUploadService extends JobIntentService {
     private static final String LOGTAG = StringUtils.safeSubstring("Gecko" + TelemetryUploadService.class.getSimpleName(), 0, 23);
-    private static final String WORKER_THREAD_NAME = LOGTAG + "Worker";
 
     public static final String ACTION_UPLOAD = "upload";
     public static final String EXTRA_STORE = "store";
 
     // TelemetryUploadService can run in a background thread so for future proofing, we set it volatile.
     private static volatile boolean isDisabled = false;
 
     public static void setDisabled(final boolean isDisabled) {
         TelemetryUploadService.isDisabled = isDisabled;
         if (isDisabled) {
             Log.d(LOGTAG, "Telemetry upload disabled (env var?");
         }
     }
 
-    public TelemetryUploadService() {
-        super(WORKER_THREAD_NAME);
-
-        // Intent redelivery can fail hard (e.g. we OOM as we try to upload, the Intent gets redelivered, repeat)
-        // so for simplicity, we avoid it. We expect the upload service to eventually get called again by the caller.
-        setIntentRedelivery(false);
+    public static void enqueueWork(@NonNull final Context context, @NonNull final Intent workIntent) {
+        enqueueWork(context, TelemetryUploadService.class, JobIdsConstants.getIdForTelemetryUploadJob(), workIntent);
     }
 
     /**
      * Handles a ping with the mandatory extras:
      *   * EXTRA_STORE: A {@link TelemetryPingStore} where the pings to upload are located
      */
     @Override
-    public void onHandleIntent(final Intent intent) {
+    protected void onHandleWork(@NonNull Intent intent) {
         Log.d(LOGTAG, "Service started");
 
         if (!isReadyToUpload(this, intent)) {
             return;
         }
 
         final TelemetryPingStore store = intent.getParcelableExtra(EXTRA_STORE);
         final boolean wereAllUploadsSuccessful = uploadPendingPingsFromStore(this, store);
         store.maybePrunePings();
         Log.d(LOGTAG, "Service finished: upload and prune attempts completed");
 
         if (!wereAllUploadsSuccessful) {
             // If we had an upload failure, we should stop the IntentService and drop any
             // pending Intents in the queue so we don't waste resources (e.g. battery)
             // trying to upload when there's likely to be another connection failure.
             Log.d(LOGTAG, "Clearing Intent queue due to connection failures");
+
+            // TODO investigate the opportunity and possible implications of calling stopSelf()
+            // in this JobIntentService when running on >= Oreo - bug 1468284
             stopSelf();
         }
     }
 
+    @Override
+    public boolean onStopCurrentWork() {
+        // The work could fail hard (e.g. we OOM as we try to upload) so we will not restart it.
+        // We expect the upload service to eventually get called again by the caller.
+        return false;
+    }
+
     /**
      * @return true if all pings were uploaded successfully, false otherwise.
      */
     private static boolean uploadPendingPingsFromStore(final Context context, final TelemetryPingStore store) {
         final List<TelemetryPing> pingsToUpload = store.getAllPings();
         if (pingsToUpload.isEmpty()) {
             return true;
         }
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/schedulers/TelemetryUploadAllPingsImmediatelyScheduler.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/schedulers/TelemetryUploadAllPingsImmediatelyScheduler.java
@@ -19,14 +19,13 @@ public class TelemetryUploadAllPingsImme
     @Override
     public boolean isReadyToUpload(final Context applicationContext, final TelemetryPingStore store) {
         // We're ready since we don't have any conditions to wait on (e.g. on wifi, accumulated X pings).
         return true;
     }
 
     @Override
     public void scheduleUpload(final Context applicationContext, final TelemetryPingStore store) {
-        final Intent i = new Intent(TelemetryUploadService.ACTION_UPLOAD);
-        i.setClass(applicationContext, TelemetryUploadService.class);
-        i.putExtra(TelemetryUploadService.EXTRA_STORE, store);
-        applicationContext.startService(i);
+        final Intent intent = new Intent(TelemetryUploadService.ACTION_UPLOAD);
+        intent.putExtra(TelemetryUploadService.EXTRA_STORE, store);
+        TelemetryUploadService.enqueueWork(applicationContext, intent);
     }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
@@ -24,16 +24,18 @@ public class JobIdsConstants {
     private static final int JOB_ID_DLC_SYNCHRONIZE = -3;
     private static final int JOB_ID_DLC_CLEANUP = -4;
 
     private static final int JOB_ID_TAB_RECEIVED = -5;
 
     private static final int JOB_ID_PROFILE_FETCH = -6;
     private static final int JOB_ID_PROFILE_DELETE = -7;
 
+    private static final int JOB_ID_TELEMETRY_UPLOAD = -8;
+
     public static int getIdForDlcStudyJob() {
         return getIdWithOffset(JOB_ID_DLC_STUDY);
     }
 
     public static int getIdForDlcDownloadJob() {
         return getIdWithOffset(JOB_ID_DLC_DOWNLOAD);
     }
 
@@ -52,16 +54,20 @@ public class JobIdsConstants {
     public static int getIdForProfileFetchJob() {
         return getIdWithOffset(JOB_ID_PROFILE_FETCH);
     }
 
     public static int getIdForProfileDeleteJob() {
         return getIdWithOffset(JOB_ID_PROFILE_DELETE);
     }
 
+    public static int getIdForTelemetryUploadJob() {
+        return getIdWithOffset(JOB_ID_TELEMETRY_UPLOAD);
+    }
+
     private static boolean isReleaseBuild() {
         return AppConstants.RELEASE_OR_BETA;
     }
 
     private static int getIdWithOffset(final int jobIdUsedInRelease) {
         return isReleaseBuild() ? jobIdUsedInRelease : jobIdUsedInRelease + BETA_OFFSET;
     }
 }