RUM-2708: Add tests for ResourceResolver

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/SessionReplayRecorder.kt

@@ -32,14 +32,17 @@ import com.datadog.android.sessionreplay.internal.recorder.callback.OnWindowRefr
 import com.datadog.android.sessionreplay.internal.recorder.mapper.DecorViewMapper
 import com.datadog.android.sessionreplay.internal.recorder.mapper.MapperTypeWrapper
 import com.datadog.android.sessionreplay.internal.recorder.mapper.ViewWireframeMapper
+import com.datadog.android.sessionreplay.internal.recorder.resources.BitmapCachesManager
 import com.datadog.android.sessionreplay.internal.recorder.resources.BitmapPool
 import com.datadog.android.sessionreplay.internal.recorder.resources.DefaultImageWireframeHelper
 import com.datadog.android.sessionreplay.internal.recorder.resources.ImageTypeResolver
+import com.datadog.android.sessionreplay.internal.recorder.resources.MD5HashGenerator
 import com.datadog.android.sessionreplay.internal.recorder.resources.ResourceResolver
 import com.datadog.android.sessionreplay.internal.recorder.resources.ResourcesLRUCache
 import com.datadog.android.sessionreplay.internal.recorder.resources.WebPImageCompression
 import com.datadog.android.sessionreplay.internal.storage.RecordWriter
 import com.datadog.android.sessionreplay.internal.storage.ResourcesWriter
+import com.datadog.android.sessionreplay.internal.utils.DrawableUtils
 import com.datadog.android.sessionreplay.internal.utils.RumContextProvider
 import com.datadog.android.sessionreplay.internal.utils.TimeProvider
 import com.datadog.android.sessionreplay.utils.ColorStringFormatter
@@ -122,20 +125,28 @@ internal class SessionReplayRecorder : OnWindowRefreshedCallback, Recorder {
             drawableToColorMapper
         )
 
