For #13959: change StrictModeManager to class from object.

I originally tried to create this PR leaving this as an object to keep
the change simple but it wasn't worth it - once the object started to
keep state, we'd need to manually reset the state between runs. Also,
the tests were already getting hacky with static mocking so it was
easier to address some of those issues this way too.
pull/184/head
Michael Comella 4 years ago committed by Michael Comella
parent d4ab728cff
commit 6abeb2d9e7

@ -40,7 +40,6 @@ import mozilla.components.support.rusthttp.RustHttpConfig
import mozilla.components.support.rustlog.RustLog import mozilla.components.support.rustlog.RustLog
import mozilla.components.support.utils.logElapsedTime import mozilla.components.support.utils.logElapsedTime
import mozilla.components.support.webextensions.WebExtensionSupport import mozilla.components.support.webextensions.WebExtensionSupport
import org.mozilla.fenix.StrictModeManager.enableStrictMode
import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.Components
import org.mozilla.fenix.components.metrics.MetricServiceType import org.mozilla.fenix.components.metrics.MetricServiceType
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
@ -126,7 +125,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
val megazordSetup = setupMegazord() val megazordSetup = setupMegazord()
setDayNightTheme() setDayNightTheme()
enableStrictMode(true) components.strictMode.enableStrictMode(true)
warmBrowsersCache() warmBrowsersCache()
// Make sure the engine is initialized and ready to use. // Make sure the engine is initialized and ready to use.

@ -150,7 +150,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
private lateinit var navigationToolbar: Toolbar private lateinit var navigationToolbar: Toolbar
final override fun onCreate(savedInstanceState: Bundle?) { 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 // There is disk read violations on some devices such as samsung and pixel for android 9/10
StrictMode.allowThreadDiskReads().resetPoliciesAfter { StrictMode.allowThreadDiskReads().resetPoliciesAfter {
super.onCreate(savedInstanceState) super.onCreate(savedInstanceState)

@ -9,24 +9,24 @@ import android.os.StrictMode
import androidx.fragment.app.Fragment import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager 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. * 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. * 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. * @param setPenaltyDeath boolean value to decide setting the penaltyDeath as a penalty.
*/ */
fun enableStrictMode(setPenaltyDeath: Boolean) { fun enableStrictMode(setPenaltyDeath: Boolean) {
// The expression in this if is duplicated in StrictMode.ThreadPolicy.resetPoliciesAfter if (isEnabledByBuildConfig) {
// 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) {
val threadPolicy = StrictMode.ThreadPolicy.Builder() val threadPolicy = StrictMode.ThreadPolicy.Builder()
.detectAll() .detectAll()
.penaltyLog() .penaltyLog()
@ -67,9 +67,6 @@ object StrictModeManager {
}, false) }, 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. * 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 * These classes violates the [StrictMode] policies on startup. As a workaround, we create

@ -19,6 +19,7 @@ import mozilla.components.support.migration.state.MigrationStore
import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.BuildConfig
import org.mozilla.fenix.Config import org.mozilla.fenix.Config
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.StrictModeManager
import org.mozilla.fenix.components.metrics.AppStartupTelemetry import org.mozilla.fenix.components.metrics.AppStartupTelemetry
import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.utils.ClipboardHandler import org.mozilla.fenix.utils.ClipboardHandler
@ -126,6 +127,7 @@ class Components(private val context: Context) {
val performance by lazy { PerformanceComponent() } val performance by lazy { PerformanceComponent() }
val push by lazy { Push(context, analytics.crashReporter) } val push by lazy { Push(context, analytics.crashReporter) }
val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context as Application) } val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context as Application) }
val strictMode by lazy { StrictModeManager(Config) }
val settings by lazy { Settings(context) } val settings by lazy { Settings(context) }

@ -11,10 +11,8 @@ import io.mockk.confirmVerified
import io.mockk.every import io.mockk.every
import io.mockk.impl.annotations.MockK import io.mockk.impl.annotations.MockK
import io.mockk.mockk import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.mockkStatic import io.mockk.mockkStatic
import io.mockk.slot import io.mockk.slot
import io.mockk.unmockkObject
import io.mockk.unmockkStatic import io.mockk.unmockkStatic
import io.mockk.verify import io.mockk.verify
import org.junit.After import org.junit.After
@ -26,44 +24,49 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@RunWith(FenixRobolectricTestRunner::class) @RunWith(FenixRobolectricTestRunner::class)
class StrictModeManagerTest { class StrictModeManagerTest {
private lateinit var debugManager: StrictModeManager
private lateinit var releaseManager: StrictModeManager
@MockK(relaxUnitFun = true) private lateinit var fragmentManager: FragmentManager @MockK(relaxUnitFun = true) private lateinit var fragmentManager: FragmentManager
@Before @Before
fun setup() { fun setup() {
MockKAnnotations.init(this) MockKAnnotations.init(this)
mockkStatic(StrictMode::class) 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 @After
fun teardown() { fun teardown() {
unmockkStatic(StrictMode::class) unmockkStatic(StrictMode::class)
unmockkObject(Config)
} }
@Test @Test
fun `test enableStrictMode in release`() { fun `test enableStrictMode in release`() {
every { Config.channel } returns ReleaseChannel.Release releaseManager.enableStrictMode(false)
StrictModeManager.enableStrictMode(false)
verify(exactly = 0) { StrictMode.setThreadPolicy(any()) } verify(exactly = 0) { StrictMode.setThreadPolicy(any()) }
verify(exactly = 0) { StrictMode.setVmPolicy(any()) } verify(exactly = 0) { StrictMode.setVmPolicy(any()) }
} }
@Test @Test
fun `test enableStrictMode in debug`() { fun `test enableStrictMode in debug`() {
every { Config.channel } returns ReleaseChannel.Debug debugManager.enableStrictMode(false)
StrictModeManager.enableStrictMode(false)
verify { StrictMode.setThreadPolicy(any()) } verify { StrictMode.setThreadPolicy(any()) }
verify { StrictMode.setVmPolicy(any()) } verify { StrictMode.setVmPolicy(any()) }
} }
@Test @Test
fun `test changeStrictModePolicies`() { fun `test changeStrictModePolicies in debug`() {
val callbacks = slot<FragmentManager.FragmentLifecycleCallbacks>() val callbacks = slot<FragmentManager.FragmentLifecycleCallbacks>()
StrictModeManager.attachListenerToDisablePenaltyDeath(fragmentManager) debugManager.attachListenerToDisablePenaltyDeath(fragmentManager)
verify { fragmentManager.registerFragmentLifecycleCallbacks(capture(callbacks), false) } verify { fragmentManager.registerFragmentLifecycleCallbacks(capture(callbacks), false) }
confirmVerified(fragmentManager) confirmVerified(fragmentManager)

Loading…
Cancel
Save