Bug 1467461 - Migrate CrashReportingService to JobIntentService; r?snorp draft
authorPetru Lingurar <petru.lingurar@softvision.ro>
Mon, 11 Jun 2018 14:19:32 +0300
changeset 806731 69d3afb2bec1442d22fed223d8c68e3472809e65
parent 806725 9941eb8c3b29d152851220b5d9791326c35e1c68
push id112923
push userplingurar@mozilla.com
push dateMon, 11 Jun 2018 11:36:06 +0000
reviewerssnorp
bugs1467461
milestone62.0a1
Bug 1467461 - Migrate CrashReportingService to JobIntentService; r?snorp Use a stub service to actually start our crash reporter. On 26+ devices it will be called with "am start-foreground-service". This ensures it can be started even from background and the crash reporting process would work as before but ActivityManager will post an ANR error to logcat after 5 seconds because we aren't calling Service.startForeground() (which would mean a user visible notification). MozReview-Commit-ID: GATl6Waa9St
mobile/android/geckoview/build.gradle
mobile/android/geckoview/src/main/AndroidManifest.xml
mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterService.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterStarterService.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/GeckoLoader.java
mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/mobile/android/geckoview/build.gradle
+++ b/mobile/android/geckoview/build.gradle
@@ -135,16 +135,17 @@ android {
                     exclude 'org/mozilla/gecko/media/GeckoHlsAudioRenderer.java'
                     exclude 'org/mozilla/gecko/media/GeckoHlsPlayer.java'
                     exclude 'org/mozilla/gecko/media/GeckoHlsRendererBase.java'
                     exclude 'org/mozilla/gecko/media/GeckoHlsVideoRenderer.java'
                     exclude 'org/mozilla/gecko/media/Utils.java'
                 }
 
                 if (!mozconfig.substs.MOZ_CRASHREPORTER) {
+                    exclude 'org/mozilla/gecko/CrashReporterStarterService.java'
                     exclude 'org/mozilla/gecko/CrashReporterService.java'
                 }
 
                 if (mozconfig.substs.MOZ_WEBRTC) {
                     srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/base/java/src"
                     srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/audio_device/android/java/src"
                     srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/video_capture/android/java/src"
                 }
--- a/mobile/android/geckoview/src/main/AndroidManifest.xml
+++ b/mobile/android/geckoview/src/main/AndroidManifest.xml
@@ -75,14 +75,23 @@
         <service
                 android:name="org.mozilla.gecko.gfx.SurfaceAllocatorService"
                 android:enabled="true"
                 android:exported="false"
                 android:isolatedProcess="false">
         </service>
         <service
                 android:name="org.mozilla.gecko.CrashReporterService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"
                 android:process=":crashreporter">
         </service>
+
+        <service
+            android:name="org.mozilla.gecko.CrashReporterStarterService"
+            android:enabled="true"
+            android:exported="false"
+            android:process=":crashreporter">
+        </service>
+
     </application>
 
 </manifest>
\ No newline at end of file
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java
@@ -1,39 +1,38 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; 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;
 
+import android.content.Context;
+import android.content.Intent;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.net.Uri;
+import android.os.Build;
+import android.os.Bundle;
+import android.os.Process;
+import android.util.Log;
+
+import org.mozilla.geckoview.BuildConfig;
+
 import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.Arrays;
 import java.util.UUID;
 