-        val resourcesSerializer = ResourceResolver.Builder(
-            applicationId = applicationId,
-            recordedDataQueueHandler = recordedDataQueueHandler,
+        val bitmapCachesManager = BitmapCachesManager(
             bitmapPool = BitmapPool(),
             resourcesLRUCache = ResourcesLRUCache(),
+            logger = internalLogger
+        )
+
+        val resourceResolver = ResourceResolver(
+            applicationId = applicationId,
+            recordedDataQueueHandler = recordedDataQueueHandler,
+            bitmapCachesManager = bitmapCachesManager,
+            drawableUtils = DrawableUtils(internalLogger, bitmapCachesManager),
+            logger = internalLogger,
+            md5HashGenerator = MD5HashGenerator(internalLogger),
             webPImageCompression = WebPImageCompression(internalLogger)
-        ).build()
+        )
 
         this.viewOnDrawInterceptor = ViewOnDrawInterceptor(
             recordedDataQueueHandler = recordedDataQueueHandler,
             SnapshotProducer(
                 DefaultImageWireframeHelper(
                     logger = internalLogger,
-                    resourceResolver = resourcesSerializer,
+                    resourceResolver = resourceResolver,
                     viewIdentifierResolver = viewIdentifierResolver,
                     viewUtilsInternal = ViewUtilsInternal(),
                     imageTypeResolver = ImageTypeResolver()

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/DefaultImageWireframeHelper.kt

@@ -24,9 +24,9 @@ import com.datadog.android.sessionreplay.utils.ImageWireframeHelper
 import com.datadog.android.sessionreplay.utils.ViewIdentifierResolver
 import java.util.Locale
 
-// This should not have a callback but it should just create a placeholder for base64Serializer
-// The resourcesSerializer dependency should be removed from here
-// TODO: RUM-0000 Remove the resourcesSerializer dependency from here
+// This should not have a callback but it should just create a placeholder for resourceResolver
+// The resourceResolver dependency should be removed from here
+// TODO: RUM-0000 Remove the resourceResolver dependency from here
 internal class DefaultImageWireframeHelper(
     private val logger: InternalLogger,
     private val resourceResolver: ResourceResolver,
@@ -114,7 +114,7 @@ internal class DefaultImageWireframeHelper(
             drawable = drawableProperties.drawable,
             drawableWidth = width,
             drawableHeight = height,
-            resourcesSerializerCallback = object : ResourcesSerializerCallback {
+            resourceResolverCallback = object : ResourceResolverCallback {
                 override fun onSuccess(resourceId: String) {
                     populateResourceIdInWireframe(resourceId, imageWireframe)
                     asyncJobStatusCallback.jobFinished()

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResolveResourceCallback.kt

@@ -7,6 +7,6 @@
 package com.datadog.android.sessionreplay.internal.recorder.resources
 
 internal interface ResolveResourceCallback {
-    fun onResolved(resourceId: String, byteArray: ByteArray)
+    fun onResolved(resourceId: String, resourceData: ByteArray)
     fun onFailed()
 }

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceItemCreationHandler.kt

@@ -20,13 +20,13 @@ internal class ResourceItemCreationHandler(
     @VisibleForTesting internal val resourceIdsSeen: MutableSet<String> =
         Collections.synchronizedSet(HashSet<String>())
 
-    internal fun queueItem(resourceId: String, byteArray: ByteArray) {
+    internal fun queueItem(resourceId: String, resourceData: ByteArray) {
         if (!resourceIdsSeen.contains(resourceId)) {
             resourceIdsSeen.add(resourceId)
 
             recordedDataQueueHandler.addResourceItem(
                 identifier = resourceId,
-                resourceData = byteArray,
+                resourceData = resourceData,
                 applicationId = applicationId
             )
         }

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolver.kt

@@ -48,7 +48,7 @@ internal class ResourceResolver(
         drawable: Drawable,
         drawableWidth: Int,
         drawableHeight: Int,
-        resourcesSerializerCallback: ResourcesSerializerCallback
+        resourceResolverCallback: ResourceResolverCallback
     ) {
         bitmapCachesManager.registerCallbacks(applicationContext)
 
@@ -57,34 +57,35 @@ internal class ResourceResolver(
         if (resourceId != null) {
             // if we got here it means we saw the bitmap before,
             // so we don't need to send the resource again
-            resourcesSerializerCallback.onSuccess(resourceId)
+            resourceResolverCallback.onSuccess(resourceId)
             return
         }
 
-        val bitmapFromDrawable = if (drawable is BitmapDrawable && shouldUseDrawableBitmap(drawable)) {
-            drawable.bitmap // cannot be null - we already checked in shouldUseDrawableBitmap
-        } else {
-            null
-        }
+        val bitmapFromDrawable =
+            if (drawable is BitmapDrawable && shouldUseDrawableBitmap(drawable)) {
+                drawable.bitmap // cannot be null - we already checked in shouldUseDrawableBitmap
+            } else {
+                null
+            }
 
         // do in the background
         threadPoolExecutor.executeSafe("resolveResourceId", logger) {
             @Suppress("ThreadSafety") // this runs inside an executor
-            createBitmapAsync(
+            createBitmap(
                 resources = resources,
                 drawable = drawable,
                 drawableWidth = drawableWidth,
                 drawableHeight = drawableHeight,
                 displayMetrics = displayMetrics,
                 bitmapFromDrawable = bitmapFromDrawable,
                 resolveResourceCallback = object : ResolveResourceCallback {
-                    override fun onResolved(resourceId: String, byteArray: ByteArray) {
-                        resourceItemCreationHandler.queueItem(resourceId, byteArray)
-                        resourcesSerializerCallback.onSuccess(resourceId)
+                    override fun onResolved(resourceId: String, resourceData: ByteArray) {
+                        resourceItemCreationHandler.queueItem(resourceId, resourceData)
+                        resourceResolverCallback.onSuccess(resourceId)
                     }
 
                     override fun onFailed() {
-                        resourcesSerializerCallback.onFailure()
+                        resourceResolverCallback.onFailure()
                     }
                 }
             )
@@ -96,7 +97,7 @@ internal class ResourceResolver(
     // region private
 
     @WorkerThread
-    private fun createBitmapAsync(
+    private fun createBitmap(
         resources: Resources,
         drawable: Drawable,
         drawableWidth: Int,
@@ -105,13 +106,14 @@ internal class ResourceResolver(
         bitmapFromDrawable: Bitmap?,
         resolveResourceCallback: ResolveResourceCallback
     ) {
-        var handledBitmap: Bitmap? = null
-        if (bitmapFromDrawable != null) {
-            handledBitmap = tryToGetBitmapFromBitmapDrawable(
+        val handledBitmap = if (bitmapFromDrawable != null) {
+            tryToGetBitmapFromBitmapDrawable(
                 drawable = drawable as BitmapDrawable,
                 bitmapFromDrawable = bitmapFromDrawable,
                 resolveResourceCallback = resolveResourceCallback
             )
+        } else {
+            null
         }
 
         if (handledBitmap == null) {
@@ -131,18 +133,18 @@ internal class ResourceResolver(
     private fun resolveResourceHash(
         drawable: Drawable,
         bitmap: Bitmap,
-        byteArray: ByteArray,
+        compressedBitmapBytes: ByteArray,
         shouldCacheBitmap: Boolean,
         resolveResourceCallback: ResolveResourceCallback
     ) {
         // failed to get image data
-        if (byteArray.isEmpty()) {
+        if (compressedBitmapBytes.isEmpty()) {
             // we are already logging the failure in webpImageCompression
             resolveResourceCallback.onFailed()
             return
         }
 
-        val resourceId = md5HashGenerator.generate(byteArray)
+        val resourceId = md5HashGenerator.generate(compressedBitmapBytes)
 
         // failed to resolve bitmap identifier
         if (resourceId == null) {
@@ -158,7 +160,7 @@ internal class ResourceResolver(
             drawable = drawable
         )
 
-        resolveResourceCallback.onResolved(resourceId, byteArray)
+        resolveResourceCallback.onResolved(resourceId, compressedBitmapBytes)
     }
 
     private fun cacheIfNecessary(
@@ -191,13 +193,19 @@ internal class ResourceResolver(
             displayMetrics = displayMetrics,
             bitmapCreationCallback = object : BitmapCreationCallback {
                 override fun onReady(bitmap: Bitmap) {
-                    val byteArray = webPImageCompression.compressBitmap(bitmap)
+                    val compressedBitmapBytes = webPImageCompression.compressBitmap(bitmap)
+
+                    // failed to compress bitmap
+                    if (compressedBitmapBytes.isEmpty()) {
+                        resolveResourceCallback.onFailed()
+                        return
+                    }
 
                     @Suppress("ThreadSafety") // this runs inside an executor
                     resolveResourceHash(
                         drawable = drawable,
                         bitmap = bitmap,
-                        byteArray = byteArray,
+                        compressedBitmapBytes = compressedBitmapBytes,
                         shouldCacheBitmap = true,
                         resolveResourceCallback = resolveResourceCallback
                     )
@@ -217,34 +225,37 @@ internal class ResourceResolver(
         bitmapFromDrawable: Bitmap,
         resolveResourceCallback: ResolveResourceCallback
     ): Bitmap? {
-        drawableUtils.createScaledBitmap(bitmapFromDrawable)?.let { scaledBitmap ->
-            val byteArray = webPImageCompression.compressBitmap(scaledBitmap)
+        val scaledBitmap = drawableUtils.createScaledBitmap(bitmapFromDrawable)
+            ?: return null
 
-            // failed to get byteArray potentially because the bitmap was recycled before imageCompression
-            // we'll now failover to attempting to create a new bitmap from the drawable
-            if (byteArray.isEmpty() && scaledBitmap.isRecycled) {
-                return null
-            }
+        val compressedBitmapBytes = webPImageCompression.compressBitmap(scaledBitmap)
 
-            /**
-             * Check whether the scaled bitmap is the same as the original.
-             * Since Bitmap.createScaledBitmap will return the original bitmap if the
-             * requested dimensions match the dimensions of the original
-             */
-            val shouldCacheBitmap = scaledBitmap != drawable.bitmap
+        // failed to get byteArray potentially because the bitmap was recycled before imageCompression
+        if (compressedBitmapBytes.isEmpty()) {
+            return null
+        }
 
-            resolveResourceHash(
-                drawable = drawable,
-                bitmap = scaledBitmap,
-                byteArray = byteArray,
-                shouldCacheBitmap = shouldCacheBitmap,
-                resolveResourceCallback = resolveResourceCallback
+        /**
+         * Check whether the scaled bitmap is the same as the original.
+         * Since Bitmap.createScaledBitmap will return the original bitmap if the
+         * requested dimensions match the dimensions of the original
+         * Add a specific check for isRecycled, because getting width/height from a recycled bitmap
+         * is undefined behavior
+         */
+        val shouldCacheBitmap = !bitmapFromDrawable.isRecycled && (
+            scaledBitmap.width < bitmapFromDrawable.width ||
+                scaledBitmap.height < bitmapFromDrawable.height
             )
 
-            return scaledBitmap
-        }
+        resolveResourceHash(
+            drawable = drawable,
+            bitmap = scaledBitmap,
+            compressedBitmapBytes = compressedBitmapBytes,
+            shouldCacheBitmap = shouldCacheBitmap,
+            resolveResourceCallback = resolveResourceCallback
+        )
 
-        return null
+        return scaledBitmap
     }
 
     private fun tryToGetResourceFromCache(
@@ -253,9 +264,9 @@ internal class ResourceResolver(
 
     private fun shouldUseDrawableBitmap(drawable: BitmapDrawable): Boolean {
         return drawable.bitmap != null &&
-                !drawable.bitmap.isRecycled &&
-                drawable.bitmap.width > 0 &&
-                drawable.bitmap.height > 0
+            !drawable.bitmap.isRecycled &&
+            drawable.bitmap.width > 0 &&
+            drawable.bitmap.height > 0
     }
 
     // endregion

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourcesSerializerCallback.kt → features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolverCallback.kt

@@ -6,7 +6,7 @@
 
 package com.datadog.android.sessionreplay.internal.recorder.resources
 
-internal interface ResourcesSerializerCallback {
+internal interface ResourceResolverCallback {
     fun onSuccess(resourceId: String)
 
     fun onFailure()

features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/DefaultImageWireframeHelperTest.kt

@@ -356,7 +356,7 @@ internal class DefaultImageWireframeHelperTest {
                 any()
             )
         ).thenAnswer {
-            val callback = it.arguments[6] as ResourcesSerializerCallback
+            val callback = it.arguments[6] as ResourceResolverCallback
             callback.onSuccess(fakeResourceId)
         }
 
@@ -390,15 +390,14 @@ internal class DefaultImageWireframeHelperTest {
         )
 
         // Then
-        val argumentCaptor = argumentCaptor<ResourcesSerializerCallback>()
         verify(mockResourceResolver).resolveResourceId(
             resources = any(),
             applicationContext = any(),
             displayMetrics = any(),
             drawable = any(),
             drawableWidth = any(),
             drawableHeight = any(),
-            resourcesSerializerCallback = argumentCaptor.capture()
+            resourceResolverCallback = any()
         )
         verify(mockAsyncJobStatusCallback).jobStarted()
         verify(mockAsyncJobStatusCallback).jobFinished()
@@ -454,7 +453,7 @@ internal class DefaultImageWireframeHelperTest {
         wireframes[0] as MobileSegment.Wireframe.ImageWireframe
 
         // Then
-        val argumentCaptor = argumentCaptor<ResourcesSerializerCallback>()
+        val argumentCaptor = argumentCaptor<ResourceResolverCallback>()
 
         verify(mockResourceResolver).resolveResourceId(
             any(),
@@ -499,7 +498,7 @@ internal class DefaultImageWireframeHelperTest {
         wireframes[0] as MobileSegment.Wireframe.ImageWireframe
 
         // Then
-        val argumentCaptor = argumentCaptor<ResourcesSerializerCallback>()
+        val argumentCaptor = argumentCaptor<ResourceResolverCallback>()
         verify(mockResourceResolver, times(2)).resolveResourceId(
             any(),
             any(),
@@ -575,7 +574,7 @@ internal class DefaultImageWireframeHelperTest {
             drawable = any(),
             drawableWidth = captor.capture(),
             drawableHeight = captor.capture(),
-            resourcesSerializerCallback = any()
+            resourceResolverCallback = any()
         )
         assertThat(captor.allValues).containsExactly(fakeViewWidth, fakeViewHeight)
     }
@@ -606,7 +605,7 @@ internal class DefaultImageWireframeHelperTest {
             drawable = any(),
             drawableWidth = captor.capture(),
             drawableHeight = captor.capture(),
-            resourcesSerializerCallback = any()
+            resourceResolverCallback = any()
 
         )
         assertThat(captor.allValues).containsExactly(fakeDrawableWidth, fakeDrawableHeight)
@@ -688,7 +687,7 @@ internal class DefaultImageWireframeHelperTest {
             drawable = any(),
             drawableWidth = any(),
             drawableHeight = any(),
-            resourcesSerializerCallback = any()
+            resourceResolverCallback = any()
         )
     }