Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-6195: Allow ResourcesLruCache to work with any key #2418

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ internal class ChipWireframeMapper(
height = view.chipDrawable.intrinsicHeight,
usePIIPlaceholder = false,
drawable = view.chipDrawable,
customResourceIdCacheKey = null,
asyncJobStatusCallback = asyncJobStatusCallback
)
backgroundWireframe?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ import org.mockito.Mock
import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.eq
import org.mockito.kotlin.isNull
import org.mockito.kotlin.mock
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.mockito.quality.Strictness
Expand Down Expand Up @@ -176,7 +176,8 @@ class ChipWireframeMapperTest {
clipping = isNull(),
shapeStyle = isNull(),
border = isNull(),
prefix = any()
prefix = any(),
customResourceIdCacheKey = anyOrNull()
)
}

Expand Down
4 changes: 2 additions & 2 deletions features/dd-sdk-android-session-replay/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ data class com.datadog.android.sessionreplay.utils.GlobalBounds
constructor(Long, Long, Long, Long)
interface com.datadog.android.sessionreplay.utils.ImageWireframeHelper
fun createImageWireframeByBitmap(Long, GlobalBounds, android.graphics.Bitmap, Float, Boolean, com.datadog.android.sessionreplay.ImagePrivacy, AsyncJobStatusCallback, com.datadog.android.sessionreplay.model.MobileSegment.WireframeClip? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeStyle? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeBorder? = null): com.datadog.android.sessionreplay.model.MobileSegment.Wireframe?
fun createImageWireframeByDrawable(android.view.View, com.datadog.android.sessionreplay.ImagePrivacy, Int, Long, Long, Int, Int, Boolean, android.graphics.drawable.Drawable, com.datadog.android.sessionreplay.internal.recorder.resources.DrawableCopier = DefaultDrawableCopier(), AsyncJobStatusCallback, com.datadog.android.sessionreplay.model.MobileSegment.WireframeClip? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeStyle? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeBorder? = null, String? = DRAWABLE_CHILD_NAME): com.datadog.android.sessionreplay.model.MobileSegment.Wireframe?
fun createCompoundDrawableWireframes(android.widget.TextView, com.datadog.android.sessionreplay.recorder.MappingContext, Int, AsyncJobStatusCallback): MutableList<com.datadog.android.sessionreplay.model.MobileSegment.Wireframe>
fun createImageWireframeByDrawable(android.view.View, com.datadog.android.sessionreplay.ImagePrivacy, Int, Long, Long, Int, Int, Boolean, android.graphics.drawable.Drawable, com.datadog.android.sessionreplay.internal.recorder.resources.DrawableCopier = DefaultDrawableCopier(), AsyncJobStatusCallback, com.datadog.android.sessionreplay.model.MobileSegment.WireframeClip? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeStyle? = null, com.datadog.android.sessionreplay.model.MobileSegment.ShapeBorder? = null, String? = DRAWABLE_CHILD_NAME, String?): com.datadog.android.sessionreplay.model.MobileSegment.Wireframe?
fun createCompoundDrawableWireframes(android.widget.TextView, com.datadog.android.sessionreplay.recorder.MappingContext, Int, String?, AsyncJobStatusCallback): MutableList<com.datadog.android.sessionreplay.model.MobileSegment.Wireframe>
companion object
open class com.datadog.android.sessionreplay.utils.LegacyDrawableToColorMapper : DrawableToColorMapper
constructor(List<DrawableToColorMapper> = emptyList())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1584,17 +1584,17 @@ public final class com/datadog/android/sessionreplay/utils/GlobalBounds {

public abstract interface class com/datadog/android/sessionreplay/utils/ImageWireframeHelper {
public static final field Companion Lcom/datadog/android/sessionreplay/utils/ImageWireframeHelper$Companion;
public abstract fun createCompoundDrawableWireframes (Landroid/widget/TextView;Lcom/datadog/android/sessionreplay/recorder/MappingContext;ILcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;)Ljava/util/List;
public abstract fun createCompoundDrawableWireframes (Landroid/widget/TextView;Lcom/datadog/android/sessionreplay/recorder/MappingContext;ILjava/lang/String;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;)Ljava/util/List;
public abstract fun createImageWireframeByBitmap (JLcom/datadog/android/sessionreplay/utils/GlobalBounds;Landroid/graphics/Bitmap;FZLcom/datadog/android/sessionreplay/ImagePrivacy;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
public abstract fun createImageWireframeByDrawable (Landroid/view/View;Lcom/datadog/android/sessionreplay/ImagePrivacy;IJJIIZLandroid/graphics/drawable/Drawable;Lcom/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;Ljava/lang/String;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
public abstract fun createImageWireframeByDrawable (Landroid/view/View;Lcom/datadog/android/sessionreplay/ImagePrivacy;IJJIIZLandroid/graphics/drawable/Drawable;Lcom/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;Ljava/lang/String;Ljava/lang/String;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
}

public final class com/datadog/android/sessionreplay/utils/ImageWireframeHelper$Companion {
}

public final class com/datadog/android/sessionreplay/utils/ImageWireframeHelper$DefaultImpls {
public static synthetic fun createImageWireframeByBitmap$default (Lcom/datadog/android/sessionreplay/utils/ImageWireframeHelper;JLcom/datadog/android/sessionreplay/utils/GlobalBounds;Landroid/graphics/Bitmap;FZLcom/datadog/android/sessionreplay/ImagePrivacy;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;ILjava/lang/Object;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
public static synthetic fun createImageWireframeByDrawable$default (Lcom/datadog/android/sessionreplay/utils/ImageWireframeHelper;Landroid/view/View;Lcom/datadog/android/sessionreplay/ImagePrivacy;IJJIIZLandroid/graphics/drawable/Drawable;Lcom/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;Ljava/lang/String;ILjava/lang/Object;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
public static synthetic fun createImageWireframeByDrawable$default (Lcom/datadog/android/sessionreplay/utils/ImageWireframeHelper;Landroid/view/View;Lcom/datadog/android/sessionreplay/ImagePrivacy;IJJIIZLandroid/graphics/drawable/Drawable;Lcom/datadog/android/sessionreplay/internal/recorder/resources/DrawableCopier;Lcom/datadog/android/sessionreplay/utils/AsyncJobStatusCallback;Lcom/datadog/android/sessionreplay/model/MobileSegment$WireframeClip;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeStyle;Lcom/datadog/android/sessionreplay/model/MobileSegment$ShapeBorder;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lcom/datadog/android/sessionreplay/model/MobileSegment$Wireframe;
}

public class com/datadog/android/sessionreplay/utils/LegacyDrawableToColorMapper : com/datadog/android/sessionreplay/utils/DrawableToColorMapper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ internal abstract class CheckableTextViewMapper<T>(
border = null,
usePIIPlaceholder = true,
clipping = MobileSegment.WireframeClip(),
customResourceIdCacheKey = null,
asyncJobStatusCallback = asyncJobStatusCallback
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ internal open class SwitchCompatMapper(
shapeStyle = null,
border = null,
usePIIPlaceholder = true,
customResourceIdCacheKey = null,
asyncJobStatusCallback = asyncJobStatusCallback
)
}
Expand Down Expand Up @@ -141,6 +142,7 @@ internal open class SwitchCompatMapper(
border = null,
usePIIPlaceholder = true,
clipping = null,
customResourceIdCacheKey = null,
asyncJobStatusCallback = asyncJobStatusCallback
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import androidx.annotation.MainThread
import com.datadog.android.api.InternalLogger

internal class BitmapCachesManager(
private val resourcesLRUCache: Cache<Drawable, ByteArray>,
private val resourcesLRUCache: Cache<String, ByteArray>,
private val bitmapPool: BitmapPool,
private val logger: InternalLogger
) {
Expand Down Expand Up @@ -51,15 +51,19 @@ internal class BitmapCachesManager(
isBitmapPoolRegisteredForCallbacks = true
}

internal fun putInResourceCache(drawable: Drawable, resourceId: String) {
resourcesLRUCache.put(drawable, resourceId.toByteArray(Charsets.UTF_8))
internal fun putInResourceCache(key: String, resourceId: String) {
resourcesLRUCache.put(key, resourceId.toByteArray(Charsets.UTF_8))
}

internal fun getFromResourceCache(drawable: Drawable): String? {
val resourceId = resourcesLRUCache.get(drawable) ?: return null
internal fun getFromResourceCache(key: String): String? {
val resourceId = resourcesLRUCache.get(key) ?: return null
return String(resourceId, Charsets.UTF_8)
}

internal fun generateResourceKeyFromDrawable(drawable: Drawable): String? {
return (resourcesLRUCache as? ResourcesLRUCache)?.generateKeyFromDrawable(drawable)
}

internal fun putInBitmapPool(bitmap: Bitmap) {
bitmapPool.put(bitmap)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ internal class BitmapPool(
}

@Synchronized
override fun get(element: String): Bitmap? {
val bitmapsWithReqDimensions = bitmapsBySize[element] ?: return null
override fun get(key: String): Bitmap? {
val bitmapsWithReqDimensions = bitmapsBySize[key] ?: return null

// find the first unused bitmap, mark it as used and return it
return bitmapsWithReqDimensions.find {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ package com.datadog.android.sessionreplay.internal.recorder.resources
internal interface Cache<K : Any, V : Any> {

fun put(value: V) {}
fun put(element: K, value: V) {}
fun get(element: K): V? = null
fun put(key: K, value: V) {}
fun get(key: K): V? = null
fun size(): Int
fun clear()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ internal class DefaultImageWireframeHelper(
clipping: MobileSegment.WireframeClip?,
shapeStyle: MobileSegment.ShapeStyle?,
border: MobileSegment.ShapeBorder?,
prefix: String?
prefix: String?,
customResourceIdCacheKey: String?
): MobileSegment.Wireframe? {
val id = viewIdentifierResolver.resolveChildUniqueIdentifier(view, prefix + currentWireframeIndex)
val drawableProperties = resolveDrawableProperties(
Expand All @@ -125,7 +126,9 @@ internal class DefaultImageWireframeHelper(
height = height
)

if (id == null || !drawableProperties.isValid()) return null
if (id == null || !drawableProperties.isValid()) {
return null
}

val resources = view.resources

Expand Down Expand Up @@ -204,6 +207,7 @@ internal class DefaultImageWireframeHelper(
drawableCopier = drawableCopier,
drawableWidth = width,
drawableHeight = height,
customResourceIdCacheKey = customResourceIdCacheKey,
resourceResolverCallback = object : ResourceResolverCallback {
override fun onSuccess(resourceId: String) {
populateResourceIdInWireframe(resourceId, imageWireframe)
Expand All @@ -225,6 +229,7 @@ internal class DefaultImageWireframeHelper(
textView: TextView,
mappingContext: MappingContext,
prevWireframeIndex: Int,
customResourceIdCacheKey: String?,
asyncJobStatusCallback: AsyncJobStatusCallback
): MutableList<MobileSegment.Wireframe> {
val result = mutableListOf<MobileSegment.Wireframe>()
Expand Down Expand Up @@ -252,6 +257,12 @@ internal class DefaultImageWireframeHelper(
position = compoundDrawablePosition
)

val resourceCacheKey = if (customResourceIdCacheKey != null) {
"$customResourceIdCacheKey" + "_$compoundDrawableIndex"
} else {
null
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason this modification is only applied for CompoundDrawable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CompoundDrawables can have multiple drawables associated, so in the case that an override key is provided the drawables need to be differentiated in the cache but if no key is provided then it will default to the original behavior


createImageWireframeByDrawable(
view = textView,
imagePrivacy = mappingContext.imagePrivacy,
Expand All @@ -265,6 +276,7 @@ internal class DefaultImageWireframeHelper(
border = null,
usePIIPlaceholder = true,
clipping = MobileSegment.WireframeClip(),
customResourceIdCacheKey = resourceCacheKey,
asyncJobStatusCallback = asyncJobStatusCallback
)?.let { resultWireframe ->
result.add(resultWireframe)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,12 @@ internal class ResourceResolver(
drawableCopier: DrawableCopier,
drawableWidth: Int,
drawableHeight: Int,
customResourceIdCacheKey: String?,
resourceResolverCallback: ResourceResolverCallback
) {
bitmapCachesManager.registerCallbacks(applicationContext)

val resourceId = tryToGetResourceFromCache(drawable = originalDrawable)
val resourceId = tryToGetResourceFromCache(drawable = originalDrawable, key = customResourceIdCacheKey)

if (resourceId != null) {
// if we got here it means we saw the bitmap before,
Expand Down Expand Up @@ -115,6 +116,7 @@ internal class ResourceResolver(
drawableHeight = drawableHeight,
displayMetrics = displayMetrics,
bitmapFromDrawable = bitmapFromDrawable,
customResourceIdCacheKey = customResourceIdCacheKey,
resolveResourceCallback = object : ResolveResourceCallback {
override fun onResolved(resourceId: String, resourceData: ByteArray) {
resourceItemCreationHandler.queueItem(resourceId, resourceData)
Expand All @@ -141,12 +143,14 @@ internal class ResourceResolver(
drawableHeight: Int,
displayMetrics: DisplayMetrics,
bitmapFromDrawable: Bitmap?,
customResourceIdCacheKey: String?,
resolveResourceCallback: ResolveResourceCallback
) {
val handledBitmap = if (bitmapFromDrawable != null) {
tryToGetBitmapFromBitmapDrawable(
drawable = drawable as BitmapDrawable,
bitmapFromDrawable = bitmapFromDrawable,
customResourceIdCacheKey = customResourceIdCacheKey,
resolveResourceCallback = resolveResourceCallback
)
} else {
Expand All @@ -160,6 +164,7 @@ internal class ResourceResolver(
drawableWidth = drawableWidth,
drawableHeight = drawableHeight,
displayMetrics = displayMetrics,
customResourceIdCacheKey = customResourceIdCacheKey,
resolveResourceCallback = resolveResourceCallback
)
}
Expand Down Expand Up @@ -195,6 +200,7 @@ internal class ResourceResolver(
bitmap: Bitmap,
compressedBitmapBytes: ByteArray,
shouldCacheBitmap: Boolean,
customResourceIdCacheKey: String?,
resolveResourceCallback: ResolveResourceCallback
) {
// failed to get image data
Expand All @@ -217,6 +223,7 @@ internal class ResourceResolver(
shouldCacheBitmap = shouldCacheBitmap,
bitmap = bitmap,
resourceId = resourceId,
customResourceIdCacheKey = customResourceIdCacheKey,
drawable = drawable
)

Expand All @@ -227,13 +234,17 @@ internal class ResourceResolver(
shouldCacheBitmap: Boolean,
bitmap: Bitmap,
resourceId: String,
customResourceIdCacheKey: String?,
drawable: Drawable
) {
if (shouldCacheBitmap) {
bitmapCachesManager.putInBitmapPool(bitmap)
}

bitmapCachesManager.putInResourceCache(drawable, resourceId)
val key = customResourceIdCacheKey
?: bitmapCachesManager.generateResourceKeyFromDrawable(drawable)
?: return
bitmapCachesManager.putInResourceCache(key, resourceId)
}

@WorkerThread
Expand All @@ -243,6 +254,7 @@ internal class ResourceResolver(
drawableWidth: Int,
drawableHeight: Int,
displayMetrics: DisplayMetrics,
customResourceIdCacheKey: String?,
resolveResourceCallback: ResolveResourceCallback
) {
drawableUtils.createBitmapOfApproxSizeFromDrawable(
Expand All @@ -267,6 +279,7 @@ internal class ResourceResolver(
bitmap = bitmap,
compressedBitmapBytes = compressedBitmapBytes,
shouldCacheBitmap = true,
customResourceIdCacheKey = customResourceIdCacheKey,
resolveResourceCallback = resolveResourceCallback
)
}
Expand All @@ -284,6 +297,7 @@ internal class ResourceResolver(
private fun tryToGetBitmapFromBitmapDrawable(
drawable: BitmapDrawable,
bitmapFromDrawable: Bitmap,
customResourceIdCacheKey: String?,
resolveResourceCallback: ResolveResourceCallback
): Bitmap? {
val scaledBitmap = drawableUtils.createScaledBitmap(bitmapFromDrawable)
Expand Down Expand Up @@ -313,15 +327,22 @@ internal class ResourceResolver(
bitmap = scaledBitmap,
compressedBitmapBytes = compressedBitmapBytes,
shouldCacheBitmap = shouldCacheBitmap,
customResourceIdCacheKey = customResourceIdCacheKey,
resolveResourceCallback = resolveResourceCallback
)

return scaledBitmap
}

private fun tryToGetResourceFromCache(
drawable: Drawable
): String? = bitmapCachesManager.getFromResourceCache(drawable)
drawable: Drawable,
key: String?
): String? {
val cacheKey = key
?: bitmapCachesManager.generateResourceKeyFromDrawable(drawable)
?: return null
return bitmapCachesManager.getFromResourceCache(cacheKey)
}

private fun shouldUseDrawableBitmap(drawable: BitmapDrawable): Boolean {
return drawable.bitmap != null &&
Expand Down
Loading