tor-browser

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

commit e75f6f504902ac76a6f1e303a56841209174fde3
parent a6944a13821727f23cdfecc98271d7f723bb6994
Author: Ted Campbell <tcampbell@mozilla.com>
Date:   Wed,  1 Oct 2025 04:44:00 +0000

Bug 1991579 - Avoid calling StrictMode code in Fenix release r=mcarare,android-reviewers

We have checks to avoid restoring old flags in release builds already, but the
suppress call happens in the caller while evaluating arguments. This patch changes
`resetAfter` into `allowViolation` and a method is passed that would apply
the suppression instead of doing it directly.

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

Diffstat:
Mmobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/HomeActivityTestRule.kt | 2+-
Mmobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt | 2+-
Mmobile/android/fenix/app/src/debug/java/org/mozilla/fenix/DebugFenixApplication.kt | 4++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt | 4++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt | 8++++----
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt | 4++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt | 4++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/AccountAbnormalities.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/UseCases.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/ui/FenixOverlay.kt | 9++++-----
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt | 4++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt | 59++++++++++++++++++++++++++++++-----------------------------
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/whatsnew/WhatsNew.kt | 2+-
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/widget/VoiceSearchActivity.kt | 2+-
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/helpers/perf/TestStrictModeManager.kt | 4++--
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/perf/StrictModeManagerTest.kt | 61++++++++++++++++++++++++++++++++++++++++++-------------------
20 files changed, 102 insertions(+), 79 deletions(-)

diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/HomeActivityTestRule.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/HomeActivityTestRule.kt @@ -361,7 +361,7 @@ private fun skipOnboardingBeforeLaunch() { // As we are disabling the onboarding we need to initialize glean manually, // as it runs after the onboarding finishes Handler(Looper.getMainLooper()).post { - appContext.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + appContext.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { initializeGlean( applicationContext = appContext, logger = Logger(), diff --git a/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/mobile/android/fenix/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -33,7 +33,7 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule * * Say no to main thread IO! 🙅 */ -private const val EXPECTED_SUPPRESSION_COUNT = 16 +private const val EXPECTED_SUPPRESSION_COUNT = 13 /** * The number of times we call the `runBlocking` coroutine method on the main thread during this diff --git a/mobile/android/fenix/app/src/debug/java/org/mozilla/fenix/DebugFenixApplication.kt b/mobile/android/fenix/app/src/debug/java/org/mozilla/fenix/DebugFenixApplication.kt @@ -21,7 +21,7 @@ class DebugFenixApplication : FenixApplication() { ) } - val isEnabled = components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + val isEnabled = components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { PreferenceManager.getDefaultSharedPreferences(this) .getBoolean(getPreferenceKey(R.string.pref_key_leakcanary), BuildConfig.LEAKCANARY) } @@ -31,7 +31,7 @@ class DebugFenixApplication : FenixApplication() { override fun updateLeakCanaryState(isEnabled: Boolean) { LeakCanary.showLeakDisplayActivityLauncherIcon(isEnabled) - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { LeakCanary.config = LeakCanary.config.copy(dumpHeap = isEnabled) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -234,7 +234,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { run { // Make sure the engine is initialized and ready to use. - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { components.core.engine.warmUp() } @@ -1034,7 +1034,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { // There's a strict mode violation in A-Cs LocaleAwareApplication which // reads from shared prefs: https://github.com/mozilla-mobile/android-components/issues/8816 - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { super.onConfigurationChanged(config) } } else { 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 @@ -378,7 +378,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { MarkersFragmentLifecycleCallbacks.register(supportFragmentManager, components.core.engine) // There is disk read violations on some devices such as samsung and pixel for android 9/10 - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { // Browsing mode & theme setup should always be called before super.onCreate. browsingModeManager = createBrowsingModeManager(intent) setupTheme() @@ -630,7 +630,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { ) { if (shouldShowSetAsDefaultPrompt && !isDefaultBrowser && isTheCorrectBuildVersion) { // This is to avoid disk read violations on some devices such as samsung and pixel for android 9/10 - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { components.appStore.dispatch(AppAction.UpdateWasNativeDefaultBrowserPromptShown(true)) showSetDefaultBrowserPrompt() Metrics.setAsDefaultBrowserNativePromptShown.record() @@ -1292,7 +1292,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } final override fun attachBaseContext(base: Context) { - base.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + base.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { super.attachBaseContext(base) } } @@ -1391,7 +1391,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { */ @VisibleForTesting internal fun shouldStartOnHome(intent: Intent? = this.intent): Boolean { - return components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + return components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { // We only want to open on home when users tap the app, // we want to ignore other cases when the app gets open by users clicking on links. getSettings().shouldStartOnHome() && intent?.action == ACTION_MAIN diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt @@ -54,7 +54,7 @@ class IntentReceiverActivity : Activity() { } // StrictMode violation on certain devices such as Samsung - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { super.onCreate(savedInstanceState) } @@ -124,7 +124,7 @@ class IntentReceiverActivity : Activity() { ) } // StrictMode violation on certain devices such as Samsung - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { startActivity(intent) } finish() // must finish() after starting the other activity diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -268,7 +268,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { private fun initReaderModeUpdates(context: Context, view: View) { readerViewFeature.set( - feature = context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + feature = context.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { ReaderViewFeature( context = context, engine = context.components.core.engine, @@ -345,7 +345,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { (browserToolbarView as BrowserToolbarView).toolbar.addPageAction(readerModeAction) readerViewFeature.set( - feature = context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + feature = context.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { ReaderViewFeature( context = context, engine = context.components.core.engine, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/AccountAbnormalities.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/AccountAbnormalities.kt @@ -79,7 +79,7 @@ class AccountAbnormalities( private val hadAccountPrior: Boolean init { - val prefPair = strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + val prefPair = strictMode.allowViolation(StrictMode::allowThreadDiskReads) { val p = context.getSharedPreferences(PREF_FXA_ABNORMALITIES, Context.MODE_PRIVATE) val a = p.getBoolean(KEY_HAS_ACCOUNT, false) Pair(p, a) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -615,7 +615,7 @@ class Core( val topSitesStorage by lazyMonitored { val defaultTopSites = mutableListOf<Pair<String, String>>() - strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + strictMode.allowViolation(StrictMode::allowThreadDiskReads) { if (!context.settings().defaultTopSitesAdded) { defaultTopSites.add( Pair( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/TabCollectionStorage.kt @@ -55,7 +55,7 @@ class TabCollectionStorage( var cachedTabCollections = listOf<TabCollection>() private val collectionStorage by lazy { - strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + strictMode.allowViolation(StrictMode::allowThreadDiskReads) { TabCollectionStorage(context) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/UseCases.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/UseCases.kt @@ -117,7 +117,7 @@ class UseCases( val wallpaperUseCases by lazyMonitored { // Required to even access context.filesDir property and to retrieve current locale - val (rootStorageDirectory, currentLocale) = strictMode.value.resetAfter(StrictMode.allowThreadDiskReads()) { + val (rootStorageDirectory, currentLocale) = strictMode.value.allowViolation(StrictMode::allowThreadDiskReads) { val rootStorageDirectory = context.filesDir val currentLocale = LocaleManager.getCurrentLocale(context)?.toLanguageTag() ?: LocaleManager.getSystemDefault().toLanguageTag() diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/ui/FenixOverlay.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/ui/FenixOverlay.kt @@ -108,11 +108,10 @@ fun FenixOverlay( ), ), loginsStorage = loginsStorage, - addressesDebugLocalesRepository = context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { - SharedPrefsAddressesDebugLocalesRepository( - context, - ) - }, + addressesDebugLocalesRepository = + context.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { + SharedPrefsAddressesDebugLocalesRepository(context) + }, creditCardsAddressesStorage = context.components.core.autofillStorage, inactiveTabsEnabled = inactiveTabsEnabled, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt @@ -50,7 +50,7 @@ class SpeechProcessingIntentProcessor( } private fun launchToBrowser(searchEngine: SearchEngine, text: String) { - activity.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + activity.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { MetricsUtils.recordSearchMetrics( searchEngine, searchEngine == store.state.search.selectedOrDefaultSearchEngine, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt @@ -17,7 +17,7 @@ class FenixOnboarding(context: Context) : PreferencesHolder { private val strictMode = context.components.strictMode - override val preferences: SharedPreferences = strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + override val preferences: SharedPreferences = strictMode.allowViolation(StrictMode::allowThreadDiskReads) { context.getSharedPreferences( PREF_NAME_ONBOARDING_KEY, Context.MODE_PRIVATE, @@ -40,7 +40,7 @@ class FenixOnboarding(context: Context) : PreferencesHolder { } fun userHasBeenOnboarded(): Boolean { - return strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + return strictMode.allowViolation(StrictMode::allowThreadDiskReads) { onboardedVersion == CURRENT_ONBOARDING_VERSION } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt @@ -110,7 +110,7 @@ class OnboardingFragment : Fragment() { DefaultBrowserPromptManager( storage = defaultBrowserPromptStorage, promptToSetAsDefaultBrowser = { - requireContext().components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + requireContext().components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { promptToSetAsDefaultBrowser() } }, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt @@ -66,7 +66,8 @@ open class StrictModeManager( val suppressionCount = AtomicLong(0) /*** - * Enables strict mode for debug purposes. meant to be run only in the main process. + * Enables strict mode for debug purposes. Meant to be run only in the main process. For + * simplicity it is also only supported on the main thread. * @param withPenaltyDeath boolean value to decide setting the penaltyDeath as a penalty. */ fun enableStrictMode(withPenaltyDeath: Boolean) { @@ -74,6 +75,10 @@ open class StrictModeManager( return } + if (!mainLooper.isCurrentThread) { + throw IllegalStateException("StrictModeManager only supported on main thread") + } + val threadPolicy = buildThreadPolicy(withPenaltyDeath) applyThreadPolicy(threadPolicy) @@ -164,49 +169,45 @@ open class StrictModeManager( } /** - * Runs the given [functionBlock] and sets the given [StrictMode.ThreadPolicy] after its - * completion when in a build configuration that has StrictMode enabled. If StrictMode is - * not enabled, simply runs the [functionBlock]. This function is written in the style of - * [AutoCloseable.use]. + * Runs the given [functionBlock] while temporarily suppressing StrictMode checks if they are + * enabled by using the [allowFn] before the call. A Gecko Profiler marker is recorded even + * if the strict mode checks are disabled to allow investigating performance of release builds. * - * This function contains perf improvements so it should be - * called instead of [mozilla.components.support.ktx.android.os.resetAfter] (using the wrong - * method should be prevented by a lint check). This is significantly less convenient to run than - * when it was written as an extension function on [StrictMode.ThreadPolicy] but I think this is - * okay: it shouldn't be easy to ignore StrictMode. + * For convenience we allow calling this on non-main threads in which case the [functionBlock] + * is run wihtou profiler markers or StrictMode effects. * * @return the value returned by [functionBlock]. */ - open fun <R> resetAfter(policy: StrictMode.ThreadPolicy, functionBlock: () -> R): R { - fun instrumentedFunctionBlock(): R { - val startProfilerTime = components.core.engine.profiler?.getProfilerTime() - val returnValue = functionBlock() - - if (mainLooper.thread === Thread.currentThread()) { // markers only supported on main thread. - components.core.engine.profiler?.addMarker("StrictMode.resetAfter", startProfilerTime) - } - return returnValue + open fun <R> allowViolation(allowFn: () -> StrictMode.ThreadPolicy, functionBlock: () -> R): R { + // To make things easier for callers, we allow this to be called from non-main threads but in that case + // simply run their block without any profiling or StrictMode effects. + if (!mainLooper.isCurrentThread) { + return functionBlock() } - // Calling resetAfter takes 1-2ms (unknown device) so we only execute it if StrictMode can - // actually be enabled. https://github.com/mozilla-mobile/fenix/issues/11617 - return if (isEnabledByBuildConfig) { - // This can overflow and crash. However, it's unlikely we'll suppress StrictMode 9 - // quintillion times in a build config where StrictMode is enabled so we don't handle it - // because it'd increase complexity. - val suppressionCount = suppressionCount.incrementAndGet() + // Run the functionBlock inside a profiler marker even if we won't use StrictMode so that we + // can still track the performance of the code. + val startProfilerTime = components.core.engine.profiler?.getProfilerTime() - // We log so that devs are more likely to notice that we're suppressing StrictMode violations. + val returnValue = if (isEnabledByBuildConfig) { + // Log that we are suppressing a violation to developer convenience. Technically this + // count can throw on overflow but it is 64-bit and won't practically happen. + val suppressionCount = suppressionCount.incrementAndGet() logger.warn("StrictMode violation suppressed: #$suppressionCount") + // Apply the policy variation, run block, then restore policy. + val policy = allowFn() try { - instrumentedFunctionBlock() + functionBlock() } finally { applyThreadPolicy(policy) } } else { - instrumentedFunctionBlock() + functionBlock() } + + components.core.engine.profiler?.addMarker("StrictMode.allow", startProfilerTime) + return returnValue } /** diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/whatsnew/WhatsNew.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/whatsnew/WhatsNew.kt @@ -71,7 +71,7 @@ class WhatsNew private constructor(private val storage: WhatsNewStorage) { fun shouldHighlightWhatsNew(context: Context): Boolean { return shouldHighlightWhatsNew( ContextWhatsNewVersion(context), - context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + context.components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { SharedPreferenceWhatsNewStorage(context) }, ) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/widget/VoiceSearchActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/widget/VoiceSearchActivity.kt @@ -97,7 +97,7 @@ class VoiceSearchActivity : AppCompatActivity() { ) putExtra( RecognizerIntent.EXTRA_LANGUAGE, - components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + components.strictMode.allowViolation(StrictMode::allowThreadDiskReads) { LocaleManager.getCurrentLocale(this@VoiceSearchActivity) }, ) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/helpers/perf/TestStrictModeManager.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/helpers/perf/TestStrictModeManager.kt @@ -9,7 +9,7 @@ import io.mockk.mockk import org.mozilla.fenix.perf.StrictModeManager /** - * A test version of [StrictModeManager]. This class is difficult to mock because of [resetAfter] + * A test version of [StrictModeManager]. This class is difficult to mock because of [allowViolation] * so we provide a test implementation. */ class TestStrictModeManager : @@ -17,7 +17,7 @@ class TestStrictModeManager : // This method is hard to mock because this method needs to return the return value of the // function passed in. - override fun <R> resetAfter(policy: StrictMode.ThreadPolicy, functionBlock: () -> R): R { + override fun <R> allowViolation(allowFn: () -> StrictMode.ThreadPolicy, functionBlock: () -> R): R { return functionBlock() } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/perf/StrictModeManagerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/perf/StrictModeManagerTest.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.perf +import android.os.Looper import android.os.StrictMode import androidx.fragment.app.FragmentManager import io.mockk.MockKAnnotations @@ -13,8 +14,10 @@ import io.mockk.mockk import io.mockk.slot import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals -import org.junit.Assert.fail +import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -47,6 +50,16 @@ class StrictModeManagerTest { } @Test + fun `GIVEN we're off-main-thread WHEN we enable strict mode THEN throw`() { + runBlocking(Dispatchers.Default) { + assertEquals(false, Looper.getMainLooper().isCurrentThread) + assertThrows(IllegalStateException::class.java) { + debugManager.enableStrictMode(false) + } + } + } + + @Test fun `GIVEN we're in a release build WHEN we enable strict mode THEN we don't set policies`() { releaseManager.enableStrictMode(false) verify(exactly = 0) { releaseManager.applyThreadPolicy(any()) } @@ -73,53 +86,63 @@ class StrictModeManagerTest { } @Test - fun `GIVEN we're in a release build WHEN resetAfter is called THEN we return the value from the function block`() { + fun `GIVEN we're in a release build WHEN allowViolation is called THEN we return the value from the function block`() { val expected = "Hello world" - val actual = releaseManager.resetAfter(StrictMode.allowThreadDiskReads()) { expected } + val actual = releaseManager.allowViolation(StrictMode::allowThreadDiskReads) { expected } assertEquals(expected, actual) } @Test - fun `GIVEN we're in a debug build WHEN resetAfter is called THEN we return the value from the function block`() { + fun `GIVEN we're in a debug build WHEN allowViolation is called THEN we return the value from the function block`() { val expected = "Hello world" - val actual = debugManager.resetAfter(StrictMode.allowThreadDiskReads()) { expected } + val actual = debugManager.allowViolation(StrictMode::allowThreadDiskReads) { expected } assertEquals(expected, actual) } @Test - fun `GIVEN we're in a release build WHEN resetAfter is called THEN the old policy is not set`() { - releaseManager.resetAfter(StrictMode.allowThreadDiskReads()) { "" } + fun `GIVEN we're in a release build WHEN allowViolation is called THEN the old policy is not set`() { + releaseManager.allowViolation(StrictMode::allowThreadDiskReads) { } verify(exactly = 0) { releaseManager.applyThreadPolicy(any()) } } @Test - fun `GIVEN we're in a debug build WHEN resetAfter is called THEN the old policy is set`() { + fun `GIVEN we're in a debug build WHEN allowViolation is called THEN the old policy is set`() { val expectedPolicy = StrictMode.allowThreadDiskReads() - debugManager.resetAfter(expectedPolicy) { "" } + debugManager.allowViolation({ expectedPolicy }) { } verify { debugManager.applyThreadPolicy(expectedPolicy) } } @Test - fun `GIVEN we're in a debug build WHEN resetAfter is called and an exception is thrown from the function THEN the old policy is set`() { + fun `GIVEN we're in a debug build WHEN allowViolation is called and an exception is thrown from the function THEN the old policy is set`() { val expectedPolicy = StrictMode.allowThreadDiskReads() - try { - debugManager.resetAfter(expectedPolicy) { + assertThrows(IllegalStateException::class.java) { + debugManager.allowViolation({ expectedPolicy }) { throw IllegalStateException() } - - @Suppress("UNREACHABLE_CODE") - fail("Expected previous method to throw.") - } catch (e: IllegalStateException) { - // Expected } - verify { debugManager.applyThreadPolicy(expectedPolicy) } } @Test + fun `GIVEN we're in a debug build WHEN allowViolation is called THEN the allowFn should run`() { + var runs = 0 + val allowFn = { runs += 1; StrictMode.allowThreadDiskReads() } + debugManager.allowViolation(allowFn) {} + assertEquals(1, runs) + } + + @Test + fun `GIVEN we're in a release build WHEN allowViolation is called THEN the allowFn should not run`() { + var runs = 0 + val allowFn = { runs += 1; StrictMode.allowThreadDiskReads() } + releaseManager.allowViolation(allowFn) {} + assertEquals(0, runs) + } + + @Test fun `GIVEN we're in debug mode WHEN we suppress StrictMode THEN the suppressed count increases`() { assertEquals(0, debugManager.suppressionCount.get()) - debugManager.resetAfter(StrictMode.allowThreadDiskReads()) { "" } + debugManager.allowViolation(StrictMode::allowThreadDiskReads) { } assertEquals(1, debugManager.suppressionCount.get()) } }