From 864f9209dcbec07b463312c2d90f9bc11a3e01f7 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Thu, 8 Aug 2019 15:59:11 -0400 Subject: [PATCH] For #3439 - Add "report" action to crash notification --- .../java/mozilla/components/lib/crash/Crash.kt | 2 ++ .../components/lib/crash/CrashReporter.kt | 4 ++-- .../lib/crash/handler/CrashHandlerService.kt | 12 +++++++++++- .../lib/crash/notification/CrashNotification.kt | 10 +++++++++- .../components/lib/crash/prompt/CrashPrompt.kt | 9 +++++++++ .../components/lib/crash/CrashReporterTest.kt | 17 +++++++++++++++++ .../lib/crash/handler/ExceptionHandlerTest.kt | 3 ++- 7 files changed, 52 insertions(+), 5 deletions(-) diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt index 10021c8ea90..7b0c6732c77 100644 --- a/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt +++ b/components/lib/crash/src/main/java/mozilla/components/lib/crash/Crash.kt @@ -86,6 +86,8 @@ sealed class Crash { } companion object { + internal const val INTENT_EXTRA_SKIP_PROMPT = "skipPrompt" + fun fromIntent(intent: Intent): Crash { val bundle = intent.getBundleExtra(INTENT_CRASH)!! diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt index 422b2bb5b93..cefda2b5b58 100644 --- a/components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt +++ b/components/lib/crash/src/main/java/mozilla/components/lib/crash/CrashReporter.kt @@ -85,14 +85,14 @@ class CrashReporter( logger.info("Crash report submitted to ${services.size} services") } - internal fun onCrash(context: Context, crash: Crash) { + internal fun onCrash(context: Context, crash: Crash, skipPrompt: Boolean = false) { if (!enabled) { return } logger.info("Received crash: $crash") - if (CrashPrompt.shouldPromptForCrash(shouldPrompt, crash)) { + if (CrashPrompt.shouldPromptForCrash(shouldPrompt, crash) && !skipPrompt) { if (shouldSendIntent(crash)) { // This crash was not fatal and the app has registered a pending intent: Send Intent to app and let the // app handle showing a confirmation UI. diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/handler/CrashHandlerService.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/handler/CrashHandlerService.kt index 3b8e6bc5210..317a8951bf1 100644 --- a/components/lib/crash/src/main/java/mozilla/components/lib/crash/handler/CrashHandlerService.kt +++ b/components/lib/crash/src/main/java/mozilla/components/lib/crash/handler/CrashHandlerService.kt @@ -5,6 +5,8 @@ package mozilla.components.lib.crash.handler import android.app.IntentService +import android.app.NotificationManager +import android.content.Context import android.content.Intent import mozilla.components.lib.crash.CrashReporter import mozilla.components.lib.crash.Crash @@ -29,10 +31,18 @@ class CrashHandlerService : IntentService(WORKER_THREAD_NAME) { intent.extras?.let { extras -> val crash = Crash.NativeCodeCrash.fromBundle(extras) + val skipPrompt = extras.getBoolean(Crash.INTENT_EXTRA_SKIP_PROMPT, false) + + if (skipPrompt) { + val notificationManager = getSystemService(Context.NOTIFICATION_SERVICE) + as NotificationManager + notificationManager.cancelAll() + sendBroadcast(Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)) + } CrashReporter .requireInstance - .onCrash(this, crash) + .onCrash(this, crash, skipPrompt) } ?: logger.error("Received intent with null extras") kill() diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/notification/CrashNotification.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/notification/CrashNotification.kt index 3ddfb84587a..d84e353c664 100644 --- a/components/lib/crash/src/main/java/mozilla/components/lib/crash/notification/CrashNotification.kt +++ b/components/lib/crash/src/main/java/mozilla/components/lib/crash/notification/CrashNotification.kt @@ -20,6 +20,8 @@ import mozilla.components.support.base.ids.notify private const val NOTIFICATION_CHANNEL_ID = "Crashes" private const val NOTIFICATION_TAG = "mozac.lib.crash.CRASH" private const val NOTIFICATION_SDK_LEVEL = 29 // On Android Q+ we show a notification instead of a prompt +private const val PROMPT_REQUEST_CODE = 1 +private const val REPORT_REQUEST_CODE = 2 internal class CrashNotification( private val context: Context, @@ -28,7 +30,11 @@ internal class CrashNotification( ) { fun show() { val pendingIntent = PendingIntent.getActivity( - context, 0, CrashPrompt.createIntent(context, crash), 0 + context, PROMPT_REQUEST_CODE, CrashPrompt.createIntent(context, crash), 0 + ) + + val reportPendingIntent = PendingIntent.getService( + context, REPORT_REQUEST_CODE, CrashPrompt.createReportIntent(context, crash), 0 ) val channel = ensureChannelExists() @@ -39,6 +45,8 @@ internal class CrashNotification( .setPriority(NotificationCompat.PRIORITY_DEFAULT) .setCategory(NotificationCompat.CATEGORY_ERROR) .setContentIntent(pendingIntent) + .addAction(R.drawable.mozac_lib_crash_notification, context.getString( + R.string.mozac_lib_crash_notification_action_report), reportPendingIntent) .setAutoCancel(true) .build() diff --git a/components/lib/crash/src/main/java/mozilla/components/lib/crash/prompt/CrashPrompt.kt b/components/lib/crash/src/main/java/mozilla/components/lib/crash/prompt/CrashPrompt.kt index 330ca653320..094cb12eabf 100644 --- a/components/lib/crash/src/main/java/mozilla/components/lib/crash/prompt/CrashPrompt.kt +++ b/components/lib/crash/src/main/java/mozilla/components/lib/crash/prompt/CrashPrompt.kt @@ -8,6 +8,7 @@ import android.content.Context import android.content.Intent import mozilla.components.lib.crash.Crash import mozilla.components.lib.crash.CrashReporter +import mozilla.components.lib.crash.handler.CrashHandlerService internal class CrashPrompt( private val context: Context, @@ -29,6 +30,14 @@ internal class CrashPrompt( return intent } + fun createReportIntent(context: Context, crash: Crash): Intent { + val intent = Intent(context, CrashHandlerService::class.java) + crash.fillIn(intent) + intent.putExtra(Crash.INTENT_EXTRA_SKIP_PROMPT, true) + + return intent + } + fun shouldPromptForCrash(shouldPrompt: CrashReporter.Prompt, crash: Crash): Boolean { return when (shouldPrompt) { CrashReporter.Prompt.ALWAYS -> true diff --git a/components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt b/components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt index 51383d749f5..e0cd0e5a363 100644 --- a/components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt +++ b/components/lib/crash/src/test/java/mozilla/components/lib/crash/CrashReporterTest.kt @@ -259,4 +259,21 @@ class CrashReporterTest { val instanceField = CrashReporter::class.java.getDeclaredField("instance") assertTrue(Modifier.isVolatile(instanceField.modifiers)) } + + @Test + fun `crashReport will submit the report if skipPrompt is true`() { + val service: CrashReporterService = mock() + + val reporter = spy(CrashReporter( + services = listOf(service), + shouldPrompt = CrashReporter.Prompt.ONLY_NATIVE_CRASH + ).install(testContext)) + + val crash: Crash.NativeCodeCrash = mock() + + reporter.onCrash(testContext, crash, true) + + verify(reporter).submitReport(crash) + verify(reporter, never()).showPrompt(any(), eq(crash)) + } } diff --git a/components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt b/components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt index 7c3a144f386..6b011dbd4d2 100644 --- a/components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt +++ b/components/lib/crash/src/test/java/mozilla/components/lib/crash/handler/ExceptionHandlerTest.kt @@ -16,6 +16,7 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.fail import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify @@ -48,7 +49,7 @@ class ExceptionHandlerTest { val exception = RuntimeException("Hello World") handler.uncaughtException(Thread.currentThread(), exception) - verify(crashReporter).onCrash(any(), any()) + verify(crashReporter).onCrash(any(), any(), anyBoolean()) assertNotNull(capturedCrash)