From 728ad27a0ae902e301f4386517b30a24e069c64e Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 14:46:47 -0400 Subject: [PATCH 1/6] Use applicationScope instead of GlobalScope Bug 1990613 introduced an application-provided CoroutineScope in android-components, and BrowserApplication gained an applicationScope to satisfy it. Switch the remaining GlobalScope.launch call sites over to it, which also drops the @OptIn(DelicateCoroutinesApi::class) annotations they required: - BrowserApplication, two sites. The explicit Dispatchers.Main in restoreBrowserState is redundant because applicationScope already carries it. - CrashIntegration.sendCrashReport, which reaches the scope through context.components. Crash reports should outlive the observer's lifecycle, so the application scope is the appropriate one. BrowserFragment.deleteHistorySuggestion carried the same @OptIn but uses lifecycleScope, so that annotation was already dead and is simply removed. Removing the @OptIn changes sendCrashReport's detekt baseline ID, so its UndocumentedPublicFunction entry is updated to the new signature. --- .../reference/browser/BrowserApplication.kt | 35 ++++++++----------- .../browser/browser/BrowserFragment.kt | 2 -- .../browser/browser/CrashIntegration.kt | 6 ++-- config/detekt-baseline.xml | 2 +- 4 files changed, 18 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt b/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt index 859a92b94..900084d80 100644 --- a/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt +++ b/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt @@ -8,9 +8,7 @@ import android.app.Application import java.util.concurrent.TimeUnit import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.launch import mozilla.components.browser.state.action.SystemAction @@ -120,8 +118,7 @@ open class BrowserApplication : Application() { // Initialize the push feature and service. it.initialize() } - @OptIn(DelicateCoroutinesApi::class) - GlobalScope.launch(Dispatchers.IO) { + applicationScope.launch(Dispatchers.IO) { components.core.fileUploadsDirCleaner.cleanUploadsDirectory() } } @@ -134,22 +131,20 @@ open class BrowserApplication : Application() { } } - @OptIn(DelicateCoroutinesApi::class) - private fun restoreBrowserState() = - GlobalScope.launch(Dispatchers.Main) { - val store = components.core.store - val sessionStorage = components.core.sessionStorage - - components.useCases.tabsUseCases.restore(sessionStorage) - - // Now that we have restored our previous state (if there's one) let's setup auto saving the state while - // the app is used. - sessionStorage - .autoSave(store) - .periodicallyInForeground(interval = 30, unit = TimeUnit.SECONDS) - .whenGoingToBackground() - .whenSessionsChange() - } + private fun restoreBrowserState() = applicationScope.launch { + val store = components.core.store + val sessionStorage = components.core.sessionStorage + + components.useCases.tabsUseCases.restore(sessionStorage) + + // Now that we have restored our previous state (if there's one) let's setup auto saving the state while + // the app is used. + sessionStorage + .autoSave(store) + .periodicallyInForeground(interval = 30, unit = TimeUnit.SECONDS) + .whenGoingToBackground() + .whenSessionsChange() + } companion object { const val NON_FATAL_CRASH_BROADCAST = "org.mozilla.reference.browser" diff --git a/app/src/main/java/org/mozilla/reference/browser/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/reference/browser/browser/BrowserFragment.kt index 7ae99b750..cef793f41 100644 --- a/app/src/main/java/org/mozilla/reference/browser/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/reference/browser/browser/BrowserFragment.kt @@ -9,7 +9,6 @@ import android.view.View import androidx.lifecycle.lifecycleScope import androidx.preference.PreferenceManager import com.google.android.material.floatingactionbutton.FloatingActionButton -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.browser.thumbnails.BrowserThumbnails @@ -168,7 +167,6 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { } } - @OptIn(DelicateCoroutinesApi::class) private fun deleteHistorySuggestion(suggestion: Suggestion) { lifecycleScope.launch(Dispatchers.IO) { suggestion.description?.let { diff --git a/app/src/main/java/org/mozilla/reference/browser/browser/CrashIntegration.kt b/app/src/main/java/org/mozilla/reference/browser/browser/CrashIntegration.kt index 58797cb96..12f99a487 100644 --- a/app/src/main/java/org/mozilla/reference/browser/browser/CrashIntegration.kt +++ b/app/src/main/java/org/mozilla/reference/browser/browser/CrashIntegration.kt @@ -11,13 +11,12 @@ import android.content.IntentFilter import androidx.core.content.ContextCompat import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner -import kotlinx.coroutines.DelicateCoroutinesApi -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import mozilla.components.lib.crash.Crash import mozilla.components.lib.crash.CrashReporter import mozilla.components.support.utils.ext.registerReceiverCompat import org.mozilla.reference.browser.BrowserApplication.Companion.NON_FATAL_CRASH_BROADCAST +import org.mozilla.reference.browser.ext.components class CrashIntegration( private val context: Context, @@ -53,9 +52,8 @@ class CrashIntegration( context.unregisterReceiver(receiver) } - @OptIn(DelicateCoroutinesApi::class) fun sendCrashReport(crash: Crash) { - GlobalScope.launch { + context.components.applicationScope.launch { crashReporter.submitReport(crash) } } diff --git a/config/detekt-baseline.xml b/config/detekt-baseline.xml index 37ffd6643..9ec92b0c8 100644 --- a/config/detekt-baseline.xml +++ b/config/detekt-baseline.xml @@ -61,7 +61,7 @@ UndocumentedPublicFunction:Config.kt$Config$@JvmStatic fun generateDebugVersionName(): String UndocumentedPublicFunction:Config.kt$Config$@JvmStatic fun releaseVersionName(project: Project): String UndocumentedPublicFunction:Context.kt$fun Context.getPreferenceKey(@StringRes resourceId: Int): String - UndocumentedPublicFunction:CrashIntegration.kt$CrashIntegration$@OptIn(DelicateCoroutinesApi::class) fun sendCrashReport(crash: Crash) + UndocumentedPublicFunction:CrashIntegration.kt$CrashIntegration$fun sendCrashReport(crash: Crash) UndocumentedPublicFunction:EngineProvider.kt$EngineProvider$@Synchronized fun getOrCreateRuntime(context: Context): GeckoRuntime UndocumentedPublicFunction:EngineProvider.kt$EngineProvider$fun createClient(context: Context): Client UndocumentedPublicFunction:EngineProvider.kt$EngineProvider$fun createEngine( context: Context, defaultSettings: DefaultSettings, ): Engine From 3165c5fbc843a902430918a0655b72884a7326b2 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 14:55:59 -0400 Subject: [PATCH 2/6] Use applicationScope for application-lived work These call sites created a throwaway CoroutineScope or MainScope, launched into it and dropped the reference, leaving the job orphaned with no way to cancel it. That is the same defect as the GlobalScope usage, just spelled differently. All of the work involved is push and account handling that has to survive UI teardown, so the application scope is the correct one rather than any lifecycle scope. Services and both push integrations get it threaded in; BackgroundServices already had it as a constructor parameter. --- .../reference/browser/BrowserApplication.kt | 4 ++-- .../org/mozilla/reference/browser/Components.kt | 4 +++- .../browser/components/BackgroundServices.kt | 3 +-- .../reference/browser/components/Services.kt | 5 +++-- .../reference/browser/push/PushFxaIntegration.kt | 14 ++++++++------ .../browser/push/WebPushEngineIntegration.kt | 6 +++--- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt b/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt index 900084d80..93e3d025a 100644 --- a/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt +++ b/app/src/main/java/org/mozilla/reference/browser/BrowserApplication.kt @@ -110,10 +110,10 @@ open class BrowserApplication : Application() { PushProcessor.install(it) // WebPush integration to observe and deliver push messages to engine. - WebPushEngineIntegration(components.core.engine, it).start() + WebPushEngineIntegration(components.core.engine, it, applicationScope).start() // Perform a one-time initialization of the account manager if a message is received. - PushFxaIntegration(it, lazy { components.backgroundServices.accountManager }).launch() + PushFxaIntegration(it, lazy { components.backgroundServices.accountManager }, applicationScope).launch() // Initialize the push feature and service. it.initialize() diff --git a/app/src/main/java/org/mozilla/reference/browser/Components.kt b/app/src/main/java/org/mozilla/reference/browser/Components.kt index 71a523989..1d0132ef7 100644 --- a/app/src/main/java/org/mozilla/reference/browser/Components.kt +++ b/app/src/main/java/org/mozilla/reference/browser/Components.kt @@ -64,7 +64,9 @@ class Components( applicationScope, ) } - val services by lazy { Services(context, backgroundServices.accountManager, useCases.tabsUseCases) } + val services by lazy { + Services(context, backgroundServices.accountManager, useCases.tabsUseCases, applicationScope) + } val push by lazy { Push(context, analytics.crashReporter) } @delegate:SuppressLint("NewApi") diff --git a/app/src/main/java/org/mozilla/reference/browser/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/reference/browser/components/BackgroundServices.kt index 146e44141..88ec088cb 100644 --- a/app/src/main/java/org/mozilla/reference/browser/components/BackgroundServices.kt +++ b/app/src/main/java/org/mozilla/reference/browser/components/BackgroundServices.kt @@ -8,7 +8,6 @@ import android.content.Context import android.os.Build import java.util.concurrent.TimeUnit import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.appservices.fxaclient.FxaServer import mozilla.components.browser.storage.sync.PlacesHistoryStorage @@ -96,7 +95,7 @@ class BackgroundServices( SyncedTabsIntegration(context, accountManager).launch() - CoroutineScope(Dispatchers.Main).launch { accountManager.start() } + applicationScope.launch { accountManager.start() } } } diff --git a/app/src/main/java/org/mozilla/reference/browser/components/Services.kt b/app/src/main/java/org/mozilla/reference/browser/components/Services.kt index 6cb641d02..92fa9f964 100644 --- a/app/src/main/java/org/mozilla/reference/browser/components/Services.kt +++ b/app/src/main/java/org/mozilla/reference/browser/components/Services.kt @@ -6,7 +6,7 @@ package org.mozilla.reference.browser.components import android.content.Context import androidx.preference.PreferenceManager -import kotlinx.coroutines.MainScope +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature import mozilla.components.feature.app.links.AppLinksInterceptor @@ -20,6 +20,7 @@ class Services( private val context: Context, private val accountManager: FxaAccountManager, private val tabsUseCases: TabsUseCases, + private val applicationScope: CoroutineScope, ) { private val prefs = PreferenceManager.getDefaultSharedPreferences(context) val accountsAuthFeature by lazy { @@ -27,7 +28,7 @@ class Services( accountManager, redirectUrl = BackgroundServices.REDIRECT_URL, ) { _, authUrl -> - MainScope().launch { + applicationScope.launch { tabsUseCases.addTab.invoke(authUrl) } } diff --git a/app/src/main/java/org/mozilla/reference/browser/push/PushFxaIntegration.kt b/app/src/main/java/org/mozilla/reference/browser/push/PushFxaIntegration.kt index b0acf8040..6fb68ec79 100644 --- a/app/src/main/java/org/mozilla/reference/browser/push/PushFxaIntegration.kt +++ b/app/src/main/java/org/mozilla/reference/browser/push/PushFxaIntegration.kt @@ -5,8 +5,6 @@ package org.mozilla.reference.browser.push import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.MainScope import kotlinx.coroutines.launch import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.AuthType @@ -52,11 +50,13 @@ import org.mozilla.reference.browser.components.Push class PushFxaIntegration( private val pushFeature: AutoPushFeature, lazyAccountManager: Lazy, + applicationScope: CoroutineScope, ) { private val observer = OneTimePushMessageObserver( lazyAccountManager, pushFeature, + applicationScope, ) /** @@ -73,6 +73,7 @@ class PushFxaIntegration( internal class OneTimePushMessageObserver( private val lazyAccountManager: Lazy, private val pushFeature: AutoPushFeature, + private val applicationScope: CoroutineScope, ) : AutoPushFeature.Observer { override fun onMessageReceived( scope: PushScope, @@ -85,8 +86,8 @@ internal class OneTimePushMessageObserver( if (scope.contains(FxaPushSupportFeature.PUSH_SCOPE_PREFIX)) { // If we aren't initialized, then we should do the initialization and message delivery. if (!lazyAccountManager.isInitialized()) { - CoroutineScope(Dispatchers.Main).launch { - val fxaObserver = OneTimeMessageDeliveryObserver(lazyAccountManager, rawBytes) + applicationScope.launch { + val fxaObserver = OneTimeMessageDeliveryObserver(lazyAccountManager, rawBytes, applicationScope) // Start observing the account manager, so that we can deliver our message // only when we are authenticated and are capable of processing it. @@ -94,7 +95,7 @@ internal class OneTimePushMessageObserver( } } - MainScope().launch { + applicationScope.launch { // Remove ourselves when we're done. pushFeature.unregister(this@OneTimePushMessageObserver) } @@ -109,12 +110,13 @@ internal class OneTimePushMessageObserver( internal class OneTimeMessageDeliveryObserver( private val lazyAccount: Lazy, private val message: ByteArray, + private val applicationScope: CoroutineScope, ) : AccountObserver { override fun onAuthenticated( account: OAuthAccount, authType: AuthType, ) { - MainScope().launch { + applicationScope.launch { lazyAccount.value.withConstellationIfExists { processRawEvent(String(message)) } diff --git a/app/src/main/java/org/mozilla/reference/browser/push/WebPushEngineIntegration.kt b/app/src/main/java/org/mozilla/reference/browser/push/WebPushEngineIntegration.kt index d0b5d0634..6d8d68b2b 100644 --- a/app/src/main/java/org/mozilla/reference/browser/push/WebPushEngineIntegration.kt +++ b/app/src/main/java/org/mozilla/reference/browser/push/WebPushEngineIntegration.kt @@ -6,7 +6,6 @@ package org.mozilla.reference.browser.push import android.util.Base64 import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.webpush.WebPushDelegate @@ -20,6 +19,7 @@ import mozilla.components.support.base.log.logger.Logger class WebPushEngineIntegration( private val engine: Engine, private val pushFeature: AutoPushFeature, + private val applicationScope: CoroutineScope, ) : AutoPushFeature.Observer { private var handler: WebPushHandler? = null private val delegate = WebPushEngineDelegate(pushFeature) @@ -38,13 +38,13 @@ class WebPushEngineIntegration( scope: PushScope, message: ByteArray?, ) { - CoroutineScope(Dispatchers.Main).launch { + applicationScope.launch { handler?.onPushMessage(scope, message) } } override fun onSubscriptionChanged(scope: PushScope) { - CoroutineScope(Dispatchers.Main).launch { + applicationScope.launch { handler?.onSubscriptionChanged(scope) } } From b5151cb67cedf48a9b31312eb3acd0abcd2b6816 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 14:58:43 -0400 Subject: [PATCH 3/6] Bind UI work to lifecycle scopes instead of throwaway scopes AccountSettingsFragment launched six coroutines on inline CoroutineScope(Dispatchers.Main) instances that were never cancelled, while touching findPreference, getString and requireContext from inside them. The three driven by click and preference-change listeners now use viewLifecycleOwner.lifecycleScope, matching the call already present in this file. The three SyncStatusObserver callbacks use the fragment's own lifecycleScope instead, because they do not arrive on the main thread: FxaAccountManager defaults to a single-threaded background executor, and ObserverRegistry.notifyObservers invokes observers directly on the calling thread. Fragment.getViewLifecycleOwner is not safe to call from there and throws once the view is gone, whereas Lifecycle.coroutineScope is safe. In both cases Dispatchers.Main stays explicit to preserve the previous non-immediate dispatch. IntentReceiverActivity needed no coroutine at all: IntentProcessor.process is not a suspend function, so the MainScope().launch only served to defer the work by one main-loop pass while leaking an uncancellable job. Inline it. --- .../browser/IntentReceiverActivity.kt | 24 ++++++++----------- .../settings/AccountSettingsFragment.kt | 13 +++++----- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/IntentReceiverActivity.kt b/app/src/main/java/org/mozilla/reference/browser/IntentReceiverActivity.kt index 9c025d03d..7ba5840d0 100644 --- a/app/src/main/java/org/mozilla/reference/browser/IntentReceiverActivity.kt +++ b/app/src/main/java/org/mozilla/reference/browser/IntentReceiverActivity.kt @@ -7,8 +7,6 @@ package org.mozilla.reference.browser import android.app.Activity import android.content.Intent import android.os.Bundle -import kotlinx.coroutines.MainScope -import kotlinx.coroutines.launch import org.mozilla.reference.browser.ext.components class IntentReceiverActivity : Activity() { @@ -27,20 +25,18 @@ class IntentReceiverActivity : Activity() { val utils = components.utils - MainScope().launch { - val processor = utils.intentProcessors.firstOrNull { it.process(intent) } + val processor = utils.intentProcessors.firstOrNull { it.process(intent) } - val className = - if (processor in utils.externalIntentProcessors) { - ExternalAppBrowserActivity::class - } else { - BrowserActivity::class - } + val className = + if (processor in utils.externalIntentProcessors) { + ExternalAppBrowserActivity::class + } else { + BrowserActivity::class + } - intent.setClassName(applicationContext, className.java.name) + intent.setClassName(applicationContext, className.java.name) - startActivity(intent) - finish() - } + startActivity(intent) + finish() } } diff --git a/app/src/main/java/org/mozilla/reference/browser/settings/AccountSettingsFragment.kt b/app/src/main/java/org/mozilla/reference/browser/settings/AccountSettingsFragment.kt index 8b7df9b3b..7bb0ec490 100644 --- a/app/src/main/java/org/mozilla/reference/browser/settings/AccountSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/reference/browser/settings/AccountSettingsFragment.kt @@ -15,7 +15,6 @@ import androidx.preference.CheckBoxPreference import androidx.preference.Preference import androidx.preference.Preference.OnPreferenceClickListener import androidx.preference.PreferenceFragmentCompat -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.concept.sync.SyncEngine @@ -40,7 +39,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { private val syncStatusObserver = object : SyncStatusObserver { override fun onStarted() { - CoroutineScope(Dispatchers.Main).launch { + lifecycleScope.launch(Dispatchers.Main) { val pref = findPreference(requireContext().getPreferenceKey(pref_key_sync_now)) pref?.title = getString(R.string.syncing) @@ -50,7 +49,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { // Sync stopped successfully. override fun onIdle() { - CoroutineScope(Dispatchers.Main).launch { + lifecycleScope.launch(Dispatchers.Main) { val pref = findPreference(requireContext().getPreferenceKey(pref_key_sync_now)) pref?.title = getString(R.string.sync_now) pref?.isEnabled = true @@ -61,7 +60,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { // Sync stopped after encountering a problem. override fun onError(error: Exception?) { - CoroutineScope(Dispatchers.Main).launch { + lifecycleScope.launch(Dispatchers.Main) { val pref = findPreference(requireContext().getPreferenceKey(pref_key_sync_now)) pref?.title = getString(R.string.sync_now) pref?.isEnabled = true @@ -146,7 +145,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } private fun getClickListenerForSignOut(): OnPreferenceClickListener = OnPreferenceClickListener { - CoroutineScope(Dispatchers.Main).launch { + viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { requireComponents.backgroundServices.accountManager.logout() activity?.onBackPressedDispatcher?.onBackPressed() } @@ -154,7 +153,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { } private fun getClickListenerForSyncNow(): OnPreferenceClickListener = OnPreferenceClickListener { - CoroutineScope(Dispatchers.Main).launch { + viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { // Trigger a sync & update devices. requireComponents.backgroundServices.accountManager.syncNow(SyncReason.User) // Poll for device events. @@ -196,7 +195,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { engine: SyncEngine, newState: Boolean, ) { - CoroutineScope(Dispatchers.Main).launch { + viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { requireComponents.backgroundServices.accountManager.setEngineEnabled(engine, newState) } } From b133c74231d2814ced8eaee90843aaeb3a9055ca Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 15:01:39 -0400 Subject: [PATCH 4/6] Cancel retained coroutine scopes on teardown Three classes held a long-lived CoroutineScope that was never cancelled. ToolbarIntegration launched a BrowserStore collector from init and left it running. Because the store is application-lived, that subscription retained the feature, and with it the fragment's view and context, well past view destruction. Move the collector into start() and cancel it in stop() so it follows the LifecycleAwareFeature contract. Cancelling the scope in stop() alone would not have worked, since nothing would restart the collector. InstalledAddonDetailsActivity and AddonsFragment each hand-rolled a CoroutineScope(Dispatchers.IO) with no cancellation. Both already have a lifecycle to hang off, so use lifecycleScope and viewLifecycleOwner's lifecycleScope respectively, keeping the IO dispatcher where the work is off the main thread. AddonsFragment uses the view lifecycle because its main-thread continuations touch the RecyclerView. --- .../browser/addons/AddonsFragment.kt | 38 +++++++++---------- .../addons/InstalledAddonDetailsActivity.kt | 10 ++--- .../browser/browser/ToolbarIntegration.kt | 28 ++++++++------ 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt index 15738f8c1..d08a3552a 100644 --- a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt +++ b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt @@ -11,11 +11,12 @@ import android.view.View import android.view.ViewGroup import android.widget.Toast import androidx.fragment.app.Fragment +import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import mozilla.components.feature.addons.Addon import mozilla.components.feature.addons.AddonManagerException import mozilla.components.feature.addons.R as addonsR @@ -29,7 +30,6 @@ import org.mozilla.reference.browser.ext.components class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate { private val webExtensionPromptFeature = ViewBoundFeatureWrapper() private lateinit var recyclerView: RecyclerView - private val scope = CoroutineScope(Dispatchers.IO) private lateinit var addons: List private var adapter: AddonsManagerAdapter? = null @@ -73,28 +73,24 @@ class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate { private fun bindRecyclerView(rootView: View) { recyclerView = rootView.findViewById(R.id.add_ons_list) recyclerView.layoutManager = LinearLayoutManager(requireContext()) - scope.launch { + viewLifecycleOwner.lifecycleScope.launch { try { - addons = requireContext().components.core.addonManager.getAddons() + addons = withContext(Dispatchers.IO) { requireContext().components.core.addonManager.getAddons() } - scope.launch(Dispatchers.Main) { - adapter = - AddonsManagerAdapter( - this@AddonsFragment, - addons, - store = requireContext().components.core.store, - ) - recyclerView.adapter = adapter - } + adapter = + AddonsManagerAdapter( + this@AddonsFragment, + addons, + store = requireContext().components.core.store, + ) + recyclerView.adapter = adapter } catch (e: AddonManagerException) { - scope.launch(Dispatchers.Main) { - Toast.makeText( - activity, - addonsR.string.mozac_feature_addons_failed_to_load_extensions, - Toast.LENGTH_SHORT, - ) - .show() - } + Toast.makeText( + activity, + addonsR.string.mozac_feature_addons_failed_to_load_extensions, + Toast.LENGTH_SHORT, + ) + .show() } } } diff --git a/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt b/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt index acd575019..eab36e068 100644 --- a/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt +++ b/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt @@ -14,7 +14,7 @@ import androidx.activity.enableEdgeToEdge import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.widget.SwitchCompat import androidx.core.view.isVisible -import kotlinx.coroutines.CoroutineScope +import androidx.lifecycle.lifecycleScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.feature.addons.Addon @@ -28,8 +28,6 @@ import org.mozilla.reference.browser.ext.components /** An activity to show the details of a installed add-on. */ class InstalledAddonDetailsActivity : AppCompatActivity() { - private val scope = CoroutineScope(Dispatchers.IO) - override fun onCreate(savedInstanceState: Bundle?) { enableEdgeToEdge(SystemBarStyle.dark(Color.TRANSPARENT)) super.onCreate(savedInstanceState) @@ -45,10 +43,10 @@ class InstalledAddonDetailsActivity : AppCompatActivity() { } private fun bindAddon(addon: Addon) { - scope.launch { + lifecycleScope.launch(Dispatchers.IO) { try { val addons = baseContext.components.core.addonManager.getAddons() - scope.launch(Dispatchers.Main) { + lifecycleScope.launch(Dispatchers.Main) { addons .find { addon.id == it.id } .let { @@ -60,7 +58,7 @@ class InstalledAddonDetailsActivity : AppCompatActivity() { } } } catch (e: AddonManagerException) { - scope.launch(Dispatchers.Main) { + lifecycleScope.launch(Dispatchers.Main) { Toast.makeText( baseContext, addonsR.string.mozac_feature_addons_failed_to_load_extensions, diff --git a/app/src/main/java/org/mozilla/reference/browser/browser/ToolbarIntegration.kt b/app/src/main/java/org/mozilla/reference/browser/browser/ToolbarIntegration.kt index 0b8ac7e15..483eb32f8 100644 --- a/app/src/main/java/org/mozilla/reference/browser/browser/ToolbarIntegration.kt +++ b/app/src/main/java/org/mozilla/reference/browser/browser/ToolbarIntegration.kt @@ -10,6 +10,7 @@ import android.view.View import android.view.ViewGroup import androidx.core.content.ContextCompat import androidx.core.content.res.ResourcesCompat +import kotlinx.coroutines.Job import kotlinx.coroutines.MainScope import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map @@ -52,7 +53,7 @@ class ToolbarIntegration( toolbar: BrowserToolbar, toolbarParentView: View, historyStorage: PlacesHistoryStorage, - store: BrowserStore, + private val store: BrowserStore, private val sessionUseCases: SessionUseCases, private val tabsUseCases: TabsUseCases, private val webAppUseCases: WebAppUseCases, @@ -64,6 +65,7 @@ class ToolbarIntegration( } private val scope = MainScope() + private var menuUpdateJob: Job? = null private fun menuToolbar(session: SessionState?): RowMenuCandidate { val tint = ContextCompat.getColor(context, R.color.icons) @@ -217,16 +219,6 @@ class ToolbarIntegration( } }, ) - - scope.launch { - store - .flow() - .map { state -> state.selectedTab } - .distinctUntilChanged() - .collect { tab -> - browserMenuController.submitList(menuItems(tab)) - } - } } private val toolbarFeature: ToolbarFeature = @@ -246,10 +238,24 @@ class ToolbarIntegration( override fun start() { toolbarFeature.start() + + menuUpdateJob?.cancel() + menuUpdateJob = scope.launch { + store + .flow() + .map { state -> state.selectedTab } + .distinctUntilChanged() + .collect { tab -> + browserMenuController.submitList(menuItems(tab)) + } + } } override fun stop() { toolbarFeature.stop() + + menuUpdateJob?.cancel() + menuUpdateJob = null } override fun onBackPressed(): Boolean = toolbarFeature.onBackPressed() From fc43cc38e199c0ea36cdc8863c64842451fe4a6c Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 15:25:06 -0400 Subject: [PATCH 5/6] Handle add-on load failures instead of crashing or going silent bindAddon in InstalledAddonDetailsActivity threw AddonManagerException from inside a nested lifecycleScope.launch(Dispatchers.Main), a separate coroutine from the try meant to handle it. The catch never saw it, so an add-on missing from the manager's list crashed instead of showing the failure toast. Collapse the nested launches into a single main-dispatched coroutine with withContext around the blocking getAddons call, which puts the throw and the catch in the same coroutine. Both this catch and the one in AddonsFragment also dropped the exception on the floor. Log it so a provider or storage failure leaves a trace beyond the generic toast. --- .../browser/addons/AddonsFragment.kt | 3 ++ .../addons/InstalledAddonDetailsActivity.kt | 39 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt index d08a3552a..8a8735f59 100644 --- a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt +++ b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt @@ -23,11 +23,13 @@ import mozilla.components.feature.addons.R as addonsR import mozilla.components.feature.addons.ui.AddonsManagerAdapter import mozilla.components.feature.addons.ui.AddonsManagerAdapterDelegate import mozilla.components.support.base.feature.ViewBoundFeatureWrapper +import mozilla.components.support.base.log.logger.Logger import org.mozilla.reference.browser.R import org.mozilla.reference.browser.ext.components /** Fragment use for managing add-ons. */ class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate { + private val logger = Logger("AddonsFragment") private val webExtensionPromptFeature = ViewBoundFeatureWrapper() private lateinit var recyclerView: RecyclerView private lateinit var addons: List @@ -85,6 +87,7 @@ class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate { ) recyclerView.adapter = adapter } catch (e: AddonManagerException) { + logger.error("Failed to load add-ons", e) Toast.makeText( activity, addonsR.string.mozac_feature_addons_failed_to_load_extensions, diff --git a/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt b/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt index eab36e068..360d73af2 100644 --- a/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt +++ b/app/src/main/java/org/mozilla/reference/browser/addons/InstalledAddonDetailsActivity.kt @@ -17,10 +17,12 @@ import androidx.core.view.isVisible import androidx.lifecycle.lifecycleScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import mozilla.components.feature.addons.Addon import mozilla.components.feature.addons.AddonManagerException import mozilla.components.feature.addons.R as addonsR import mozilla.components.feature.addons.ui.translateName +import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.ktx.android.view.setupPersistentInsets import mozilla.components.support.utils.ext.getParcelableExtraCompat import org.mozilla.reference.browser.R @@ -28,6 +30,8 @@ import org.mozilla.reference.browser.ext.components /** An activity to show the details of a installed add-on. */ class InstalledAddonDetailsActivity : AppCompatActivity() { + private val logger = Logger("InstalledAddonDetailsActivity") + override fun onCreate(savedInstanceState: Bundle?) { enableEdgeToEdge(SystemBarStyle.dark(Color.TRANSPARENT)) super.onCreate(savedInstanceState) @@ -43,29 +47,22 @@ class InstalledAddonDetailsActivity : AppCompatActivity() { } private fun bindAddon(addon: Addon) { - lifecycleScope.launch(Dispatchers.IO) { + lifecycleScope.launch { try { - val addons = baseContext.components.core.addonManager.getAddons() - lifecycleScope.launch(Dispatchers.Main) { - addons - .find { addon.id == it.id } - .let { - if (it == null) { - throw AddonManagerException(Exception("Addon ${addon.id} not found")) - } else { - bindUI(it) - } - } - } + val addons = withContext(Dispatchers.IO) { baseContext.components.core.addonManager.getAddons() } + val installed = + addons.find { addon.id == it.id } + ?: throw AddonManagerException(Exception("Addon ${addon.id} not found")) + + bindUI(installed) } catch (e: AddonManagerException) { - lifecycleScope.launch(Dispatchers.Main) { - Toast.makeText( - baseContext, - addonsR.string.mozac_feature_addons_failed_to_load_extensions, - Toast.LENGTH_SHORT, - ) - .show() - } + logger.error("Failed to load add-on ${addon.id}", e) + Toast.makeText( + baseContext, + addonsR.string.mozac_feature_addons_failed_to_load_extensions, + Toast.LENGTH_SHORT, + ) + .show() } } } From 760d4433934a2cead5f87e1783d6ecf9c5e3158e Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 1 Sep 2026 15:40:43 -0400 Subject: [PATCH 6/6] Load the add-on list once per start instead of twice AddonsFragment called bindRecyclerView from both onViewCreated and onStart. Since onStart always follows onViewCreated, every first display issued two concurrent getAddons calls and built two adapters racing to set recyclerView.adapter. Keep the onStart call, which also serves as the refresh after returning from the details activity, and drop the redundant one. --- .../java/org/mozilla/reference/browser/addons/AddonsFragment.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt index 8a8735f59..f78b9281c 100644 --- a/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt +++ b/app/src/main/java/org/mozilla/reference/browser/addons/AddonsFragment.kt @@ -49,7 +49,6 @@ class AddonsFragment : Fragment(), AddonsManagerAdapterDelegate { savedInstanceState: Bundle?, ) { super.onViewCreated(rootView, savedInstanceState) - bindRecyclerView(rootView) webExtensionPromptFeature.set( feature = WebExtensionPromptFeature(