commit 8b8cd08d51ed623210d5ca422752a0d6573766f1
parent 22f946ca7e75cf58dbb359d28c9d3299c12a74d8
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Tue, 28 Oct 2025 11:35:24 +0000
Bug 1996568 - Abstract display unit conversion for better testability. r=android-reviewers,avirvara
This patch introduces a `DisplayUnitConverter` interface to abstract away the direct dependency on Android's `DisplayMetrics` for converting dp to px. This change makes classes like `TabSheetBehaviorManager` easier to test in a non-Android environment.
The `AndroidDisplayUnitConverter` is provided as the production implementation. `TabSheetBehaviorManager` and `TabsTrayFragment` are updated to use this new abstraction. Unit tests for `TabSheetBehaviorManager` are updated to use a test implementation of the converter, removing the need to mock static Android framework methods.
Differential Revision: https://phabricator.services.mozilla.com/D270195
Diffstat:
4 files changed, 117 insertions(+), 127 deletions(-)
diff --git a/mobile/android/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/util/DisplayMetrics.kt b/mobile/android/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/util/DisplayMetrics.kt
@@ -35,3 +35,31 @@ fun Float.spToPx(displayMetrics: DisplayMetrics) = TypedValue.applyDimension(
this,
displayMetrics,
)
+
+/**
+ * An interface for converting display units (like dp) to pixels.
+ * */
+interface DisplayUnitConverter {
+ /**
+ * Converts a value in density independent pixels (dp) to the actual pixel value for the display.
+ *
+ * @param dp The value in dp.
+ * @return The value in pixels.
+ */
+ fun dpToPx(dp: Int): Int
+}
+
+/**
+ * A [DisplayUnitConverter] that uses the Android framework to perform the conversion.
+ *
+ * It requires a [DisplayMetrics] instance to correctly calculate the pixel values.
+ *
+ * @param displayMetrics The [DisplayMetrics] of the current display.
+ */
+class AndroidDisplayUnitConverter(
+ private val displayMetrics: DisplayMetrics,
+) : DisplayUnitConverter {
+ override fun dpToPx(dp: Int): Int {
+ return dp.dpToPx(displayMetrics)
+ }
+}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabSheetBehaviorManager.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabSheetBehaviorManager.kt
@@ -5,13 +5,12 @@
package org.mozilla.fenix.tabstray
import android.content.res.Configuration
-import android.util.DisplayMetrics
import android.view.View
import androidx.annotation.VisibleForTesting
import androidx.core.view.ViewCompat
import androidx.core.view.WindowInsetsCompat
import com.google.android.material.bottomsheet.BottomSheetBehavior
-import mozilla.components.support.ktx.android.util.dpToPx
+import mozilla.components.support.ktx.android.util.DisplayUnitConverter
@VisibleForTesting internal const val EXPANDED_OFFSET_IN_LANDSCAPE_DP = 0
@@ -34,14 +33,14 @@ private const val DIM_CONVERSION = 1000f
* @param orientation current Configuration.ORIENTATION_* of the device.
* @param maxNumberOfTabs highest number of tabs in each tray page.
* @param numberForExpandingTray limit depending on which the tray should be collapsed or expanded.
- * @param displayMetrics [DisplayMetrics] used for adapting resources to the current display.
+ * @param displayUnitConverter [DisplayUnitConverter] used for adapting resources to the current display.
*/
internal class TabSheetBehaviorManager(
private val behavior: BottomSheetBehavior<out View>,
orientation: Int,
private val maxNumberOfTabs: Int,
private val numberForExpandingTray: Int,
- private val displayMetrics: DisplayMetrics,
+ private val displayUnitConverter: DisplayUnitConverter,
) {
@VisibleForTesting
internal var currentOrientation = orientation
@@ -77,9 +76,9 @@ internal class TabSheetBehaviorManager(
@VisibleForTesting
internal fun updateBehaviorExpandedOffset(isLandscape: Boolean) {
behavior.expandedOffset = if (isLandscape) {
- EXPANDED_OFFSET_IN_LANDSCAPE_DP.dpToPx(displayMetrics)
+ displayUnitConverter.dpToPx(EXPANDED_OFFSET_IN_LANDSCAPE_DP)
} else {
- EXPANDED_OFFSET_IN_PORTRAIT_DP.dpToPx(displayMetrics)
+ displayUnitConverter.dpToPx(EXPANDED_OFFSET_IN_PORTRAIT_DP)
}
}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt
@@ -41,6 +41,7 @@ import mozilla.components.feature.downloads.ui.DownloadCancelDialogFragment
import mozilla.components.feature.tabs.tabstray.TabsFeature
import mozilla.components.lib.state.ext.observeAsState
import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
+import mozilla.components.support.ktx.android.util.AndroidDisplayUnitConverter
import mozilla.telemetry.glean.private.NoExtras
import org.mozilla.fenix.Config
import org.mozilla.fenix.GleanMetrics.PrivateBrowsingLocked
@@ -474,7 +475,9 @@ class TabsTrayFragment : AppCompatDialogFragment() {
} else {
EXPAND_AT_LIST_SIZE
},
- displayMetrics = requireContext().resources.displayMetrics,
+ displayUnitConverter = AndroidDisplayUnitConverter(
+ requireContext().resources.displayMetrics,
+ ),
)
setupBackgroundDismissalListener {
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/TabSheetBehaviorManagerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/TabSheetBehaviorManagerTest.kt
@@ -5,7 +5,6 @@
package org.mozilla.fenix.tabstray
import android.content.res.Configuration
-import android.util.DisplayMetrics
import android.view.View
import androidx.constraintlayout.widget.ConstraintLayout
import com.google.android.material.bottomsheet.BottomSheetBehavior
@@ -19,11 +18,9 @@ import com.google.android.material.floatingactionbutton.ExtendedFloatingActionBu
import io.mockk.Called
import io.mockk.every
import io.mockk.mockk
-import io.mockk.mockkStatic
import io.mockk.spyk
-import io.mockk.unmockkStatic
import io.mockk.verify
-import mozilla.components.support.ktx.android.util.dpToPx
+import mozilla.components.support.ktx.android.util.DisplayUnitConverter
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
@@ -32,6 +29,8 @@ import org.junit.Test
class TabSheetBehaviorManagerTest {
+ private val testDisplayUnitConverter = TestDisplayUnitConverter(1)
+
@Test
fun `WHEN state is hidden THEN invoke interactor`() {
val interactor = mockk<NavigationInteractor>(relaxed = true)
@@ -401,7 +400,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(Configuration.ORIENTATION_UNDEFINED, manager0.currentOrientation)
@@ -410,7 +409,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(Configuration.ORIENTATION_PORTRAIT, manager1.currentOrientation)
@@ -419,7 +418,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_LANDSCAPE,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(Configuration.ORIENTATION_LANDSCAPE, manager2.currentOrientation)
}
@@ -433,7 +432,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -448,7 +447,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
5,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -463,7 +462,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_COLLAPSED, behavior.state)
@@ -478,7 +477,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -493,7 +492,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -508,7 +507,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_COLLAPSED, behavior.state)
@@ -523,7 +522,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_LANDSCAPE,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -538,7 +537,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_LANDSCAPE,
5,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -553,7 +552,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_LANDSCAPE,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
)
assertEquals(STATE_EXPANDED, behavior.state)
@@ -567,7 +566,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(false)
@@ -583,7 +582,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
5,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(false)
@@ -599,7 +598,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(false)
@@ -615,7 +614,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
4,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(true)
@@ -631,7 +630,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
5,
5,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(true)
@@ -647,7 +646,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_UNDEFINED,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
)
manager.updateBehaviorState(true)
@@ -663,7 +662,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
),
)
@@ -682,7 +681,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
),
)
@@ -705,7 +704,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
),
)
@@ -720,7 +719,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
),
)
@@ -735,7 +734,7 @@ class TabSheetBehaviorManagerTest {
Configuration.ORIENTATION_PORTRAIT,
4,
5,
- mockk(),
+ testDisplayUnitConverter,
),
)
@@ -747,22 +746,14 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
-
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_LANDSCAPE_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_LANDSCAPE_DP
- TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_LANDSCAPE,
- 5,
- 4,
- displayMetrics,
- )
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_LANDSCAPE,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
assertEquals(0, behavior.expandedOffset)
}
@@ -772,22 +763,14 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_PORTRAIT_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_PORTRAIT_DP
-
- TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_PORTRAIT,
- 5,
- 4,
- displayMetrics,
- )
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_PORTRAIT,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
assertEquals(40, behavior.expandedOffset)
}
@@ -797,22 +780,14 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
-
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_PORTRAIT_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_PORTRAIT_DP
- TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_UNDEFINED,
- 5,
- 4,
- displayMetrics,
- )
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_UNDEFINED,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
assertEquals(40, behavior.expandedOffset)
}
@@ -822,23 +797,16 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
-
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_PORTRAIT_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_PORTRAIT_DP
- val manager = TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_LANDSCAPE,
- 5,
- 4,
- displayMetrics,
- )
- manager.updateDependingOnOrientation(Configuration.ORIENTATION_PORTRAIT)
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ val manager = TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_LANDSCAPE,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
+
+ manager.updateDependingOnOrientation(Configuration.ORIENTATION_PORTRAIT)
assertEquals(40, behavior.expandedOffset)
}
@@ -848,23 +816,16 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
-
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_PORTRAIT_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_PORTRAIT_DP
- val manager = TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_LANDSCAPE,
- 5,
- 4,
- displayMetrics,
- )
- manager.updateDependingOnOrientation(Configuration.ORIENTATION_UNDEFINED)
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ val manager = TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_LANDSCAPE,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
+
+ manager.updateDependingOnOrientation(Configuration.ORIENTATION_UNDEFINED)
assertEquals(40, behavior.expandedOffset)
}
@@ -874,24 +835,23 @@ class TabSheetBehaviorManagerTest {
val behavior = BottomSheetBehavior<ConstraintLayout>()
// expandedOffset is only used if isFitToContents == false
behavior.isFitToContents = false
- val displayMetrics: DisplayMetrics = mockk()
-
- try {
- mockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- every { EXPANDED_OFFSET_IN_LANDSCAPE_DP.dpToPx(displayMetrics) } returns EXPANDED_OFFSET_IN_LANDSCAPE_DP
- val manager = TabSheetBehaviorManager(
- behavior,
- Configuration.ORIENTATION_UNDEFINED,
- 5,
- 4,
- displayMetrics,
- )
- manager.updateDependingOnOrientation(Configuration.ORIENTATION_LANDSCAPE)
- } finally {
- unmockkStatic("mozilla.components.support.ktx.android.util.DisplayMetricsKt")
- }
+ val manager = TabSheetBehaviorManager(
+ behavior,
+ Configuration.ORIENTATION_UNDEFINED,
+ 5,
+ 4,
+ testDisplayUnitConverter,
+ )
+
+ manager.updateDependingOnOrientation(Configuration.ORIENTATION_LANDSCAPE)
assertEquals(0, behavior.expandedOffset)
}
+
+ class TestDisplayUnitConverter(private val conversionDensity: Int) : DisplayUnitConverter {
+ override fun dpToPx(dp: Int): Int {
+ return dp * conversionDensity
+ }
+ }
}
