Merge #6057

6057: Closes #6022: Let GeckoView prompt user when native code crash is non-fatal r=pocmo a=rocketsroger
Co-authored-by: Roger Yang <royang@mozilla.com>

components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt

@@ -153,15 +153,17 @@ class CrashReporter(
 
         logger.info("Received crash: $crash")
 
-        if (shouldSendIntent(crash)) {
-            // This crash was not fatal and the app has registered a pending intent
-            launchCrashIntent(context, crash)
-        }
-
         if (telemetryServices.isNotEmpty()) {
             sendCrashTelemetry(context, crash)
         }
 
+        // If crash is native code and non fatal then the view will handle the user prompt
+        if (shouldSendIntent(crash)) {
+            // App has registered a pending intent
+            sendNonFatalCrashIntent(context, crash)
+            return
+        }
+
         if (services.isNotEmpty()) {
             if (CrashPrompt.shouldPromptForCrash(shouldPrompt, crash)) {
                 showPromptOrNotification(context, crash)
@@ -171,7 +173,8 @@ class CrashReporter(
         }
     }
 
-    private fun launchCrashIntent(context: Context, crash: Crash) {
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun sendNonFatalCrashIntent(context: Context, crash: Crash) {
         logger.info("Invoking non-fatal PendingIntent")
 
         val additionalIntent = Intent()

components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt

@@ -29,6 +29,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
+import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.robolectric.Robolectric
 import org.robolectric.Shadows.shadowOf
@@ -132,7 +133,7 @@ class CrashReporterTest {
     }
 
     @Test
-    fun `CrashReporter will show prompt for native crash and with setup Prompt-ONLY_NATIVE_CRASH`() {
+    fun `CrashReporter will show prompt for fatal native crash and with setup Prompt-ONLY_NATIVE_CRASH`() {
         val service: CrashReporterService = mock()
         val telemetryService: CrashReporterService = mock()
 
@@ -143,7 +144,12 @@ class CrashReporterTest {
             scope = scope
         ).install(testContext))
 
-        val crash: Crash.NativeCodeCrash = mock()
+        val crash = Crash.NativeCodeCrash(
+            "dump.path",
+            true,
+            "extras.path",
+            isFatal = true,
+            breadcrumbs = arrayListOf())
 
         reporter.onCrash(testContext, crash)
 
@@ -455,6 +461,57 @@ class CrashReporterTest {
         assertEquals(false, receivedCrash.isFatal)
     }
 
+    @Test
+    fun `CrashReporter sends telemetry but don't send native crash if the crash is non-fatal and nonFatalPendingIntent is not null`() {
+        val service: CrashReporterService = mock()
+        val telemetryService: CrashReporterService = mock()
+
+        val reporter = spy(CrashReporter(
+            services = listOf(service),
+            telemetryServices = listOf(telemetryService),
+            shouldPrompt = CrashReporter.Prompt.NEVER,
+            nonFatalCrashIntent = mock(),
+            scope = scope
+        ).install(testContext))
+
+        val nativeCrash = Crash.NativeCodeCrash(
+            "dump.path",
+            true,
+            "extras.path",
+            isFatal = false,
+            breadcrumbs = arrayListOf())
+        reporter.onCrash(testContext, nativeCrash)
+
+        verify(reporter, never()).sendCrashReport(testContext, nativeCrash)
+        verify(reporter, times(1)).sendCrashTelemetry(testContext, nativeCrash)
+        verify(reporter, never()).showPrompt(any(), eq(nativeCrash))
+    }
+
+    @Test
+    fun `CrashReporter sends telemetry and crash if the crash is non-fatal and nonFatalPendingIntent is null`() {
+        val service: CrashReporterService = mock()
+        val telemetryService: CrashReporterService = mock()
+
+        val reporter = spy(CrashReporter(
+            services = listOf(service),
+            telemetryServices = listOf(telemetryService),
+            shouldPrompt = CrashReporter.Prompt.NEVER,
+            scope = scope
+        ).install(testContext))
+
+        val nativeCrash = Crash.NativeCodeCrash(
+            "dump.path",
+            true,
+            "extras.path",
+            isFatal = false,
+            breadcrumbs = arrayListOf())
+        reporter.onCrash(testContext, nativeCrash)
+
+        verify(reporter, times(1)).sendCrashReport(testContext, nativeCrash)
+        verify(reporter, times(1)).sendCrashTelemetry(testContext, nativeCrash)
+        verify(reporter, never()).showPrompt(any(), eq(nativeCrash))
+    }
+
     @Test
     fun `CrashReporter instance writes are visible across threads`() {
         val instanceField = CrashReporter::class.java.getDeclaredField("instance")

components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/CrashHandlerServiceTest.kt

@@ -21,6 +21,7 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.doNothing
+import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.verify
 
@@ -41,6 +42,7 @@ class CrashHandlerServiceTest {
         reporter = spy(CrashReporter(
             shouldPrompt = CrashReporter.Prompt.NEVER,
             services = listOf(mock()),
+            nonFatalCrashIntent = mock(),
             scope = scope
         )).install(testContext)
 
@@ -72,7 +74,9 @@ class CrashHandlerServiceTest {
 
         intent?.putExtra("fatal", true)
         service?.onStartCommand(intent, 0, 0)
+        verify(reporter)?.onCrash(any(), any())
         verify(reporter)?.sendCrashReport(any(), any())
+        verify(reporter, never())?.sendNonFatalCrashIntent(any(), any())
     }
 
     @Test
@@ -81,6 +85,8 @@ class CrashHandlerServiceTest {
 
         intent?.putExtra("fatal", false)
         service?.onStartCommand(intent, 0, 0)
-        verify(reporter)?.sendCrashReport(any(), any())
+        verify(reporter)?.onCrash(any(), any())
+        verify(reporter)?.sendNonFatalCrashIntent(any(), any())
+        verify(reporter, never())?.sendCrashReport(any(), any())
     }
 }