From 1a9b3eaf58a8a73134926a2ba98141600ae422d3 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 785669f24f3..4d8431c69e1 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 @@ -468,62 +468,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 a7740f99b6e..9bcc79f3345 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 @@ -1752,34 +1752,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(