diff --git a/app/src/main/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListener.kt b/app/src/main/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListener.kt new file mode 100644 index 0000000000..fcc7774b32 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListener.kt @@ -0,0 +1,73 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import android.os.Build +import android.os.StrictMode +import android.os.strictmode.Violation +import androidx.annotation.RequiresApi +import mozilla.components.support.base.log.logger.Logger +import org.mozilla.fenix.utils.ManufacturerCodes + +private const val FCQN_EDM_STORAGE_PROVIDER_BASE = "com.android.server.enterprise.storage.EdmStorageProviderBase" + +/** + * A [StrictMode.OnThreadViolationListener] that recreates + * [StrictMode.ThreadPolicy.Builder.penaltyDeath] but will ignore some violations. For example, + * sometimes OEMs will add code that violates StrictMode so we can ignore them here instead of + * cluttering up our code with resetAfter. + * + * This class can only be used with Android P+ so we'd have to implement workarounds if the + * violations we want to ignore affect older devices. + */ +@RequiresApi(Build.VERSION_CODES.P) +class ThreadPenaltyDeathWithIgnoresListener( + private val logger: Logger = Performance.logger +) : StrictMode.OnThreadViolationListener { + + override fun onThreadViolation(violation: Violation?) { + if (violation == null) return + + // Unfortunately, this method gets called many (~5+) times with the same violation so we end + // up logging/throwing redundantly. + if (shouldViolationBeIgnored(violation)) { + logger.debug("Ignoring StrictMode ThreadPolicy violation", violation) + } else { + penaltyDeath(violation) + } + } + + @Suppress("TooGenericExceptionThrown") // we throw what StrictMode's penaltyDeath throws. + private fun penaltyDeath(violation: Violation) { + throw RuntimeException("StrictMode ThreadPolicy violation", violation) + } + + private fun shouldViolationBeIgnored(violation: Violation): Boolean = + isSamsungLgEdmStorageProviderStartupViolation(violation) + + private fun isSamsungLgEdmStorageProviderStartupViolation(violation: Violation): Boolean { + // Root issue: https://github.com/mozilla-mobile/fenix/issues/17920 + // + // This fix may address the issues seen in this bug: + // https://github.com/mozilla-mobile/fenix/issues/15430 + // So we might be able to back out the changes made there. However, I don't have a device to + // test so I didn't bother. + // + // This issue occurs on the Galaxy S10e, Galaxy A50, Note 10, and LG G7 FIT but not the S7: + // I'm guessing it's just a problem on recent Samsungs and LGs so it's okay being in this P+ + // listener. + if (!ManufacturerCodes.isSamsung && !ManufacturerCodes.isLG) { + return false + } + + // To ignore this warning, we can inspect the stack trace. There are no parts of the + // violation stack trace that are clearly unique to this violation but + // EdmStorageProviderBase doesn't appear in Android code search so we match against it. + // This class may be used in other violations that we're capable of fixing but this + // code may ignore them. I think it's okay - we keep this code simple and if it was a serious + // issue, we'd catch it on other manufacturers. + return violation.stackTrace.any { it.className == FCQN_EDM_STORAGE_PROVIDER_BASE } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/helpers/StackTraces.kt b/app/src/test/java/org/mozilla/fenix/helpers/StackTraces.kt new file mode 100644 index 0000000000..19fc97f62b --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/helpers/StackTraces.kt @@ -0,0 +1,39 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.helpers + +/** + * A collection of test helper functions for manipulating stack traces. + */ +object StackTraces { + + /** + * Gets a stack trace from logcat output. To use this, you should remove the name of the + * Exception or "Caused by" lines causing the problem and only use the stack trace lines below + * it. See src/test/resources/EdmStorageProviderBaseLogcat.txt for an example. + */ + fun getStackTraceFromLogcat(logcatResourcePath: String): Array { + val logcat = javaClass.classLoader!!.getResource(logcatResourcePath).readText() + val lines = logcat.split('\n').filter(String::isNotBlank) + return lines.map(::logcatLineToStackTraceElement).toTypedArray() + } + + private fun logcatLineToStackTraceElement(line: String): StackTraceElement { + // Expected format: + // 02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1556) + + // Expected: android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk + val methodInfo = line.substringBefore('(').substringAfterLast(' ') + val methodName = methodInfo.substringAfterLast('.') + val declaringClass = methodInfo.substringBeforeLast('.') + + // Expected: StrictMode.java:1556 + val fileInfo = line.substringAfter('(').substringBefore(')') + val fileName = fileInfo.substringBefore(':') + val lineNumber = fileInfo.substringAfter(':').toInt() + + return StackTraceElement(declaringClass, methodName, fileName, lineNumber) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListenerTest.kt b/app/src/test/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListenerTest.kt new file mode 100644 index 0000000000..f046e406c3 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/perf/ThreadPenaltyDeathWithIgnoresListenerTest.kt @@ -0,0 +1,78 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import android.os.strictmode.Violation +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.impl.annotations.RelaxedMockK +import io.mockk.mockkObject +import io.mockk.unmockkAll +import io.mockk.verify +import mozilla.components.support.base.log.logger.Logger +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.helpers.StackTraces +import org.mozilla.fenix.utils.ManufacturerCodes + +class ThreadPenaltyDeathWithIgnoresListenerTest { + + @RelaxedMockK private lateinit var logger: Logger + private lateinit var listener: ThreadPenaltyDeathWithIgnoresListener + @MockK private lateinit var violation: Violation + private lateinit var stackTrace: Array + + @Before + fun setUp() { + MockKAnnotations.init(this) + + listener = ThreadPenaltyDeathWithIgnoresListener(logger) + + stackTrace = emptyArray() + every { violation.stackTrace } answers { stackTrace } + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test(expected = RuntimeException::class) + fun `WHEN provided an arbitrary violation that does not conflict with ignores THEN we throw an exception`() { + stackTrace = Exception().stackTrace + listener.onThreadViolation(violation) + } + + @Test + fun `GIVEN we're on a Samsung WHEN provided the EdmStorageProvider violation THEN it will be ignored and logged`() { + mockkObject(ManufacturerCodes) + every { ManufacturerCodes.isSamsung } returns true + + every { violation.stackTrace } returns getEdmStorageProviderStackTrace() + listener.onThreadViolation(violation) + + verify { logger.debug("Ignoring StrictMode ThreadPolicy violation", violation) } + } + + @Test(expected = RuntimeException::class) + fun `GIVEN we're not on a Samsung or LG WHEN provided the EdmStorageProvider violation THEN we throw an exception`() { + mockkObject(ManufacturerCodes) + every { ManufacturerCodes.isSamsung } returns false + every { ManufacturerCodes.isLG } returns false + + every { violation.stackTrace } returns getEdmStorageProviderStackTrace() + listener.onThreadViolation(violation) + } + + @Test + fun `WHEN violation is null THEN we don't throw an exception`() { + listener.onThreadViolation(null) + } + + private fun getEdmStorageProviderStackTrace() = + StackTraces.getStackTraceFromLogcat("EdmStorageProviderBaseLogcat.txt") +} diff --git a/app/src/test/resources/EdmStorageProviderBaseLogcat.txt b/app/src/test/resources/EdmStorageProviderBaseLogcat.txt new file mode 100644 index 0000000000..1eb534116c --- /dev/null +++ b/app/src/test/resources/EdmStorageProviderBaseLogcat.txt @@ -0,0 +1,20 @@ +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1556) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteConnection.applyBlockGuardPolicy(SQLiteConnection.java:1524) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteConnection.executeForCursorWindow(SQLiteConnection.java:1193) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteSession.executeForCursorWindow(SQLiteSession.java:838) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteQuery.fillWindow(SQLiteQuery.java:62) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteCursor.fillWindow(SQLiteCursor.java:165) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteCursor.getCount(SQLiteCursor.java:152) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.AbstractCursor.moveToPosition(AbstractCursor.java:232) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.AbstractCursor.moveToFirst(AbstractCursor.java:271) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProviderBase.getValues(EdmStorageProviderBase.java:388) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProviderBase.getValuesList(EdmStorageProviderBase.java:1104) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProvider.getValuesList(EdmStorageProvider.java:300) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.application.ApplicationPolicy.getApplicationStateEnabledAsUser(ApplicationPolicy.java:3745) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.application.ApplicationPolicy.getApplicationStateEnabledAsUser(ApplicationPolicy.java:3714) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.setEnabledSetting(PackageManagerService.java:26031) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.setComponentEnabledSetting(PackageManagerService.java:26007) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.content.pm.IPackageManager$Stub.onTransact(IPackageManager.java:3774) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.onTransact(PackageManagerService.java:5145) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.Binder.execTransactInternal(Binder.java:1056) +02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.Binder.execTransact(Binder.java:1029)