Prevent crash reporter from being triggered once onDestroy has been c…

…alled. (#1801)
MozillaReality · Sep 9, 2019 · 17793e1 · 17793e1
1 parent 5b9f2ea
commit 17793e1
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 15 deletions.
diff --git a/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java b/app/src/common/shared/org/mozilla/vrbrowser/VRBrowserActivity.java
@@ -22,6 +22,7 @@
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Looper;
+import android.os.Process;
 import android.util.Log;
 import android.util.Pair;
 import android.view.KeyEvent;
@@ -179,6 +180,7 @@ protected void attachBaseContext(Context base) {
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
+        SettingsStore.getInstance(getBaseContext()).setPid(Process.myPid());
         // Fix for infinite restart on startup crashes.
         long count = SettingsStore.getInstance(getBaseContext()).getCrashRestartCount();
         boolean cancelRestart = count > CrashReporterService.MAX_RESTART_COUNT;
@@ -318,12 +320,14 @@ private void attachToWindow(@NonNull WindowWidget aWindow, @Nullable WindowWidge
 
     @Override
     protected void onStart() {
+        SettingsStore.getInstance(getBaseContext()).setPid(Process.myPid());
         super.onStart();
         TelemetryWrapper.start();
     }
 
     @Override
     protected void onStop() {
+        SettingsStore.getInstance(getBaseContext()).setPid(0);
         super.onStop();
 
         if (SettingsStore.getInstance(this).getCylinderDensity() > 0.0f) {
@@ -377,6 +381,7 @@ protected void onResume() {
 
     @Override
     protected void onDestroy() {
+        SettingsStore.getInstance(getBaseContext()).setPid(0);
         // Unregister the crash service broadcast receiver
         unregisterReceiver(mCrashReceiver);
         mSearchEngineWrapper.unregisterForUpdates();

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/browser/SettingsStore.java b/app/src/common/shared/org/mozilla/vrbrowser/browser/SettingsStore.java
@@ -577,5 +577,15 @@ public void setAutoplayEnabled(boolean isEnabled) {
         editor.putBoolean(mContext.getString(R.string.settings_key_autoplay), isEnabled);
         editor.commit();
     }
+
+    public void setPid(int aPid) {
+        SharedPreferences.Editor editor = mPrefs.edit();
+        editor.putInt(mContext.getString(R.string.settings_key_pid), aPid);
+        editor.commit();
+    }
+
+    public int getPid() {
+        return mPrefs.getInt(mContext.getString(R.string.settings_key_pid), 0);
+    }
 }
 
diff --git a/app/src/common/shared/org/mozilla/vrbrowser/crashreporting/CrashReporterService.java b/app/src/common/shared/org/mozilla/vrbrowser/crashreporting/CrashReporterService.java
@@ -5,7 +5,6 @@
 import android.content.Intent;
 import android.os.Build;
 import android.os.Process;
-import android.preference.PreferenceManager;
 import android.util.Log;
 
 import org.mozilla.geckoview.GeckoRuntime;
@@ -32,11 +31,9 @@ public class CrashReporterService extends JobIntentService {
     public static final long MAX_RESTART_COUNT = 2;
     private static final int MAX_PID_CHECK_COUNT = 5;
 
-    private int mPidCheckCount = 0;
-
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
-        Log.d(LOGTAG, "======> onStartCommand");
+        Log.d(LOGTAG, "onStartCommand");
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
             enqueueWork(this, CrashReporterService.class, JOB_ID, intent);
         }
@@ -48,42 +45,47 @@ public int onStartCommand(Intent intent, int flags, int startId) {
     protected void onHandleWork(@NonNull Intent intent) {
         String action = intent.getAction();
         if (GeckoRuntime.ACTION_CRASHED.equals(action)) {
+            final int activityPid = SettingsStore.getInstance(getBaseContext()).getPid();
             boolean fatal = intent.getBooleanExtra(GeckoRuntime.EXTRA_CRASH_FATAL, false);
 
             long count = SettingsStore.getInstance(getBaseContext()).getCrashRestartCount();
             boolean cancelRestart = count > MAX_RESTART_COUNT;
             if (cancelRestart || BuildConfig.DISABLE_CRASH_RESTART) {
-                Log.e(LOGTAG, "CANCEL CRASH HANDLER");
+                Log.e(LOGTAG, "Too many restarts. Abort crash reporter service.");
                 return;
             }
 
             if (fatal) {
-                Log.d(LOGTAG, "======> NATIVE CRASH PARENT " + intent);
-                final int pid = Process.myPid();
+                Log.d(LOGTAG, "Main process crash " + intent);
+                if (activityPid == 0) {
+                    Log.e(LOGTAG, "Application was quitting. Crash reporter will not trigger a restart.");
+                    return;
+                }
                 final ActivityManager activityManager = (ActivityManager) this.getSystemService(Context.ACTIVITY_SERVICE);
                 if (activityManager == null) {
                     return;
                 }
 
+                int pidCheckCount = 0;
                 do {
-                    boolean otherProcessesFound = false;
+                    boolean activityFound = false;
                     for (final ActivityManager.RunningAppProcessInfo info : activityManager.getRunningAppProcesses()) {
-                        if (pid != info.pid) {
-                            otherProcessesFound = true;
-                            Log.e(LOGTAG, "======> Found PID " + info.pid);
+                        if (activityPid == info.pid) {
+                            activityFound = true;
+                            Log.e(LOGTAG, "Main activity still running: " + activityPid);
                             break;
                         }
                     }
 
-                    if (!otherProcessesFound || (mPidCheckCount > MAX_PID_CHECK_COUNT)) {
+                    if (!activityFound || (pidCheckCount > MAX_PID_CHECK_COUNT)) {
                         intent.setClass(CrashReporterService.this, VRBrowserActivity.class);
                         intent.setPackage(BuildConfig.APPLICATION_ID);
                         intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
                         startActivity(intent);
                         break;
 
                     } else {
-                        mPidCheckCount++;
+                        pidCheckCount++;
                         try {
                             Thread.sleep(PID_CHECK_INTERVAL);
                         } catch (InterruptedException e) {
@@ -94,14 +96,14 @@ protected void onHandleWork(@NonNull Intent intent) {
                 } while (true);
 
             } else {
-                Log.d(LOGTAG, "======> NATIVE CRASH CONTENT" + intent);
+                Log.d(LOGTAG, "Content process crash " + intent);
                 Intent broadcastIntent = new Intent(CRASH_ACTION);
                 broadcastIntent.putExtra(DATA_TAG, intent);
                 sendBroadcast(broadcastIntent, getString(R.string.app_permission_name));
             }
         }
 
-        Log.d(LOGTAG, "======> Crash reporter job finished");
+        Log.d(LOGTAG, "Crash reporter job finished");
     }
 
 }
diff --git a/app/src/main/res/values/non_L10n.xml b/app/src/main/res/values/non_L10n.xml
@@ -46,6 +46,7 @@
     <string name="settings_key_notifications" translatable="false">settings_key_notifications</string>
     <string name="settings_key_debug_logging" translatable="false">settings_key_debug_logging</string>
     <string name="settings_key_autoplay" translatable="false">settings_key_autoplay</string>
+    <string name="settings_key_pid" translatable="false">settings_key_pid</string>
     <string name="environment_override_help_url" translatable="false">https://github.com/MozillaReality/FirefoxReality/wiki/Environments</string>
     <string name="private_policy_url" translatable="false">https://www.mozilla.org/privacy/firefox/</string>
     <string name="private_report_url" translatable="false">https://mixedreality.mozilla.org/fxr/report?src=browser-fxr&amp;label=browser-firefox-reality&amp;url=%1$s</string>