commit c932fd8c5f7aedb4dcf3d7748289fce49e83601f parent 47362f8052957f8bc162ff033990a4f633c39535 Author: Sandor Molnar <smolnar@mozilla.com> Date: Tue, 18 Nov 2025 10:35:23 +0200 Revert "Bug 1955887 - Part 5: Refactor app icon state to have a separate warning state member r=android-reviewers,marcin,twhite" for causing fenix debug failures @ AppIconMiddlewareTest.kt This reverts commit 32b64690ec2dee5777ff9a526e4f11b5116b6d7f. Revert "Bug 1955887 - Part 4: Report app icon change failures to sentry r=android-reviewers,marcin,twhite" This reverts commit baf64f20f33d129bc68b69b608252fe4b6a67a1a. Revert "Bug 1955887 - Part 3: Preserve store state during configuration change r=android-reviewers,marcin,twhite" This reverts commit 989e6a4ac9109403a85419051e54b73694a83a2f. Revert "Bug 1955887 - Part 2: Add an error snackbar in case changing the icon failed r=android-reviewers,android-l10n-reviewers,flod,twhite" This reverts commit b61e00261553f3a669536d22ba846f4df4c3945d. Revert "Bug 1955887 - Part 1: Introduce a data layer to the app icon selector feature r=android-reviewers,marcin,twhite" This reverts commit 2a496edebb2ec930dae3ea146a7e9f64e233061c. Diffstat:
13 files changed, 69 insertions(+), 782 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,7 +773,6 @@ 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/AppIconAction.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconAction.kt @@ -1,72 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import mozilla.components.lib.state.Action - -/** - * Actions related to the app icon selection screen. - */ -sealed interface AppIconAction : Action - -/** - * User-initiated actions. - */ -sealed interface UserAction : AppIconAction { - /** - * The user has clicked on an icon. - * - * @property appIcon the new selected icon. - */ - data class Selected(val appIcon: AppIcon) : UserAction - - /** - * The user has confirmed applying the new icon on the warning dialog screen. - * - * @property oldIcon the currently used app icon. - * @property newIcon the new app icon to apply. - */ - data class Confirmed(val oldIcon: AppIcon, val newIcon: AppIcon) : UserAction - - /** - * The user has pressed the "Dismiss" button on the warning dialog. - */ - data object Dismissed : UserAction -} - -/** - * System-initiated actions. - */ -sealed interface SystemAction : AppIconAction { - /** - * The warning dialog has been canceled through means other than dismiss button (back gesture, - * tap outside of the dialog area, etc) - */ - data object DialogDismissed : SystemAction - - /** - * The system app icon has been successfully updated. - * - * @property newIcon the new icon. - */ - data class Applied(val newIcon: AppIcon) : SystemAction - - /** - * The app icon update failed (due to an exception thrown by a system call). - */ - data object UpdateFailed : SystemAction - - /** - * The app icon update error snackbar was dismissed. - */ - data object SnackbarDismissed : SystemAction - - /** - * Signals a context update. For example, due to config change. - * - * @property appIconUpdater an interface used by [AppIconMiddleware] for changing the app icon. - */ - data class EnvironmentRehydrated(val appIconUpdater: AppIconUpdater) : SystemAction -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconMiddleware.kt @@ -1,60 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import androidx.annotation.VisibleForTesting -import mozilla.components.lib.state.Middleware -import mozilla.components.lib.state.MiddlewareContext - -/** - * A middleware for handling side-effects in response to [AppIconAction]s. - * - * @property updateAppIcon A interface that updates the main activity alias with the newly selected one. - */ -class AppIconMiddleware( - @get:VisibleForTesting - internal var updateAppIcon: AppIconUpdater, -) : Middleware<AppIconState, AppIconAction> { - - override fun invoke( - context: MiddlewareContext<AppIconState, AppIconAction>, - next: (AppIconAction) -> Unit, - action: AppIconAction, - ) { - next(action) - - when (action) { - is UserAction.Confirmed -> { - if (updateAppIcon(old = action.newIcon, new = action.oldIcon)) { - context.dispatch(SystemAction.Applied(action.newIcon)) - } else { - context.dispatch(SystemAction.UpdateFailed) - } - } - - is SystemAction.EnvironmentRehydrated -> { - updateAppIcon = action.appIconUpdater - } - - is UserAction.Dismissed, - is UserAction.Selected, - is SystemAction.Applied, - is SystemAction.DialogDismissed, - is SystemAction.SnackbarDismissed, - is SystemAction.UpdateFailed, - - -> { - // no-op - } - } - } -} - -/** - * An interface for applying a new app icon. - */ -fun interface AppIconUpdater : (AppIcon, AppIcon) -> Boolean { - override fun invoke(old: AppIcon, new: AppIcon): Boolean -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconReducer.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconReducer.kt @@ -1,49 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -/** - * Function for reducing a new app icon state based on the received action. - */ -internal fun appIconReducer(state: AppIconState, action: AppIconAction) = when (action) { - is UserAction -> state.handleUserAction(action) - is SystemAction -> state.handleSystemAction(action) -} - -private fun AppIconState.handleUserAction(action: UserAction): AppIconState { - return when (action) { - is UserAction.Selected -> this.copy( - userSelectedAppIcon = action.appIcon, - warningDialogState = AppIconWarningDialog.Presenting(newIcon = action.appIcon), - ) - - is UserAction.Confirmed -> this - is UserAction.Dismissed -> this.copy( - userSelectedAppIcon = null, - warningDialogState = AppIconWarningDialog.None, - ) - } -} - -private fun AppIconState.handleSystemAction(action: SystemAction): AppIconState { - return when (action) { - is SystemAction.Applied -> this.copy( - currentAppIcon = action.newIcon, - userSelectedAppIcon = null, - warningDialogState = AppIconWarningDialog.None, - ) - SystemAction.DialogDismissed -> this.copy( - userSelectedAppIcon = null, - warningDialogState = AppIconWarningDialog.None, - ) - SystemAction.SnackbarDismissed -> this.copy(snackbarState = AppIconSnackbarState.None) - SystemAction.UpdateFailed -> this.copy( - userSelectedAppIcon = null, - snackbarState = AppIconSnackbarState.ApplyingNewIconError, - warningDialogState = AppIconWarningDialog.None, - ) - is SystemAction.EnvironmentRehydrated -> this - } -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconState.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconState.kt @@ -1,54 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import mozilla.components.lib.state.State - -/** - * Represents the state of the app icon picker screen. - * - * @property currentAppIcon The icon currently being used by the application. - * @property userSelectedAppIcon The icon the user has selected in the picker, if any. - * @property groupedIconOptions A map of all available app icons. - * @property snackbarState The current snackbar state. - * @property warningDialogState The current warning dialog state. - */ -data class AppIconState( - val currentAppIcon: AppIcon = AppIcon.AppDefault, - val userSelectedAppIcon: AppIcon? = null, - val groupedIconOptions: Map<IconGroupTitle, List<AppIcon>> = mapOf(), - val snackbarState: AppIconSnackbarState = AppIconSnackbarState.None, - val warningDialogState: AppIconWarningDialog = AppIconWarningDialog.None, -) : State - -/** - * The state of the snackbar to display. - */ -sealed class AppIconSnackbarState { - /** - * There is no snackbar to display. - */ - data object None : AppIconSnackbarState() - - /** - * Display a snackbar of the app icon update failure. - */ - data object ApplyingNewIconError : AppIconSnackbarState() -} - -/** - * The state of the app reset warning dialog - */ -sealed class AppIconWarningDialog { - /** - * No dialog to display. - */ - data object None : AppIconWarningDialog() - - /** - * The dialog is visible. - */ - data class Presenting(val newIcon: AppIcon) : AppIconWarningDialog() -} diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/AppIconStore.kt @@ -1,26 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import mozilla.components.lib.state.Middleware -import mozilla.components.lib.state.Reducer -import mozilla.components.lib.state.Store - -/** - * A Store for handling [AppIconState] and dispatching [AppIconAction]. - * - * @param initialState The initial state of the Store. - * @param reducer Reducer to handle state updates based on dispatched actions. - * @param middleware A list of Middleware to handle side-effects in response to dispatched actions. - */ -class AppIconStore( - initialState: AppIconState, - reducer: Reducer<AppIconState, AppIconAction> = ::appIconReducer, - middleware: List<Middleware<AppIconState, AppIconAction>> = listOf(), -) : Store<AppIconState, AppIconAction>( - initialState = initialState, - reducer = reducer, - middleware = middleware, -) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelection.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/iconpicker/ui/AppIconSelection.kt @@ -9,13 +9,10 @@ import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width @@ -27,15 +24,12 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.AlertDialog import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Scaffold -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -49,25 +43,15 @@ import androidx.compose.ui.semantics.heading import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp -import kotlinx.coroutines.launch import mozilla.components.compose.base.annotation.FlexibleWindowLightDarkPreview import mozilla.components.compose.base.button.TextButton -import mozilla.components.compose.base.snackbar.Snackbar -import mozilla.components.compose.base.snackbar.displaySnackbar -import mozilla.components.lib.state.ext.observeAsState import org.mozilla.fenix.R import org.mozilla.fenix.compose.button.RadioButton import org.mozilla.fenix.iconpicker.AppIcon -import org.mozilla.fenix.iconpicker.AppIconSnackbarState -import org.mozilla.fenix.iconpicker.AppIconState -import org.mozilla.fenix.iconpicker.AppIconStore -import org.mozilla.fenix.iconpicker.AppIconWarningDialog import org.mozilla.fenix.iconpicker.DefaultAppIconRepository import org.mozilla.fenix.iconpicker.DefaultPackageManagerWrapper import org.mozilla.fenix.iconpicker.IconBackground import org.mozilla.fenix.iconpicker.IconGroupTitle -import org.mozilla.fenix.iconpicker.SystemAction -import org.mozilla.fenix.iconpicker.UserAction import org.mozilla.fenix.theme.FirefoxTheme private val ListItemHeight = 56.dp @@ -82,86 +66,24 @@ private val GroupSpacerHeight = 8.dp /** * A composable that displays a list of app icon options. * - * @param store A store for managing the app icon selection screen state. + * @param currentAppIcon The currently selected app icon alias. + * @param groupedIconOptions Icons are displayed in sections under their respective titles. + * @param onAppIconSelected A callback invoked when the user has confirmed an alternative icon to be + * applied (they get informed about the required restart providing an opportunity to back out). */ @Composable fun AppIconSelection( - store: AppIconStore, + currentAppIcon: AppIcon, + groupedIconOptions: Map<IconGroupTitle, List<AppIcon>>, + onAppIconSelected: (AppIcon) -> Unit, ) { - val state by store.observeAsState(store.state) { it } - val selectedIcon = state.userSelectedAppIcon ?: state.currentAppIcon - val scope = rememberCoroutineScope() - val snackbarHostState = remember { SnackbarHostState() } + var currentAppIcon by remember { mutableStateOf(currentAppIcon) } + var selectedAppIcon by remember { mutableStateOf<AppIcon?>(null) } - val errorSnackbarMessage = stringResource(R.string.shortcuts_update_error) - LaunchedEffect(state.snackbarState) { - when (state.snackbarState) { - AppIconSnackbarState.None -> return@LaunchedEffect - AppIconSnackbarState.ApplyingNewIconError -> scope.launch { - snackbarHostState.displaySnackbar( - message = errorSnackbarMessage, - onDismissPerformed = { - store.dispatch(SystemAction.SnackbarDismissed) - }, - ) - } - } - } - - Scaffold( - snackbarHost = { - SnackbarHost( - hostState = snackbarHostState, - snackbar = { snackbarData -> - Snackbar(snackbarData = snackbarData) - }, - modifier = Modifier.imePadding(), - ) - }, - contentWindowInsets = WindowInsets(), // empty insets, activity toolbar is handled by the host fragment - ) { paddingValues -> - AppIconList( - paddingValues = paddingValues, - selectedIcon = selectedIcon, - groupedIcons = state.groupedIconOptions, - onIconSelected = { icon -> store.dispatch(UserAction.Selected(icon)) }, - ) - } - - when (val warning = state.warningDialogState) { - is AppIconWarningDialog.Presenting -> RestartWarningDialog( - onConfirmClicked = { - store.dispatch( - UserAction.Confirmed( - oldIcon = state.currentAppIcon, - newIcon = warning.newIcon, - ), - ) - }, - onDismissClicked = { - store.dispatch(UserAction.Dismissed) - }, - onDismissed = { - store.dispatch(SystemAction.DialogDismissed) - }, - ) - else -> Unit - } -} - -@Composable -private fun AppIconList( - paddingValues: PaddingValues, - selectedIcon: AppIcon, - groupedIcons: Map<IconGroupTitle, List<AppIcon>>, - onIconSelected: (AppIcon) -> Unit, -) { LazyColumn( - modifier = Modifier - .padding(paddingValues) - .background(color = FirefoxTheme.colors.layer1), + modifier = Modifier.background(color = FirefoxTheme.colors.layer1), ) { - groupedIcons.forEach { (header, icons) -> + groupedIconOptions.forEach { (header, icons) -> item(contentType = { header::class }) { AppIconGroupHeader(header) } @@ -170,14 +92,14 @@ private fun AppIconList( items = icons, contentType = { item -> item::class }, ) { icon -> - val iconSelected = icon == selectedIcon + val iconSelected = icon == currentAppIcon AppIconOption( appIcon = icon, selected = iconSelected, onClick = { if (!iconSelected) { - onIconSelected(icon) + selectedAppIcon = icon } }, ) @@ -190,6 +112,19 @@ private fun AppIconList( } } } + + selectedAppIcon?.let { + RestartWarningDialog( + onConfirm = { + currentAppIcon = it + onAppIconSelected(it) + selectedAppIcon = null + }, + onDismiss = { + selectedAppIcon = null + }, + ) + } } @Composable @@ -313,9 +248,8 @@ fun AppIcon( @Composable private fun RestartWarningDialog( - onConfirmClicked: () -> Unit, - onDismissClicked: () -> Unit, - onDismissed: () -> Unit, + onConfirm: () -> Unit, + onDismiss: () -> Unit, ) { AlertDialog( title = { @@ -335,17 +269,17 @@ private fun RestartWarningDialog( style = FirefoxTheme.typography.body2, ) }, - onDismissRequest = { onDismissed() }, + onDismissRequest = { onDismiss() }, confirmButton = { TextButton( text = stringResource(id = R.string.restart_warning_dialog_button_positive_2), - onClick = { onConfirmClicked() }, + onClick = { onConfirm() }, ) }, dismissButton = { TextButton( text = stringResource(id = R.string.restart_warning_dialog_button_negative), - onClick = { onDismissClicked() }, + onClick = { onDismiss() }, ) }, ) @@ -356,16 +290,12 @@ private fun RestartWarningDialog( private fun AppIconSelectionPreview() { FirefoxTheme { AppIconSelection( - store = AppIconStore( - initialState = AppIconState( - currentAppIcon = AppIcon.AppDefault, - userSelectedAppIcon = null, - groupedIconOptions = DefaultAppIconRepository( - packageManager = DefaultPackageManagerWrapper(LocalContext.current.packageManager), - packageName = LocalContext.current.packageName, - ).groupedAppIcons, - ), - ), + currentAppIcon = AppIcon.AppDefault, + groupedIconOptions = DefaultAppIconRepository( + packageManager = DefaultPackageManagerWrapper(LocalContext.current.packageManager), + packageName = LocalContext.current.packageName, + ).groupedAppIcons, + onAppIconSelected = {}, ) } } @@ -391,9 +321,8 @@ private fun AppIconOptionWithSubtitlePreview() { private fun RestartWarningDialogPreview() { FirefoxTheme { RestartWarningDialog( - onConfirmClicked = {}, - onDismissClicked = {}, - onDismissed = {}, + onConfirm = {}, + onDismiss = {}, ) } } 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 @@ -12,18 +12,12 @@ import androidx.fragment.app.Fragment import androidx.fragment.compose.content import androidx.navigation.fragment.findNavController import mozilla.components.support.base.feature.UserInteractionHandler +import org.mozilla.fenix.GleanMetrics.AppIconSelection 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 -import org.mozilla.fenix.iconpicker.AppIconState -import org.mozilla.fenix.iconpicker.AppIconStore -import org.mozilla.fenix.iconpicker.AppIconUpdater import org.mozilla.fenix.iconpicker.DefaultAppIconRepository import org.mozilla.fenix.iconpicker.DefaultPackageManagerWrapper -import org.mozilla.fenix.iconpicker.SystemAction import org.mozilla.fenix.theme.FirefoxTheme import org.mozilla.fenix.utils.ShortcutManagerWrapperDefault import org.mozilla.fenix.utils.ShortcutsUpdaterDefault @@ -48,38 +42,38 @@ class AppIconSelectionFragment : Fragment(), UserInteractionHandler { ) = content { FirefoxTheme { AppIconSelection( - store = StoreProvider.get(this) { - AppIconStore( - initialState = AppIconState( - currentAppIcon = appIconRepository.selectedAppIcon, - groupedIconOptions = appIconRepository.groupedAppIcons, - ), - middleware = listOf( - AppIconMiddleware( - updateAppIcon = updateAppIcon(), - ), + currentAppIcon = appIconRepository.selectedAppIcon, + groupedIconOptions = appIconRepository.groupedAppIcons, + onAppIconSelected = { selectedAppIcon -> + val currentAliasSuffix = appIconRepository.selectedAppIcon.aliasSuffix + + AppIconSelection.appIconSelectionConfirmed.record( + extra = AppIconSelection.AppIconSelectionConfirmedExtra( + oldIcon = currentAliasSuffix, + newIcon = selectedAppIcon.aliasSuffix, ), ) - }.also { - it.dispatch( - SystemAction.EnvironmentRehydrated( - appIconUpdater = updateAppIcon(), - ), + + updateAppIcon( + currentAliasSuffix = currentAliasSuffix, + newAliasSuffix = selectedAppIcon.aliasSuffix, ) }, ) } } - private fun updateAppIcon(): AppIconUpdater = AppIconUpdater { newIcon, currentIcon -> + private fun updateAppIcon( + currentAliasSuffix: String, + newAliasSuffix: String, + ) { with(requireContext()) { changeAppLauncherIcon( packageManager = packageManager, shortcutManager = ShortcutManagerWrapperDefault(this), shortcutInfo = ShortcutsUpdaterDefault(this), - appAlias = ComponentName(this, "$packageName.${currentIcon.aliasSuffix}"), - newAppAlias = ComponentName(this, "$packageName.${newIcon.aliasSuffix}"), - crashReporter = components.analytics.crashReporter, + appAlias = ComponentName(this, "$packageName.$currentAliasSuffix"), + newAppAlias = ComponentName(this, "$packageName.$newAliasSuffix"), ) } } 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,7 +10,6 @@ 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 @@ -28,8 +27,6 @@ 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, @@ -38,25 +35,17 @@ 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, crashReporter) + resetAppIconsToDefault(context, shortcutManager, shortcutInfo, appAlias, alternativeAppAlias) return } if (!resetToDefault && !userHasAlternativeAppIconSet) { - changeAppLauncherIcon( - context.packageManager, - shortcutManager, - shortcutInfo, - appAlias, - alternativeAppAlias, - crashReporter, - ) + changeAppLauncherIcon(context.packageManager, shortcutManager, shortcutInfo, appAlias, alternativeAppAlias) } } @@ -83,11 +72,7 @@ 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` */ fun changeAppLauncherIcon( packageManager: PackageManager, @@ -95,14 +80,12 @@ fun changeAppLauncherIcon( shortcutInfo: ShortcutsUpdater, appAlias: ComponentName, newAppAlias: ComponentName, - crashReporter: CrashReporting, - updateShortcuts: (ShortcutManagerWrapper, ShortcutsUpdater, ComponentName, CrashReporting) -> Boolean = + updateShortcuts: (ShortcutManagerWrapper, ShortcutsUpdater, ComponentName) -> Boolean = ::updateShortcutsComponentName, -): Boolean { +) { newAppAlias.setEnabledStateTo(packageManager, true) - val updated = updateShortcuts(shortcutManager, shortcutInfo, newAppAlias, crashReporter) - if (updated) { + if (updateShortcuts(shortcutManager, shortcutInfo, newAppAlias)) { logger.info("Successfully attempted to update the app icon to the alternative.") appAlias.setEnabledStateTo(packageManager, false) } else { @@ -110,7 +93,6 @@ fun changeAppLauncherIcon( // If we can't successfully update the user shortcuts, then re-disable the app alias component. newAppAlias.setEnabledStateTo(packageManager, false) } - return updated } private fun resetAppIconsToDefault( @@ -119,12 +101,11 @@ private fun resetAppIconsToDefault( shortcutInfo: ShortcutsUpdater, appAlias: ComponentName, alternativeAppAlias: ComponentName, - crashReporter: CrashReporting, ) { val packageManager = context.packageManager appAlias.setEnabledStateToDefault(packageManager) - if (updateShortcutsComponentName(shortcutManager, shortcutInfo, appAlias, crashReporter)) { + if (updateShortcutsComponentName(shortcutManager, shortcutInfo, appAlias)) { logger.info("Successfully attempted to reset the app icon to default.") alternativeAppAlias.setEnabledStateToDefault(packageManager) } else { @@ -166,12 +147,10 @@ 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 } @@ -182,7 +161,6 @@ 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/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -971,8 +971,6 @@ <string name="restart_warning_dialog_button_positive_2">Change icon</string> <!-- Dialog button text for cancelling app icon change--> <string name="restart_warning_dialog_button_negative">Cancel</string> - <!-- Error message snackbar shown after the user tried to apply the new icon but the Android OS threw an error while updating existing app shortcuts (e.g. private mode shortcut) --> - <string name="shortcuts_update_error">Failed to update the app icon. Remove existing shortcuts and try again.</string> <!-- Add-ons general availability nimbus message--> <!-- Title of the Nimbus message for extension general availability--> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconMiddlewareTest.kt @@ -1,110 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import androidx.test.ext.junit.runners.AndroidJUnit4 -import mozilla.components.support.test.ext.joinBlocking -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Test -import org.junit.runner.RunWith - -@RunWith(AndroidJUnit4::class) -class AppIconMiddlewareTest { - - @Test - fun `WHEN the store receives UserAction Confirmed THEN the middleware calls the updateAppIcon interface with the correct data from the action`() { - val currentIcon = AppIcon.AppDefault - val newIcon = AppIcon.AppRetro2004 - var updatedCurrentIcon: AppIcon? = null - var updatedNewIcon: AppIcon? = null - val middleware = AppIconMiddleware { newIcon, currentIcon -> - updatedNewIcon = newIcon - updatedCurrentIcon = currentIcon - true - } - val store = AppIconStore( - initialState = AppIconState( - currentAppIcon = currentIcon, - userSelectedAppIcon = null, - groupedIconOptions = mapOf(), - ), - middleware = listOf(middleware), - ) - - store.dispatch(UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon)).joinBlocking() - - assertEquals(currentIcon, updatedCurrentIcon) - assertEquals(newIcon, updatedNewIcon) - } - - @Test - fun `WHEN updateAppIcon call is successful THEN the middleware dispatches the Applied system action to the store`() { - val currentIcon = AppIcon.AppDefault - val newIcon = AppIcon.AppRetro2004 - val middleware = AppIconMiddleware { newIcon, currentIcon -> true } - val result = mutableListOf<AppIconAction>() - val store = AppIconStore( - initialState = AppIconState( - currentAppIcon = currentIcon, - ), - reducer = { state, action -> - result.add(action) - state - }, - middleware = listOf(middleware), - ) - - assertTrue(result.isEmpty()) - - val confirmAction = UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon) - store.dispatch(confirmAction).joinBlocking() - - assertEquals(listOf(confirmAction, SystemAction.Applied(newIcon)), result) - } - - @Test - fun `WHEN updateAppIcon call returns with an a failure THEN the middleware dispatches the UpdateFailed system action to the store`() { - val currentIcon = AppIcon.AppDefault - val newIcon = AppIcon.AppRetro2004 - val middleware = AppIconMiddleware { newIcon, currentIcon -> false } - val result = mutableListOf<AppIconAction>() - val store = AppIconStore( - initialState = AppIconState( - currentAppIcon = currentIcon, - ), - reducer = { state, action -> - result.add(action) - state - }, - middleware = listOf(middleware), - ) - - assertTrue(result.isEmpty()) - - val confirmAction = UserAction.Confirmed(newIcon = newIcon, oldIcon = currentIcon) - store.dispatch(confirmAction).joinBlocking() - - assertEquals(listOf(confirmAction, SystemAction.UpdateFailed), result) - } - - @Test - fun `GIVEN EnvironmentRehydrated system action WHEN middleware is called THEN the new app icon updater replaces the old one`() { - val initialUpdater = AppIconUpdater { _, _ -> false } - val middleware = AppIconMiddleware(initialUpdater) - val store = AppIconStore( - initialState = AppIconState(), - reducer = { state, action -> - state - }, - middleware = listOf(middleware), - ) - val newUpdater = AppIconUpdater { _, _ -> false } - - store.dispatch(SystemAction.EnvironmentRehydrated(newUpdater)).joinBlocking() - - assertEquals(newUpdater, middleware.updateAppIcon) - } -} diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconReducerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/iconpicker/AppIconReducerTest.kt @@ -1,135 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.iconpicker - -import org.junit.Assert.assertEquals -import org.junit.Test - -class AppIconReducerTest { - @Test - fun `GIVEN Selected user action WHEN reducer is called THEN state is updated with the selected icon and the warning is displayed`() { - val initialState = AppIconState() - val newIcon = AppIcon.AppRetro2004 - - assertEquals(null, initialState.userSelectedAppIcon) - - val result = appIconReducer(initialState, UserAction.Selected(newIcon)) - - assert(result.warningDialogState is AppIconWarningDialog.Presenting) - assertEquals(newIcon, result.userSelectedAppIcon) - } - - @Test - fun `GIVEN Selected user action WHEN reducer is called THEN the current icon is not changed`() { - val initialState = AppIconState() - val newIcon = AppIcon.AppRetro2004 - - assertEquals(AppIcon.AppDefault, initialState.currentAppIcon) - - val result = appIconReducer(initialState, UserAction.Selected(newIcon)) - - assertEquals(AppIcon.AppDefault, result.currentAppIcon) - } - - @Test - fun `GIVEN Dismissed user action WHEN reducer is called THEN state resets the warning state to none and the user selected icon to null`() { - val initialState = AppIconState( - userSelectedAppIcon = AppIcon.AppRetro2004, - warningDialogState = AppIconWarningDialog.Presenting(AppIcon.AppRetro2004), - ) - - assertEquals(AppIcon.AppRetro2004, initialState.userSelectedAppIcon) - - val result = appIconReducer(initialState, UserAction.Dismissed) - - assertEquals(AppIconWarningDialog.None, result.warningDialogState) - assertEquals(null, result.userSelectedAppIcon) - } - - @Test - fun `GIVEN Dismissed user action WHEN reducer is called THEN the current icon is not changed`() { - val initialState = AppIconState() - - assertEquals(AppIcon.AppDefault, initialState.currentAppIcon) - - val result = appIconReducer(initialState, UserAction.Dismissed) - - assertEquals(AppIcon.AppDefault, result.currentAppIcon) - } - - @Test - fun `GIVEN DialogDismissed system action WHEN reducer is called THEN the warning dialog is hidden and the user selected icon is reset to null`() { - val initialState = AppIconState( - userSelectedAppIcon = AppIcon.AppRetro2004, - warningDialogState = AppIconWarningDialog.Presenting(AppIcon.AppRetro2004), - ) - - assertEquals(AppIcon.AppRetro2004, initialState.userSelectedAppIcon) - - val result = appIconReducer(initialState, SystemAction.DialogDismissed) - - assertEquals(AppIconWarningDialog.None, result.warningDialogState) - assertEquals(null, result.userSelectedAppIcon) - } - - @Test - fun `GIVEN Applied system action WHEN reducer is called THEN the new icon becomes the new current and the user selected icon resets to null and the dialog is hidden`() { - val newIcon = AppIcon.AppRetro2004 - val currentIcon = AppIcon.AppDefault - val dialogState = AppIconWarningDialog.Presenting(newIcon) - val initialState = AppIconState( - currentAppIcon = currentIcon, - userSelectedAppIcon = newIcon, - warningDialogState = dialogState, - ) - - assertEquals(currentIcon, initialState.currentAppIcon) - assertEquals(newIcon, initialState.userSelectedAppIcon) - assertEquals(dialogState, initialState.warningDialogState) - - val result = appIconReducer(initialState, SystemAction.Applied(newIcon = newIcon)) - - assertEquals(newIcon, result.currentAppIcon) - assertEquals(null, result.userSelectedAppIcon) - assertEquals(AppIconWarningDialog.None, result.warningDialogState) - } - - @Test - fun `GIVEN UpdateFailed system action WHEN reducer is called THEN the user selected icon resets to null and the dialog is hidden and the snackbar is shown`() { - val newIcon = AppIcon.AppRetro2004 - val currentIcon = AppIcon.AppDefault - val dialogState = AppIconWarningDialog.Presenting(newIcon) - val initialState = AppIconState( - currentAppIcon = currentIcon, - userSelectedAppIcon = newIcon, - warningDialogState = dialogState, - ) - - assertEquals(newIcon, initialState.userSelectedAppIcon) - assertEquals(dialogState, initialState.warningDialogState) - assertEquals(AppIconSnackbarState.None, initialState.snackbarState) - - val result = appIconReducer(initialState, SystemAction.UpdateFailed) - - assertEquals(null, result.userSelectedAppIcon) - assertEquals(AppIconWarningDialog.None, result.warningDialogState) - assertEquals(AppIconSnackbarState.ApplyingNewIconError, result.snackbarState) - } - - @Test - fun `GIVEN SnackbarDismissed system action WHEN reducer is called THEN the snackbar is hidden`() { - val currentIcon = AppIcon.AppDefault - val initialState = AppIconState( - currentAppIcon = currentIcon, - snackbarState = AppIconSnackbarState.ApplyingNewIconError, - ) - - assertEquals(AppIconSnackbarState.ApplyingNewIconError, initialState.snackbarState) - - val result = appIconReducer(initialState, SystemAction.SnackbarDismissed) - - assertEquals(AppIconSnackbarState.None, result.snackbarState) - } -} 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,11 +9,6 @@ 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 @@ -37,12 +32,10 @@ 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) } @@ -69,7 +62,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -112,7 +104,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -154,7 +145,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -193,7 +183,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, true, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -237,7 +226,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -280,7 +268,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -318,7 +305,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -357,7 +343,6 @@ class ChangeAppLauncherIconTest { appAlias, alternativeAppAlias, false, - fakeCrashReporter, ) val appAliasState = packageManager.getComponentEnabledSetting(appAlias) @@ -399,8 +384,7 @@ class ChangeAppLauncherIconTest { shortcutInfo = shortcutsUpdater, appAlias = appAlias, newAppAlias = newAppAlias, - crashReporter = fakeCrashReporter, - updateShortcuts = { _, _, _, _ -> false }, + updateShortcuts = { _, _, _ -> false }, ) assertEquals(PackageManager.COMPONENT_ENABLED_STATE_ENABLED, packageManager.getComponentEnabledSetting(appAlias)) @@ -433,8 +417,7 @@ class ChangeAppLauncherIconTest { shortcutInfo = shortcutsUpdater, appAlias = appAlias, newAppAlias = newAppAlias, - crashReporter = fakeCrashReporter, - updateShortcuts = { _, _, _, _ -> true }, + updateShortcuts = { _, _, _ -> true }, ) assertEquals(PackageManager.COMPONENT_ENABLED_STATE_DISABLED, packageManager.getComponentEnabledSetting(appAlias)) @@ -460,7 +443,6 @@ class ChangeAppLauncherIconTest { shortcutManager = throwingWrapper, shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), - crashReporter = fakeCrashReporter, ), ) } @@ -483,7 +465,6 @@ class ChangeAppLauncherIconTest { shortcutManager = throwingWrapper, shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), - crashReporter = fakeCrashReporter, ), ) } @@ -495,7 +476,6 @@ class ChangeAppLauncherIconTest { shortcutManager = TestShortcutManagerWrapper(testContext), shortcutInfo = shortcutsUpdater, targetAlias = ComponentName("test", "AppAlternative"), - crashReporter = fakeCrashReporter, ), ) } @@ -515,78 +495,6 @@ 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) = @@ -612,16 +520,3 @@ 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) {} -}