-import android.content.ComponentName;
-import android.content.Context;
-import android.content.Intent;
-import android.content.pm.PackageInfo;
-import android.content.pm.PackageManager;
-import android.net.Uri;
-import android.os.Build;
-import android.os.Bundle;
-import android.os.Process;
-import android.util.Log;
-
-import org.mozilla.geckoview.BuildConfig;
-
 public class CrashHandler implements Thread.UncaughtExceptionHandler {
 
     private static final String LOGTAG = "GeckoCrashHandler";
     private static final Thread MAIN_THREAD = Thread.currentThread();
     private static final String DEFAULT_SERVER_URL =
         "https://crash-reports.mozilla.com/submit?id=%1$s&version=%2$s&buildid=%3$s";
 
     // Context for getting device information
@@ -298,19 +297,18 @@ public class CrashHandler implements Thr
             final String javaPkg = getJavaPackageName();
             final String pkg = getAppPackageName();
             final String component = javaPkg + ".CrashReporterService";
             final String action = javaPkg + ".reportCrash";
             final ProcessBuilder pb;
 
             if (context != null) {
                 final Intent intent = new Intent(action);
-                intent.setComponent(new ComponentName(pkg, component));
                 intent.putExtra("minidumpPath", dumpFile);
-                context.startService(intent);
+                CrashReporterService.enqueueWork(context, intent);
                 return true;
             }
 
             if (Build.VERSION.SDK_INT < 17) {
                 pb = new ProcessBuilder(
                     "/system/bin/am", "start",
                     "-a", action,
                     "-n", pkg + '/' + component,
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterService.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterService.java
@@ -1,21 +1,23 @@
 package org.mozilla.gecko;
 
+import android.content.Context;
+import android.content.Intent;
+import android.os.Build;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
+import android.util.Log;
+
 import org.mozilla.gecko.mozglue.GeckoLoader;
 import org.mozilla.gecko.mozglue.MinidumpAnalyzer;
 import org.mozilla.gecko.util.INIParser;
 import org.mozilla.gecko.util.INISection;
 import org.mozilla.gecko.util.ProxySelector;
 
-import android.app.IntentService;
-import android.content.Intent;
-import android.os.Build;
-import android.util.Log;
-
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -28,17 +30,17 @@ import java.net.URLDecoder;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.security.MessageDigest;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.zip.GZIPOutputStream;
 
-public class CrashReporterService extends IntentService {
+public class CrashReporterService extends JobIntentService {
     private static final String LOGTAG = "CrashReporter";
     private static final String ACTION_REPORT_CRASH = "org.mozilla.gecko.reportCrash";
     private static final String PASSED_MINI_DUMP_KEY = "minidumpPath";
     private static final String PASSED_MINI_DUMP_SUCCESS_KEY = "minidumpSuccess";
     private static final String MINI_DUMP_PATH_KEY = "upload_file_minidump";
     private static final String PAGE_URL_KEY = "URL";
     private static final String NOTES_KEY = "Notes";
     private static final String SERVER_URL_KEY = "ServerURL";
@@ -47,38 +49,45 @@ public class CrashReporterService extend
     private static final String PENDING_SUFFIX = CRASH_REPORT_SUFFIX + "pending";
     private static final String SUBMITTED_SUFFIX = CRASH_REPORT_SUFFIX + "submitted";
 
     private File mPendingMinidumpFile;
     private File mPendingExtrasFile;
     private HashMap<String, String> mExtrasStringMap;
     private boolean mMinidumpSucceeded;
 
-    public CrashReporterService() {
-        super("CrashReporterService");
-    }
-
     @Override
-    protected void onHandleIntent(Intent intent) {
-        if (intent == null || !intent.getAction().equals(ACTION_REPORT_CRASH)) {
+    protected void onHandleWork(Intent intent) {
+        if (intent == null || intent.getAction() == null || !intent.getAction().equals(ACTION_REPORT_CRASH)) {
             Log.d(LOGTAG, "Invalid or unknown action");
             return;
         }
 
         Class<?> reporterActivityCls = getFennecReporterActivity();
         if (reporterActivityCls != null) {
             intent.setClass(this, reporterActivityCls);
             intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
             startActivity(intent);
             return;
         }
 
         submitCrash(intent);
     }
 
+    @Override
+    public boolean onStopCurrentWork() {
+        // If JobScheduler stopped execution before work being completed it should not be restarted.
+        return false;
+    }
+
+    static void enqueueWork(@NonNull final Context context, @NonNull final Intent intent) {
+        final int jobId = -14;  // taken from org.mozilla.gecko.background.JobIdsConstants
+        enqueueWork(context, CrashReporterService.class, jobId, intent);
+    }
+
     private Class<?> getFennecReporterActivity() {
         try {
             return Class.forName("org.mozilla.gecko.CrashReporterActivity");
         } catch (ClassNotFoundException e) {
             return null;
         }
     }
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashReporterStarterService.java
@@ -0,0 +1,44 @@
+package org.mozilla.gecko;
+
+import android.app.Service;
+import android.content.Intent;
+import android.os.IBinder;
+import android.support.annotation.Nullable;
+
+/**
+ * Used by <b>nsExceptionHandler.cpp</b> to start the {@link CrashReporterService}
+ * responsible for reporting about crashes.<br>
+ *
+ * This is an ugly hack to actually start a JobIntentService but needed because:
+ * <ul>
+ *     <li>We need to use <em>/system/bin/am</em> to start our crash reporting
+ *          as after a crash, in many cases, the Dalvik VM is in an unusable state.
+ *     </li>
+ *     <li>
+ *         <em>/system/bin/am</em> cannot start a JobIntentService
+ *     </li>
+ *     <li>A recent bug in Android prevents us from sending a broadcast from <em>am</em>.
+ *          See <a href="http://dev.w3.org/html5/webvtt">call stack</a>
+ *     </li>
+ * </ul>
+ *
+ * This service is started with <em>am start-foreground-service</em> which let's us start
+ * as a background service.<br>
+ * We do not though call <a href="http://dev.w3.org/html5/webvtt">startForeground(..)</a> immediately
+ * and accept that after the 5 seconds the system would automatically stop us.<br>
+ * (We should have started {@link CrashReporterService} a long time before that anyway.)
+ */
+public class CrashReporterStarterService extends Service {
+    @Override
+    public int onStartCommand(Intent intent, int flags, int startId) {
+        CrashReporterService.enqueueWork(this, intent);
+
+        return Service.START_NOT_STICKY;
+    }
+
+    @Nullable
+    @Override
+    public IBinder onBind(Intent intent) {
+        return null;
+    }
+}
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/GeckoLoader.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/GeckoLoader.java
@@ -133,16 +133,18 @@ public final class GeckoLoader {
                 putenv("MOZ_ANDROID_USER_SERIAL_NUMBER=" + um.getSerialNumberForUser(android.os.Process.myUserHandle()));
             } else {
                 Log.d(LOGTAG, "Unable to obtain user manager service on a device with SDK version " + Build.VERSION.SDK_INT);
             }
         }
 
         putenv("LANG=" + Locale.getDefault().toString());
 
+        putenv("MOZ_ANDROID_DEVICE_SDK_VERSION=" + Build.VERSION.SDK_INT);
+
         // env from extras could have reset out linker flags; set them again.
         loadLibsSetupLocked(context);
     }
 
     private static void loadLibsSetupLocked(Context context) {
         // setup the libs cache
         putenv("GRE_HOME=" + getGREDir(context).getPath());
         putenv("MOZ_LINKER_CACHE=" + getCacheDir(context).getPath());
new file mode 100644
--- /dev/null
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/background/JobIdsConstants.java
@@ -0,0 +1,35 @@
+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;
+
+    public static final int JOB_ID_TAB_RECEIVED = -5;
+
+    public static final int JOB_ID_PROFILE_FETCH = -6;
+    public static final int JOB_ID_PROFILE_DELETE = -7;
+
+    public static final int JOB_ID_TELEMETRY_UPLOAD = -8;
+
+    public static final int JOB_ID_FILE_CLEANUP = -9;
+
+    public static final int JOB_ID_UPDATES_REGISTER = -10;
+    public static final int JOB_ID_UPDATES_CHECK_FOR = -11;
+    public static final int JOB_ID_UPDATES_DOWNLOAD = -12;
+    public static final int JOB_ID_UPDATES_APPLY = -13;
+
+    // Our package is unavailable to geckoview classes.
+    // Currently used in org.mozilla.gecko classes.CrashReporterService
+    // The id is kept here to ensure unicity between all job ids.
+    private static final int JOB_ID_CRASH_REPORTER = -14;
+}
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -215,16 +215,20 @@ static time_t lastCrashTime = 0;
 // The pathname of a file to store the crash time in
 static XP_CHAR lastCrashTimeFilename[XP_PATH_MAX] = {0};
 
 #if defined(MOZ_WIDGET_ANDROID)
 // on Android 4.2 and above there is a user serial number associated
 // with the current process that gets lost when we fork so we need to
 // explicitly pass it to am
 static char* androidUserSerial = nullptr;
+
+// Before Android 8 we needed to use "startservice" to start the crash reporting service.
+// After Android 8 we need to use "start-foreground-service"
+static char* startServiceCommand = nullptr;
 #endif
 
 // this holds additional data sent via the API
 static Mutex* crashReporterAPILock;
 static Mutex* notesFieldLock;
 static AnnotationTable* crashReporterAPIData_Hash;
 static nsCString* crashReporterAPIData = nullptr;
 static nsCString* crashEventAPIData = nullptr;
@@ -835,27 +839,27 @@ LaunchCrashReporterActivity(XP_CHAR* aPr
 
   if (pid == -1)
     return false;
   else if (pid == 0) {
     // Invoke the reportCrash activity using am
     if (androidUserSerial) {
       Unused << execlp("/system/bin/am",
                        "/system/bin/am",
-                       "startservice",
+                       startServiceCommand,
                        "--user", androidUserSerial,
                        "-a", "org.mozilla.gecko.reportCrash",
                        "-n", aProgramPath,
                        "--es", "minidumpPath", aMinidumpPath,
                        "--ez", "minidumpSuccess", aSucceeded ? "true" : "false",
                        (char*)0);
     } else {
       Unused << execlp("/system/bin/am",
                        "/system/bin/am",
-                       "startservice",
+                       startServiceCommand,
                        "-a", "org.mozilla.gecko.reportCrash",
                        "-n", aProgramPath,
                        "--es", "minidumpPath", aMinidumpPath,
                        "--ez", "minidumpSuccess", aSucceeded ? "true" : "false",
                        (char*)0);
     }
     _exit(1);
 
@@ -1539,22 +1543,29 @@ nsresult SetExceptionHandler(nsIFile* aX
 #endif // XP_WIN32
 #else
   // On Android, we launch using the application package name instead of a
   // filename, so use the dynamically set MOZ_ANDROID_PACKAGE_NAME, or fall
   // back to the static ANDROID_PACKAGE_NAME.
   const char* androidPackageName = PR_GetEnv("MOZ_ANDROID_PACKAGE_NAME");
   if (androidPackageName != nullptr) {
     nsCString package(androidPackageName);
-    package.AppendLiteral("/org.mozilla.gecko.CrashReporterService");
+    package.AppendLiteral("/org.mozilla.gecko.CrashReporterStarterService");
     crashReporterPath = ToNewCString(package);
   } else {
-    nsCString package(ANDROID_PACKAGE_NAME "/org.mozilla.gecko.CrashReporterService");
+    nsCString package(ANDROID_PACKAGE_NAME "/org.mozilla.gecko.CrashReporterStarterService");
     crashReporterPath = ToNewCString(package);
   }
+
+  const int deviceSdkVersion = std::stoi(getenv("MOZ_ANDROID_DEVICE_SDK_VERSION"));
+  if (deviceSdkVersion >= 26) {
+    startServiceCommand = (char*)"start-foreground-service";
+  } else {
+    startServiceCommand = (char*)"startservice";
+  }
 #endif // !defined(MOZ_WIDGET_ANDROID)
 
   // get temp path to use for minidump path
 #if defined(XP_WIN32)
   nsString tempPath;
 #else
   nsCString tempPath;
 #endif