From 29831a18162a5c3f6b95fe56b4c71d81e1414afa Mon Sep 17 00:00:00 2001 From: Mugurell Date: Tue, 18 May 2021 11:10:38 +0300 Subject: [PATCH] For #8989 - Don't catch/rethrow prompt cast exceptions This try-catch was added in #6687 as an effort to better individualize prompt cast exceptions in our crash reporting platforms to better asses their frequency and hopefully get more data to debug and fix the underlying issue. The original issue is resolved in the first commit of this patch so there is no need to keep the try-catch. --- .../feature/prompts/PromptFeature.kt | 93 +++++++++---------- .../feature/prompts/PromptFeatureTest.kt | 28 ------ 2 files changed, 43 insertions(+), 78 deletions(-) diff --git a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt index 1792420745f..68269b9ae06 100644 --- a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt +++ b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt @@ -425,62 +425,55 @@ class PromptFeature private constructor( @Suppress("UNCHECKED_CAST", "ComplexMethod") override fun onConfirm(sessionId: String, promptRequestUID: String, value: Any?) { store.consumePromptFrom(sessionId, promptRequestUID, activePrompt) { - try { - when (it) { - is TimeSelection -> it.onConfirm(value as Date) - is Color -> it.onConfirm(value as String) - is Alert -> { - val shouldNotShowMoreDialogs = value as Boolean - promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) - it.onConfirm(!shouldNotShowMoreDialogs) - } - is SingleChoice -> it.onConfirm(value as Choice) - is MenuChoice -> it.onConfirm(value as Choice) - is BeforeUnload -> it.onLeave() - is Popup -> { - val shouldNotShowMoreDialogs = value as Boolean - promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) - it.onAllow() - } - is MultipleChoice -> it.onConfirm(value as Array) + when (it) { + is TimeSelection -> it.onConfirm(value as Date) + is Color -> it.onConfirm(value as String) + is Alert -> { + val shouldNotShowMoreDialogs = value as Boolean + promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) + it.onConfirm(!shouldNotShowMoreDialogs) + } + is SingleChoice -> it.onConfirm(value as Choice) + is MenuChoice -> it.onConfirm(value as Choice) + is BeforeUnload -> it.onLeave() + is Popup -> { + val shouldNotShowMoreDialogs = value as Boolean + promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) + it.onAllow() + } + is MultipleChoice -> it.onConfirm(value as Array) - is Authentication -> { - val (user, password) = value as Pair - it.onConfirm(user, password) - } + is Authentication -> { + val (user, password) = value as Pair + it.onConfirm(user, password) + } - is TextPrompt -> { - val (shouldNotShowMoreDialogs, text) = value as Pair + is TextPrompt -> { + val (shouldNotShowMoreDialogs, text) = value as Pair - promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) - it.onConfirm(!shouldNotShowMoreDialogs, text) - } + promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs) + it.onConfirm(!shouldNotShowMoreDialogs, text) + } - is Share -> it.onSuccess() - - is SaveLoginPrompt -> it.onConfirm(value as Login) - - is Confirm -> { - val (isCheckBoxChecked, buttonType) = - value as Pair - promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked) - when (buttonType) { - MultiButtonDialogFragment.ButtonType.POSITIVE -> - it.onConfirmPositiveButton(!isCheckBoxChecked) - MultiButtonDialogFragment.ButtonType.NEGATIVE -> - it.onConfirmNegativeButton(!isCheckBoxChecked) - MultiButtonDialogFragment.ButtonType.NEUTRAL -> - it.onConfirmNeutralButton(!isCheckBoxChecked) - } + is Share -> it.onSuccess() + + is SaveLoginPrompt -> it.onConfirm(value as Login) + + is Confirm -> { + val (isCheckBoxChecked, buttonType) = + value as Pair + promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked) + when (buttonType) { + MultiButtonDialogFragment.ButtonType.POSITIVE -> + it.onConfirmPositiveButton(!isCheckBoxChecked) + MultiButtonDialogFragment.ButtonType.NEGATIVE -> + it.onConfirmNegativeButton(!isCheckBoxChecked) + MultiButtonDialogFragment.ButtonType.NEUTRAL -> + it.onConfirmNeutralButton(!isCheckBoxChecked) } - - is Repost -> it.onConfirm() } - } catch (e: ClassCastException) { - throw IllegalArgumentException( - "PromptFeature onConsume cast failed with ${it.javaClass}", - e - ) + + is Repost -> it.onConfirm() } } } diff --git a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt index f72c4cd37f0..e511d5d3d0a 100644 --- a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt +++ b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt @@ -1678,34 +1678,6 @@ class PromptFeatureTest { assertFalse(prompt!!.shouldDismissOnLoad) } - @Test - fun `PromptFeature throws IllegalArgumentException when ClassCastException is triggered`() { - val feature = PromptFeature( - activity = mock(), - store = store, - fragmentManager = fragmentManager - ) { } - feature.start() - - val singleChoiceRequest = SingleChoice(arrayOf()) {} - var illegalArgumentExceptionThrown = false - store.dispatch(ContentAction.UpdatePromptRequestAction(tabId, singleChoiceRequest)) - .joinBlocking() - - try { - feature.onConfirm(tabId, singleChoiceRequest.uid, "wrong") - } catch (e: IllegalArgumentException) { - illegalArgumentExceptionThrown = true - assertEquals( - "PromptFeature onConsume cast failed with class mozilla.components.concept.engine.prompt.PromptRequest\$SingleChoice", - e.message - ) - } - - store.waitUntilIdle() - assert(illegalArgumentExceptionThrown) - } - @Test fun `A Repost PromptRequest prompt will be shown as a ConfirmDialogFragment`() { val feature = PromptFeature(