Merge pull request #2365 from DataDog/yl/sr-mnt/fix-checkable-image-cache

ambushwork · web-flow · commit b1d6ebb82d26 · 2024-11-04T14:52:53.000+01:00
Fix CompoundButton mapper drawable clone issue
diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckableCompoundButtonMapper.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckableCompoundButtonMapper.kt
@@ -103,10 +103,10 @@ internal abstract class CheckableCompoundButtonMapper<T : CompoundButton>(
                 null
             }
         }
-        return originCheckableDrawable?.let { cloneCheckableDrawable(view, it) }
+        return originCheckableDrawable
     }
 
-    private fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {
+    override fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {
         return drawable.constantState?.newDrawable(view.resources)?.apply {
             // Set state to make the drawable have correct tint.
             setState(view.drawableState)
diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckableTextViewMapper.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckableTextViewMapper.kt
@@ -11,6 +11,7 @@ import android.widget.Checkable
 import android.widget.TextView
 import androidx.annotation.UiThread
 import com.datadog.android.api.InternalLogger
+import com.datadog.android.sessionreplay.internal.recorder.resources.DrawableCopier
 import com.datadog.android.sessionreplay.model.MobileSegment
 import com.datadog.android.sessionreplay.recorder.MappingContext
 import com.datadog.android.sessionreplay.recorder.mapper.TextViewMapper
@@ -109,6 +110,7 @@ internal abstract class CheckableTextViewMapper<T>(
                 view,
                 mappingContext.systemInformation.screenDensity
             )
+            val drawableCopier = DrawableCopier { _, _ -> cloneCheckableDrawable(view, it) }
             mappingContext.imageWireframeHelper.createImageWireframe(
                 view = view,
                 imagePrivacy = mapInputPrivacyToImagePrivacy(mappingContext.textAndInputPrivacy),
@@ -117,6 +119,7 @@ internal abstract class CheckableTextViewMapper<T>(
                 y = checkBoxBounds.y,
                 width = it.intrinsicWidth,
                 height = it.intrinsicHeight,
+                drawableCopier = drawableCopier,
                 drawable = it,
                 shapeStyle = null,
                 border = null,
@@ -127,6 +130,8 @@ internal abstract class CheckableTextViewMapper<T>(
         }
     }
 
+    abstract fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable?
+
     @UiThread
     abstract fun getCheckableDrawable(view: T): Drawable?
 
diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckedTextViewMapper.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/CheckedTextViewMapper.kt
@@ -75,7 +75,11 @@ internal open class CheckedTextViewMapper(
 
         return (view.checkMarkDrawable?.constantState as? DrawableContainer.DrawableContainerState)?.getChild(
             checkableDrawableIndex
-        )?.constantState?.newDrawable(view.resources)?.apply {
+        )
+    }
+
+    override fun cloneCheckableDrawable(view: CheckedTextView, drawable: Drawable): Drawable? {
+        return drawable.constantState?.newDrawable(view.resources)?.apply {
             // Set state to make the drawable have correct tint according to the state.
             setState(view.drawableState)
             // Set tint list to drawable if the button has declared `checkMarkTint` attribute.
diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/SwitchCompatMapper.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/SwitchCompatMapper.kt
@@ -6,8 +6,6 @@
 
 package com.datadog.android.sessionreplay.internal.recorder.mapper
 
-import android.content.res.Resources
-import android.graphics.drawable.Drawable
 import androidx.annotation.UiThread
 import androidx.appcompat.widget.SwitchCompat
 import com.datadog.android.api.InternalLogger
@@ -75,14 +73,12 @@ internal open class SwitchCompatMapper(
         )
         return trackBounds?.let {
             val trackDrawable = view.trackDrawable
-            val drawableCopier = object : DrawableCopier {
-                override fun copy(originalDrawable: Drawable, resources: Resources): Drawable? {
-                    return originalDrawable.constantState?.newDrawable(view.resources)?.apply {
-                        setState(view.trackDrawable.state)
-                        bounds = view.trackDrawable.bounds
-                        view.trackTintList?.let {
-                            setTintList(it)
-                        }
+            val drawableCopier = DrawableCopier { originalDrawable, resources ->
+                originalDrawable.constantState?.newDrawable(resources)?.apply {
+                    setState(view.trackDrawable.state)
+                    bounds = view.trackDrawable.bounds
+                    view.trackTintList?.let { tintList ->
+                        setTintList(tintList)
                     }
                 }
             }
@@ -118,17 +114,16 @@ internal open class SwitchCompatMapper(
         )
 
         val thumbDrawable = view.thumbDrawable
-        val drawableCopier = object : DrawableCopier {
-            override fun copy(originalDrawable: Drawable, resources: Resources): Drawable? {
-                return originalDrawable.constantState?.newDrawable(view.resources)?.apply {
+        val drawableCopier =
+            DrawableCopier { originalDrawable, resources ->
+                originalDrawable.constantState?.newDrawable(resources)?.apply {
                     setState(view.thumbDrawable.state)
                     bounds = view.thumbDrawable.bounds
                     view.thumbTintList?.let {
                         setTintList(it)
                     }
                 }
             }
-        }
         return thumbBounds?.let {
             mappingContext.imageWireframeHelper.createImageWireframe(
                 view = view,
diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier.kt
@@ -14,7 +14,7 @@ import com.datadog.android.lint.InternalApi
  * Interface of copying drawable to a new one.
  */
 @InternalApi
-interface DrawableCopier {
+fun interface DrawableCopier {
 
     /**
      * Called to copy the drawable.
diff --git a/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/BaseCheckableTextViewMapperTest.kt b/features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/mapper/BaseCheckableTextViewMapperTest.kt
@@ -87,9 +87,6 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
     @Mock
     lateinit var mockConstantState: DrawableContainer.DrawableContainerState
 
-    @Mock
-    lateinit var mockCheckedConstantState: DrawableContainer.DrawableContainerState
-
     @IntForgery(min = 0, max = 0xffffff)
     var fakeCurrentTextColor: Int = 0
 
@@ -108,35 +105,27 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
     @Mock
     lateinit var mockNotCheckedDrawable: Drawable
 
-    @Mock
-    lateinit var mockClonedDrawable: Drawable
-
     @Mock
     lateinit var mockDrawableCopier: DrawableCopier
 
     @IntForgery
-    var mockCloneDrawableIntrinsicHeight: Int = 0
+    var fakeDrawableIntrinsicHeight: Int = 0
 
     @IntForgery
-    var mockCloneDrawableIntrinsicWidth: Int = 0
+    var fakeDrawableIntrinsicWidth: Int = 0
 
     @Forgery
     lateinit var fakeImagePrivacy: ImagePrivacy
 
     @BeforeEach
     fun `set up`() {
-        mockClonedDrawable = mock {
-            whenever(it.intrinsicHeight) doReturn mockCloneDrawableIntrinsicHeight
-            whenever(it.intrinsicWidth) doReturn mockCloneDrawableIntrinsicWidth
-        }
-        mockCheckedConstantState = mock {
-            whenever(it.newDrawable(mockResources)) doReturn mockClonedDrawable
-        }
         mockCheckedDrawable = mock {
-            whenever(it.constantState) doReturn mockCheckedConstantState
+            whenever(it.intrinsicHeight) doReturn fakeDrawableIntrinsicHeight
+            whenever(it.intrinsicWidth) doReturn fakeDrawableIntrinsicWidth
         }
         mockNotCheckedDrawable = mock {
-            whenever(it.constantState) doReturn mockCheckedConstantState
+            whenever(it.intrinsicHeight) doReturn fakeDrawableIntrinsicHeight
+            whenever(it.intrinsicWidth) doReturn fakeDrawableIntrinsicWidth
         }
         mockConstantState = mock {
             whenever(it.getChild(CHECK_BOX_CHECKED_DRAWABLE_INDEX)) doReturn mockCheckedDrawable
@@ -226,8 +215,8 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
             currentWireframeIndex = anyInt(),
             x = eq(expectedX),
             y = eq(expectedY),
-            width = eq(mockCloneDrawableIntrinsicWidth),
-            height = eq(mockCloneDrawableIntrinsicHeight),
+            width = eq(fakeDrawableIntrinsicWidth),
+            height = eq(fakeDrawableIntrinsicHeight),
             usePIIPlaceholder = anyBoolean(),
             drawable = any(),
             drawableCopier = any(),
@@ -270,10 +259,10 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
             currentWireframeIndex = anyInt(),
             x = eq(expectedX),
             y = eq(expectedY),
-            width = eq(mockCloneDrawableIntrinsicWidth),
-            height = eq(mockCloneDrawableIntrinsicHeight),
+            width = eq(fakeDrawableIntrinsicWidth),
+            height = eq(fakeDrawableIntrinsicHeight),
             usePIIPlaceholder = anyBoolean(),
-            drawable = eq(mockClonedDrawable),
+            drawable = eq(mockNotCheckedDrawable),
             drawableCopier = any(),
             asyncJobStatusCallback = eq(mockAsyncJobStatusCallback),
             clipping = eq(MobileSegment.WireframeClip()),

Original file line number	Diff line number	Diff line change
`@@ -103,10 +103,10 @@ internal abstract class CheckableCompoundButtonMapper<T : CompoundButton>(`
`103`	`103`	`null`
`104`	`104`	`}`
`105`	`105`	`}`
`106`		`- return originCheckableDrawable?.let { cloneCheckableDrawable(view, it) }`
	`106`	`+ return originCheckableDrawable`
`107`	`107`	`}`
`108`	`108`
`109`		`- private fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {`
	`109`	`+ override fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {`
`110`	`110`	`return drawable.constantState?.newDrawable(view.resources)?.apply {`
`111`	`111`	`// Set state to make the drawable have correct tint.`
`112`	`112`	`setState(view.drawableState)`