tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit baf64f20f33d129bc68b69b608252fe4b6a67a1a
parent 989e6a4ac9109403a85419051e54b73694a83a2f
Author: mike a. <mavduevskiy@mozilla.com>
Date:   Tue, 18 Nov 2025 06:20:16 +0000

Bug 1955887 - Part 4: Report app icon change failures to sentry r=android-reviewers,marcin,twhite

Differential Revision: https://phabricator.services.mozilla.com/D271724

Diffstat:
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt | 1+
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelectionFragment.kt | 2++
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/ChangeAppLauncherIcon.kt | 28+++++++++++++++++++++++-----
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/utils/ChangeAppLauncherIconTest.kt | 109+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 133 insertions(+), 7 deletions(-)

diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -773,6 +773,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { appAlias = ComponentName(this, "$packageName.App"), alternativeAppAlias = ComponentName(this, "$packageName.AlternativeApp"), resetToDefault = FxNimbus.features.alternativeAppLauncherIcon.value().resetToDefault, + crashReporter = components.analytics.crashReporter, ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelectionFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelectionFragment.kt @@ -14,6 +14,7 @@ import androidx.navigation.fragment.findNavController import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.iconpicker.AppIconMiddleware import org.mozilla.fenix.iconpicker.AppIconRepository @@ -78,6 +79,7 @@ class AppIconSelectionFragment : Fragment(), UserInteractionHandler { shortcutInfo = ShortcutsUpdaterDefault(this), appAlias = ComponentName(this, "$packageName.${currentIcon.aliasSuffix}"), newAppAlias = ComponentName(this, "$packageName.${newIcon.aliasSuffix}"), + crashReporter = components.analytics.crashReporter, ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/ChangeAppLauncherIcon.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/utils/ChangeAppLauncherIcon.kt @@ -10,6 +10,7 @@ import android.content.pm.PackageManager import androidx.annotation.VisibleForTesting import androidx.core.content.pm.ShortcutInfoCompat import androidx.core.content.pm.ShortcutManagerCompat +import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.nimbus.FxNimbus @@ -27,6 +28,8 @@ private val logger = Logger("ChangeAppLauncherIcon") * @param appAlias The 'default' app alias defined in AndroidManifest.xml. * @param alternativeAppAlias The 'alternative' app alias defined in AndroidManifest.xml. * @param resetToDefault True to reset the icon to default, otherwise false to use the alternative icon. + * @param crashReporter An instance of [CrashReporting] to record expected caught exceptions during + * shortcuts update. */ fun changeAppLauncherIcon( context: Context, @@ -35,17 +38,25 @@ fun changeAppLauncherIcon( appAlias: ComponentName, alternativeAppAlias: ComponentName, resetToDefault: Boolean, + crashReporter: CrashReporting, ) { val userHasAlternativeAppIconSet = userHasAlternativeAppIconSet(context.packageManager, appAlias, alternativeAppAlias) if (resetToDefault && userHasAlternativeAppIconSet) { - resetAppIconsToDefault(context, shortcutManager, shortcutInfo, appAlias, alternativeAppAlias) + resetAppIconsToDefault(context, shortcutManager, shortcutInfo, appAlias, alternativeAppAlias, crashReporter) return } if (!resetToDefault && !userHasAlternativeAppIconSet) { - changeAppLauncherIcon(context.packageManager, shortcutManager, shortcutInfo, appAlias, alternativeAppAlias) + changeAppLauncherIcon( + context.packageManager, + shortcutManager, + shortcutInfo, + appAlias, + alternativeAppAlias, + crashReporter, + ) } } @@ -72,6 +83,8 @@ private fun userHasAlternativeAppIconSet( * @param shortcutInfo A helper class for updating [ShortcutInfoCompat]. * @param appAlias The currently used app alias. * @param newAppAlias The app alias we are updating to. + * @param crashReporter An instance of [CrashReporting] to record expected caught exceptions during + * shortcuts update. * @param updateShortcuts A function that attempts to update the pinned shortcuts to use the [newAppAlias]. * * @returns `true` if the app icon was successfully updated, otherwise `false` @@ -82,12 +95,13 @@ fun changeAppLauncherIcon( shortcutInfo: ShortcutsUpdater, appAlias: ComponentName, newAppAlias: ComponentName, - updateShortcuts: (ShortcutManagerWrapper, ShortcutsUpdater, ComponentName) -> Boolean = + crashReporter: CrashReporting, + updateShortcuts: (ShortcutManagerWrapper, ShortcutsUpdater, ComponentName, CrashReporting) -> Boolean = ::updateShortcutsComponentName, ): Boolean { newAppAlias.setEnabledStateTo(packageManager, true) - val updated = updateShortcuts(shortcutManager, shortcutInfo, newAppAlias) + val updated = updateShortcuts(shortcutManager, shortcutInfo, newAppAlias, crashReporter) if (updated) { logger.info("Successfully attempted to update the app icon to the alternative.") appAlias.setEnabledStateTo(packageManager, false) @@ -105,11 +119,12 @@ private fun resetAppIconsToDefault( shortcutInfo: ShortcutsUpdater, appAlias: ComponentName, alternativeAppAlias: ComponentName, + crashReporter: CrashReporting, ) { val packageManager = context.packageManager appAlias.setEnabledStateToDefault(packageManager) - if (updateShortcutsComponentName(shortcutManager, shortcutInfo, appAlias)) { + if (updateShortcutsComponentName(shortcutManager, shortcutInfo, appAlias, crashReporter)) { logger.info("Successfully attempted to reset the app icon to default.") alternativeAppAlias.setEnabledStateToDefault(packageManager) } else { @@ -151,10 +166,12 @@ internal fun updateShortcutsComponentName( shortcutManager: ShortcutManagerWrapper, shortcutInfo: ShortcutsUpdater, targetAlias: ComponentName, + crashReporter: CrashReporting, ): Boolean { val currentPinnedShortcuts = try { shortcutManager.getPinnedShortcuts() } catch (e: IllegalStateException) { + crashReporter.submitCaughtException(e) logger.warn("Failed to retrieve the current Firefox pinned shortcuts", e) return false } @@ -165,6 +182,7 @@ internal fun updateShortcutsComponentName( shortcutManager.updateShortcuts(updatedPinnedShortcuts) true } catch (e: IllegalArgumentException) { + crashReporter.submitCaughtException(e) logger.warn("Failed to update the given Firefox shortcuts: $updatedPinnedShortcuts", e) false } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/utils/ChangeAppLauncherIconTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/utils/ChangeAppLauncherIconTest.kt @@ -9,6 +9,11 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager import androidx.core.content.pm.ShortcutInfoCompat +import kotlinx.coroutines.Job +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.launch +import mozilla.components.concept.base.crash.Breadcrumb +import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.support.test.any import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext @@ -32,10 +37,12 @@ class ChangeAppLauncherIconTest { @Mock private lateinit var shortcutWrapper: ShortcutManagerWrapper private lateinit var shortcutsUpdater: ShortcutsUpdater + private lateinit var fakeCrashReporter: CrashReporting @Before fun setup() { openMocks(this) + fakeCrashReporter = TestCrashReporter() shortcutsUpdater = ShortcutsUpdaterDefault(testContext) } @@ -62,6 +69,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -104,6 +112,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -145,6 +154,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -183,6 +193,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -226,6 +237,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -268,6 +280,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -305,6 +318,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -343,6 +357,7 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, + fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -384,7 +399,8 @@ class ChangeAppLauncherIconTest { shortcutInfo = shortcutsUpdater, appAlias = appAlias, newAppAlias = newAppAlias, - updateShortcuts = { _, _, _ -> false }, + crashReporter = fakeCrashReporter, + updateShortcuts = { _, _, _, _ -> false }, ) assertEquals(PackageManager.COMPONENT_ENABLED_STATE_ENABLED, packageManager.getComponentEnabledSetting(appAlias)) @@ -417,7 +433,8 @@ class ChangeAppLauncherIconTest { shortcutInfo = shortcutsUpdater, appAlias = appAlias, newAppAlias = newAppAlias, - updateShortcuts = { _, _, _ -> true }, + crashReporter = fakeCrashReporter, + updateShortcuts = { _, _, _, _ -> true }, ) assertEquals(PackageManager.COMPONENT_ENABLED_STATE_DISABLED, packageManager.getComponentEnabledSetting(appAlias)) @@ -443,6 +460,7 @@ class ChangeAppLauncherIconTest { shortcutManager = throwingWrapper, shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), + crashReporter = fakeCrashReporter, ), ) } @@ -465,6 +483,7 @@ class ChangeAppLauncherIconTest { shortcutManager = throwingWrapper, shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), + crashReporter = fakeCrashReporter, ), ) } @@ -476,6 +495,7 @@ class ChangeAppLauncherIconTest { shortcutManager = TestShortcutManagerWrapper(testContext), shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), + crashReporter = fakeCrashReporter, ), ) } @@ -495,6 +515,78 @@ class ChangeAppLauncherIconTest { assertEquals(shortcut.intent, result.intent) assertEquals(newAppAlias, result.activity) } + + @Test + fun `GIVEN getPinnedShortcuts throws WHEN changeAppLauncherIcon is called THEN the error is submitted to a crash recorder`() { + val packageManager = testContext.packageManager + val appAlias = ComponentName("test", "App") + packageManager.setComponentEnabledSetting( + appAlias, + PackageManager.COMPONENT_ENABLED_STATE_DISABLED, + PackageManager.DONT_KILL_APP, + ) + val alternativeAppAlias = ComponentName("test", "AppAlternative") + packageManager.setComponentEnabledSetting( + alternativeAppAlias, + PackageManager.COMPONENT_ENABLED_STATE_ENABLED, + PackageManager.DONT_KILL_APP, + ) + val fakeCrashReporter = TestCrashReporter() + val error = IllegalStateException("Pinned shortcuts were too fast to catch!") + val shortcutWrapper = object : ShortcutManagerWrapper { + override fun getPinnedShortcuts(): List<ShortcutInfoCompat> { throw error } + override fun updateShortcuts(updatedShortcuts: List<ShortcutInfoCompat>) {} + } + + changeAppLauncherIcon( + testContext, + shortcutWrapper, + shortcutsUpdater, + appAlias, + alternativeAppAlias, + true, + fakeCrashReporter, + ) + + assertTrue(fakeCrashReporter.submitCaughtExceptionInvoked) + assertTrue(fakeCrashReporter.errors.contains(error)) + } + + @Test + fun `GIVEN updateShortcuts throws WHEN changeAppLauncherIcon is called THEN the error is submitted to a crash recorder`() { + val packageManager = testContext.packageManager + val appAlias = ComponentName("test", "App") + packageManager.setComponentEnabledSetting( + appAlias, + PackageManager.COMPONENT_ENABLED_STATE_DISABLED, + PackageManager.DONT_KILL_APP, + ) + val alternativeAppAlias = ComponentName("test", "AppAlternative") + packageManager.setComponentEnabledSetting( + alternativeAppAlias, + PackageManager.COMPONENT_ENABLED_STATE_ENABLED, + PackageManager.DONT_KILL_APP, + ) + val fakeCrashReporter = TestCrashReporter() + val error = IllegalArgumentException("Provided shortcuts were too cool to be updated!") + val shortcutWrapper = object : ShortcutManagerWrapper { + override fun getPinnedShortcuts(): List<ShortcutInfoCompat> { return emptyList() } + override fun updateShortcuts(updatedShortcuts: List<ShortcutInfoCompat>) { throw error } + } + + changeAppLauncherIcon( + testContext, + shortcutWrapper, + shortcutsUpdater, + appAlias, + alternativeAppAlias, + true, + fakeCrashReporter, + ) + + assertTrue(fakeCrashReporter.submitCaughtExceptionInvoked) + assertTrue(fakeCrashReporter.errors.contains(error)) + } } private fun createShortcut(componentName: ComponentName) = @@ -520,3 +612,16 @@ private class TestShortcutManagerWrapper(context: Context) : ShortcutManagerWrap updateShortcutsCapture = updatedShortcuts } } + +private class TestCrashReporter() : CrashReporting { + var submitCaughtExceptionInvoked = false + var errors: MutableList<Throwable> = mutableListOf() + + override fun submitCaughtException(throwable: Throwable): Job { + submitCaughtExceptionInvoked = true + errors.add(throwable) + return MainScope().launch {} + } + + override fun recordCrashBreadcrumb(breadcrumb: Breadcrumb) {} +}