diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index e3b7e9ed6..8213dcb03 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -40,7 +40,6 @@ import mozilla.components.support.rusthttp.RustHttpConfig import mozilla.components.support.rustlog.RustLog import mozilla.components.support.utils.logElapsedTime import mozilla.components.support.webextensions.WebExtensionSupport -import org.mozilla.fenix.StrictModeManager.enableStrictMode import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.MetricServiceType import org.mozilla.fenix.ext.components @@ -126,7 +125,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { val megazordSetup = setupMegazord() setDayNightTheme() - enableStrictMode(true) + components.strictMode.enableStrictMode(true) warmBrowsersCache() // Make sure the engine is initialized and ready to use. diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index ea8d863de..e997c9f55 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -150,7 +150,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { private lateinit var navigationToolbar: Toolbar final override fun onCreate(savedInstanceState: Bundle?) { - StrictModeManager.attachListenerToDisablePenaltyDeath(supportFragmentManager) + components.strictMode.attachListenerToDisablePenaltyDeath(supportFragmentManager) // There is disk read violations on some devices such as samsung and pixel for android 9/10 StrictMode.allowThreadDiskReads().resetPoliciesAfter { super.onCreate(savedInstanceState) diff --git a/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt b/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt index 25944f7d6..2b9594e50 100644 --- a/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt +++ b/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt @@ -9,24 +9,24 @@ import android.os.StrictMode import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager +private const val MANUFACTURE_HUAWEI: String = "HUAWEI" +private const val MANUFACTURE_ONE_PLUS: String = "OnePlus" + /** * Manages strict mode settings for the application. - * - * Due to the static dependencies in this class, it's getting hard to test: if this class takes any - * many more static dependencies, consider switching it, and other code that uses [StrictMode] like - * [StrictMode.ThreadPolicy].resetPoliciesAfter, to a class with dependency injection. */ -object StrictModeManager { +class StrictModeManager(config: Config) { + + // The expression in this if is duplicated in StrictMode.ThreadPolicy.resetPoliciesAfter + // because we don't want to have to pass in a dependency each time the ext fn is called. + private val isEnabledByBuildConfig = config.channel.isDebug /*** * Enables strict mode for debug purposes. meant to be run only in the main process. * @param setPenaltyDeath boolean value to decide setting the penaltyDeath as a penalty. */ fun enableStrictMode(setPenaltyDeath: Boolean) { - // The expression in this if is duplicated in StrictMode.ThreadPolicy.resetPoliciesAfter - // because the tests break in unexpected ways if the value is shared as a constant in this - // class. It wasn't worth the time to address it. - if (Config.channel.isDebug) { + if (isEnabledByBuildConfig) { val threadPolicy = StrictMode.ThreadPolicy.Builder() .detectAll() .penaltyLog() @@ -67,9 +67,6 @@ object StrictModeManager { }, false) } - private const val MANUFACTURE_HUAWEI: String = "HUAWEI" - private const val MANUFACTURE_ONE_PLUS: String = "OnePlus" - /** * There are certain manufacturers that have custom font classes for the OS systems. * These classes violates the [StrictMode] policies on startup. As a workaround, we create diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 24846cc4d..8bcbce420 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -19,6 +19,7 @@ import mozilla.components.support.migration.state.MigrationStore import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.StrictModeManager import org.mozilla.fenix.components.metrics.AppStartupTelemetry import org.mozilla.fenix.ext.settings import org.mozilla.fenix.utils.ClipboardHandler @@ -126,6 +127,7 @@ class Components(private val context: Context) { val performance by lazy { PerformanceComponent() } val push by lazy { Push(context, analytics.crashReporter) } val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context as Application) } + val strictMode by lazy { StrictModeManager(Config) } val settings by lazy { Settings(context) } diff --git a/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt b/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt index 62ec64d28..5c5601c9d 100644 --- a/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt @@ -11,10 +11,8 @@ import io.mockk.confirmVerified import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk -import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.slot -import io.mockk.unmockkObject import io.mockk.unmockkStatic import io.mockk.verify import org.junit.After @@ -26,44 +24,49 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) class StrictModeManagerTest { + private lateinit var debugManager: StrictModeManager + private lateinit var releaseManager: StrictModeManager + @MockK(relaxUnitFun = true) private lateinit var fragmentManager: FragmentManager @Before fun setup() { MockKAnnotations.init(this) mockkStatic(StrictMode::class) - mockkObject(Config) + + // These tests log a warning that mockk couldn't set the backing field of Config.channel + // but it doesn't seem to impact their correctness so I'm ignoring it. + val debugConfig: Config = mockk { every { channel } returns ReleaseChannel.Debug } + debugManager = StrictModeManager(debugConfig) + + val releaseConfig: Config = mockk { every { channel } returns ReleaseChannel.Release } + releaseManager = StrictModeManager(releaseConfig) } @After fun teardown() { unmockkStatic(StrictMode::class) - unmockkObject(Config) } @Test fun `test enableStrictMode in release`() { - every { Config.channel } returns ReleaseChannel.Release - StrictModeManager.enableStrictMode(false) - + releaseManager.enableStrictMode(false) verify(exactly = 0) { StrictMode.setThreadPolicy(any()) } verify(exactly = 0) { StrictMode.setVmPolicy(any()) } } @Test fun `test enableStrictMode in debug`() { - every { Config.channel } returns ReleaseChannel.Debug - StrictModeManager.enableStrictMode(false) - + debugManager.enableStrictMode(false) verify { StrictMode.setThreadPolicy(any()) } verify { StrictMode.setVmPolicy(any()) } } @Test - fun `test changeStrictModePolicies`() { + fun `test changeStrictModePolicies in debug`() { val callbacks = slot() - StrictModeManager.attachListenerToDisablePenaltyDeath(fragmentManager) + debugManager.attachListenerToDisablePenaltyDeath(fragmentManager) verify { fragmentManager.registerFragmentLifecycleCallbacks(capture(callbacks), false) } confirmVerified(fragmentManager)