From 50959d997eb949c25f45981aabbf80d681053344 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 30 Sep 2020 16:11:53 -0700 Subject: [PATCH] For #13959: add marker when StrictMode is suppressed. --- .../java/org/mozilla/fenix/StrictModeManager.kt | 17 ++++++++++++++++- .../org/mozilla/fenix/components/Components.kt | 2 +- .../org/mozilla/fenix/StrictModeManagerTest.kt | 7 +++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt b/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt index 16ffa3d79..ce47612b4 100644 --- a/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt +++ b/app/src/main/java/org/mozilla/fenix/StrictModeManager.kt @@ -10,12 +10,14 @@ package org.mozilla.fenix import android.os.Build +import android.os.Looper import android.os.StrictMode import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting.PRIVATE import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import mozilla.components.support.ktx.android.os.resetAfter +import org.mozilla.fenix.components.Components import org.mozilla.fenix.perf.Performance private const val MANUFACTURE_HUAWEI: String = "HUAWEI" @@ -24,9 +26,19 @@ private const val MANUFACTURE_ONE_PLUS: String = "OnePlus" /** * Manages strict mode settings for the application. */ -class StrictModeManager(config: Config) { +class StrictModeManager( + config: Config, + + // Ideally, we'd pass in a more specific value but there is a circular dependency: StrictMode + // is passed into Core but we'd need to pass in Core here. Instead, we take components and later + // fetch the value we need from it. + // + // Public to be accessible by inline functions. + val components: Components +) { val logger = Performance.logger // public to be accessible by inline functions. + val mainLooper = Looper.getMainLooper() // public to be accessible by inline functions. // This is public so it can be used by inline functions. val isEnabledByBuildConfig = config.channel.isDebug @@ -111,6 +123,9 @@ class StrictModeManager(config: Config) { // because it'd increase complexity. suppressionCount += 1 logger.warn("StrictMode violation suppressed: #$suppressionCount") + if (Thread.currentThread() == mainLooper.thread) { // markers only supported on main thread. + components.core.engine.profiler?.addMarker("StrictMode.suppression", "Count: $suppressionCount") + } policy.resetAfter(functionBlock) } else { 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 cbd96e20e..29b2c115c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -132,7 +132,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 strictMode by lazy { StrictModeManager(Config, this) } 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 a6a79e5dc..49ad3e280 100644 --- a/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/StrictModeManagerTest.kt @@ -21,6 +21,7 @@ import org.junit.Assert.fail import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.components.Components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) @@ -36,13 +37,15 @@ class StrictModeManagerTest { MockKAnnotations.init(this) mockkStatic(StrictMode::class) + val components: Components = mockk(relaxed = true) + // 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) + debugManager = StrictModeManager(debugConfig, components) val releaseConfig: Config = mockk { every { channel } returns ReleaseChannel.Release } - releaseManager = StrictModeManager(releaseConfig) + releaseManager = StrictModeManager(releaseConfig, components) } @After