From 7162cb1ca0b1737bf9008fdb260aef593f2f2e06 Mon Sep 17 00:00:00 2001 From: Zac McKenney Date: Thu, 31 Aug 2023 17:04:20 -0700 Subject: [PATCH] Bug 1846979 - Add extensions process spawning disabled dialog (cherry picked from commit 2c533231b6886b1359065b7adac11fa1139aa4dc) --- app/metrics.yaml | 51 +++++++++ .../java/org/mozilla/fenix/HomeActivity.kt | 7 +- .../ExtensionProcessDisabledController.kt | 78 +++++++++++++ .../ExtensionProcessDisabledControllerTest.kt | 104 ++++++++++++++++++ 4 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 app/src/main/java/org/mozilla/fenix/addons/ExtensionProcessDisabledController.kt create mode 100644 app/src/test/java/org/mozilla/fenix/addons/ExtensionProcessDisabledControllerTest.kt diff --git a/app/metrics.yaml b/app/metrics.yaml index 999448822..d68c093a7 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -7754,6 +7754,57 @@ addons: metadata: tags: - WebExtensions + extensions_process_ui_retry: + type: counter + lifetime: application + description: | + A counter that indicates the number of times that a user + has clicked on the button try to restart add-ons + on the dialog for when the extensions process crashed. + send_in_pings: + - metrics + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1850350 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1850350#c2 + - https://github.com/mozilla-mobile/firefox-android/pull/3472 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + - wdurand@mozilla.com + - amejiamarmol@mozilla.com + - addons-dev-internal@mozilla.com + expires: never + metadata: + tags: + - WebExtensions + extensions_process_ui_disable: + type: counter + lifetime: application + description: | + A counter that indicates the number of times that a user + has clicked on the button continue with add-ons + disabled on the dialog for when the extensions + process crashed. + send_in_pings: + - metrics + bugs: + - https://github.com/mozilla-mobile/fenix/issues/19931 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1850350#c2 + - https://github.com/mozilla-mobile/firefox-android/pull/3472 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + - wdurand@mozilla.com + - amejiamarmol@mozilla.com + - addons-dev-internal@mozilla.com + expires: never + metadata: + tags: + - WebExtensions perf.startup: cold_main_app_to_first_frame: type: timing_distribution diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index fe33a904b..d01f398cc 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -91,6 +91,7 @@ import org.mozilla.fenix.GleanMetrics.SplashScreen import org.mozilla.fenix.GleanMetrics.StartOnHome import org.mozilla.fenix.addons.AddonDetailsFragmentDirections import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections +import org.mozilla.fenix.addons.ExtensionProcessDisabledController import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager @@ -192,6 +193,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { WebExtensionPopupFeature(components.core.store, ::openPopup) } + private val extensionProcessDisabledPopupFeature by lazy { + ExtensionProcessDisabledController(this@HomeActivity, components.core.store) + } + private val serviceWorkerSupport by lazy { ServiceWorkerSupportFeature(this) } @@ -340,7 +345,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } supportActionBar?.hide() - lifecycle.addObservers(webExtensionPopupFeature, serviceWorkerSupport) + lifecycle.addObservers(webExtensionPopupFeature, extensionProcessDisabledPopupFeature, serviceWorkerSupport) if (shouldAddToRecentsScreen(intent)) { intent.removeExtra(START_IN_RECENTS_SCREEN) diff --git a/app/src/main/java/org/mozilla/fenix/addons/ExtensionProcessDisabledController.kt b/app/src/main/java/org/mozilla/fenix/addons/ExtensionProcessDisabledController.kt new file mode 100644 index 000000000..b2b5c6a2c --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/addons/ExtensionProcessDisabledController.kt @@ -0,0 +1,78 @@ +/* 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.addons + +import android.app.AlertDialog +import android.content.Context +import androidx.annotation.UiContext +import mozilla.components.browser.state.action.ExtensionProcessDisabledPopupAction +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.engine.Engine +import mozilla.components.support.ktx.android.content.appName +import mozilla.components.support.webextensions.ExtensionProcessDisabledPopupFeature +import org.mozilla.fenix.GleanMetrics.Addons +import org.mozilla.fenix.R +import org.mozilla.fenix.ext.components +import org.mozilla.geckoview.WebExtensionController + +/** + * Present a dialog to the user notifying of extension process spawning disabled and also asking + * whether they would like to continue trying or disable extensions. If the user chooses to retry, + * enable the extension process spawning with [WebExtensionController.enableExtensionProcessSpawning]. + * + * @param context to show the AlertDialog + * @param store The [BrowserStore] which holds the state for showing the dialog + * @param webExtensionController to call when a user enables the process spawning + * @param builder to use for creating the dialog which can be styled as needed + * @param appName to be added to the message. Necessary to be added as a param for testing + */ +private fun presentDialog( + @UiContext context: Context, + store: BrowserStore, + engine: Engine, + builder: AlertDialog.Builder, + appName: String, +) { + val message = context.getString(R.string.addon_process_crash_dialog_message, appName) + + builder.apply { + setCancelable(false) + setTitle(R.string.addon_process_crash_dialog_title) + setMessage(message) + setPositiveButton(R.string.addon_process_crash_dialog_retry_button_text) { dialog, _ -> + engine.enableExtensionProcessSpawning() + Addons.extensionsProcessUiRetry.add() + store.dispatch(ExtensionProcessDisabledPopupAction(false)) + dialog.dismiss() + } + setNegativeButton(R.string.addon_process_crash_dialog_disable_addons_button_text) { dialog, _ -> + Addons.extensionsProcessUiDisable.add() + store.dispatch(ExtensionProcessDisabledPopupAction(false)) + dialog.dismiss() + } + } + + builder.show() +} + +/** + * Controller for showing the user a dialog when the the extension process spawning has been disabled. + * + * @param context to show the AlertDialog + * @param store The [BrowserStore] which holds the state for showing the dialog + * @param webExtensionController to call when a user enables the process spawning + * @param builder to use for creating the dialog which can be styled as needed + * @param appName to be added to the message. Optional and mainly relevant for testing + */ +class ExtensionProcessDisabledController( + @UiContext context: Context, + store: BrowserStore, + engine: Engine = context.components.core.engine, + builder: AlertDialog.Builder = AlertDialog.Builder(context, R.style.DialogStyleNormal), + appName: String = context.appName, +) : ExtensionProcessDisabledPopupFeature( + store, + { presentDialog(context, store, engine, builder, appName) }, +) diff --git a/app/src/test/java/org/mozilla/fenix/addons/ExtensionProcessDisabledControllerTest.kt b/app/src/test/java/org/mozilla/fenix/addons/ExtensionProcessDisabledControllerTest.kt new file mode 100644 index 000000000..fd85f5a54 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/addons/ExtensionProcessDisabledControllerTest.kt @@ -0,0 +1,104 @@ +/* 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.addons + +import android.app.AlertDialog +import android.content.Context +import android.content.DialogInterface.OnClickListener +import mozilla.components.browser.state.action.ExtensionProcessDisabledPopupAction +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.engine.Engine +import mozilla.components.support.test.argumentCaptor +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.whenever +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.anyInt +import org.mockito.Mockito.anyString +import org.mockito.Mockito.mock +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class ExtensionProcessDisabledControllerTest { + + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + private val dispatcher = coroutinesTestRule.testDispatcher + + @Test + fun `WHEN showExtensionProcessDisabledPopup is true AND positive button clicked then enable extension process spawning`() { + val context: Context = mock() + val store = BrowserStore() + val engine: Engine = mock() + val appName = "TestApp" + val builder: AlertDialog.Builder = mock() + val controller = ExtensionProcessDisabledController(context, store, engine, builder, appName) + controller.start() + + whenever(context.getString(anyInt(), anyString())).thenReturn("TestString") + + val posClickCaptor = argumentCaptor() + + assertFalse(store.state.showExtensionProcessDisabledPopup) + + store.dispatch(ExtensionProcessDisabledPopupAction(showPopup = true)) + dispatcher.scheduler.advanceUntilIdle() + store.waitUntilIdle() + assertTrue(store.state.showExtensionProcessDisabledPopup) + + verify(builder).setPositiveButton(anyInt(), posClickCaptor.capture()) + + verify(builder).show() + + val dialog: AlertDialog = mock() + posClickCaptor.value.onClick(dialog, 1) + store.waitUntilIdle() + + verify(engine).enableExtensionProcessSpawning() + assertFalse(store.state.showExtensionProcessDisabledPopup) + verify(dialog).dismiss() + } + + @Test + fun `WHEN showExtensionProcessDisabledPopup is true AND negative button clicked then dismiss without enabling extension process spawning`() { + val context: Context = mock() + val store = BrowserStore() + val engine: Engine = mock() + val appName = "TestApp" + val builder: AlertDialog.Builder = mock() + val controller = ExtensionProcessDisabledController(context, store, engine, builder, appName) + controller.start() + + whenever(context.getString(anyInt(), anyString())).thenReturn("TestString") + + val negClickCaptor = argumentCaptor() + + assertFalse(store.state.showExtensionProcessDisabledPopup) + + store.dispatch(ExtensionProcessDisabledPopupAction(showPopup = true)) + dispatcher.scheduler.advanceUntilIdle() + store.waitUntilIdle() + assertTrue(store.state.showExtensionProcessDisabledPopup) + + verify(builder).setNegativeButton(anyInt(), negClickCaptor.capture()) + + verify(builder).show() + + val dialog: AlertDialog = mock() + negClickCaptor.value.onClick(dialog, 1) + store.waitUntilIdle() + + assertFalse(store.state.showExtensionProcessDisabledPopup) + verify(engine, never()).enableExtensionProcessSpawning() + verify(dialog).dismiss() + } +}