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-1012 Fix RippleDrawables #1600

Merged
merged 1 commit into from
Sep 14, 2023
Merged
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
8 changes: 8 additions & 0 deletions detekt_custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ datadog:
- "android.database.sqlite.SQLiteDatabase.setTransactionSuccessful():java.lang.IllegalStateException"
- "android.graphics.Bitmap.compress(android.graphics.Bitmap.CompressFormat, kotlin.Int, java.io.OutputStream):java.lang.NullPointerException,java.lang.IllegalArgumentException"
- "android.graphics.Bitmap.createBitmap(android.util.DisplayMetrics?, kotlin.Int, kotlin.Int, android.graphics.Bitmap.Config):java.lang.IllegalArgumentException"
- "android.graphics.Bitmap.createScaledBitmap(android.graphics.Bitmap, kotlin.Int, kotlin.Int, kotlin.Boolean):java.lang.IllegalArgumentException"
- "android.graphics.Canvas.constructor(android.graphics.Bitmap):java.lang.IllegalStateException"
- "android.graphics.drawable.LayerDrawable.getDrawable(kotlin.Int):java.lang.IndexOutOfBoundsException"
- "android.net.ConnectivityManager.registerDefaultNetworkCallback(android.net.ConnectivityManager.NetworkCallback):java.lang.IllegalArgumentException,java.lang.SecurityException"
- "android.net.ConnectivityManager.unregisterNetworkCallback(android.net.ConnectivityManager.NetworkCallback):java.lang.SecurityException"
- "android.util.Base64.encodeToString(kotlin.ByteArray, kotlin.Int):java.lang.AssertionError"
Expand Down Expand Up @@ -282,6 +284,7 @@ datadog:
- "android.app.FragmentManager.unregisterFragmentLifecycleCallbacks(android.app.FragmentManager.FragmentLifecycleCallbacks)"
- "android.content.Context.createDeviceProtectedStorageContext()"
- "android.content.Context.getSystemService(kotlin.String)"
- "android.content.Context.registerComponentCallbacks(android.content.ComponentCallbacks)"
- "android.content.Context.registerReceiver(android.content.BroadcastReceiver?, android.content.IntentFilter)"
- "android.content.Context.unregisterReceiver(android.content.BroadcastReceiver)"
- "android.content.Intent.getBooleanExtra(kotlin.String, kotlin.Boolean)"
Expand Down Expand Up @@ -377,12 +380,14 @@ datadog:
# endregion
# region Android Graphics
- "android.graphics.Bitmap.recycle()"
- "android.graphics.Canvas.drawColor(kotlin.Int, android.graphics.PorterDuff.Mode)"
- "android.graphics.Color.argb(kotlin.Int, kotlin.Int, kotlin.Int, kotlin.Int)"
- "android.graphics.Color.blue(kotlin.Int)"
- "android.graphics.Color.green(kotlin.Int)"
- "android.graphics.Color.red(kotlin.Int)"
- "android.graphics.Color.rgb(kotlin.Int, kotlin.Int, kotlin.Int)"
- "android.graphics.drawable.Drawable.draw(android.graphics.Canvas)"
- "android.graphics.drawable.Drawable.ConstantState.newDrawable(android.content.res.Resources?)"
- "android.graphics.drawable.Drawable.getDrawable(kotlin.Int)"
- "android.graphics.drawable.Drawable.getPadding(android.graphics.Rect)"
- "android.graphics.drawable.Drawable.setBounds(kotlin.Int, kotlin.Int, kotlin.Int, kotlin.Int)"
Expand Down Expand Up @@ -622,6 +627,7 @@ datadog:
- "java.security.SecureRandom.constructor()"
- "java.security.SecureRandom.nextFloat()"
- "java.security.SecureRandom.nextLong()"
- "java.util.HashSet.find(kotlin.Function1)"
- "java.util.Properties.constructor()"
- "java.util.Properties.setProperty(kotlin.String, kotlin.String)"
- "java.util.UUID.constructor(kotlin.Long, kotlin.Long)"
Expand Down Expand Up @@ -653,6 +659,7 @@ datadog:
- "kotlin.Array.first(kotlin.Function1)"
- "kotlin.Array.firstOrNull(kotlin.Function1)"
- "kotlin.Array.forEach(kotlin.Function1)"
- "kotlin.Array.forEachIndexed(kotlin.Function2)"
- "kotlin.Array.joinToString(kotlin.CharSequence, kotlin.CharSequence, kotlin.CharSequence, kotlin.Int, kotlin.CharSequence, kotlin.Function1?)"
- "kotlin.Array.none(kotlin.Function1)"
- "kotlin.Array.orEmpty()"
Expand Down Expand Up @@ -856,6 +863,7 @@ datadog:
- "kotlin.Int.toFloat()"
- "kotlin.Int.toLong()"
- "kotlin.Int.and(kotlin.Int)"
- "kotlin.IntArray.joinToString(kotlin.CharSequence, kotlin.CharSequence, kotlin.CharSequence, kotlin.Int, kotlin.CharSequence, kotlin.Function1?)"
- "kotlin.IntArray.constructor(kotlin.Int)"
- "kotlin.Long.asTime()"
- "kotlin.Long.coerceIn(kotlin.Long, kotlin.Long)"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.sessionreplay.internal.recorder

import android.graphics.drawable.Drawable
import android.graphics.drawable.LayerDrawable
import com.datadog.android.api.InternalLogger

@Suppress("TooGenericExceptionCaught")
internal fun LayerDrawable.safeGetDrawable(index: Int, logger: InternalLogger = InternalLogger.UNBOUND): Drawable? {
return if (index < 0 || index >= this.numberOfLayers) {
logger.log(
level = InternalLogger.Level.ERROR,
target = InternalLogger.Target.MAINTAINER,
{ "Failed to get drawable from layer - invalid index passed: $index" }
)
null
} else {
this.getDrawable(index)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import android.graphics.drawable.DrawableContainer
import android.graphics.drawable.LayerDrawable
import androidx.annotation.VisibleForTesting
import androidx.collection.LruCache
import com.datadog.android.sessionreplay.internal.recorder.safeGetDrawable
import com.datadog.android.sessionreplay.internal.utils.CacheUtils
import com.datadog.android.sessionreplay.internal.utils.InvocationUtils

Expand Down Expand Up @@ -99,18 +100,14 @@ internal class Base64LRUCache(
}

private fun getPrefixForLayerDrawable(drawable: LayerDrawable): String {
return if (drawable.numberOfLayers > 1) {
val sb = StringBuilder()
for (index in 0 until drawable.numberOfLayers) {
val layer = drawable.getDrawable(index)
val layerHash = System.identityHashCode(layer).toString()
sb.append(layerHash)
sb.append("-")
}
"$sb"
} else {
""
val sb = StringBuilder()
for (index in 0 until drawable.numberOfLayers) {
val layer = drawable.safeGetDrawable(index)
val layerHash = System.identityHashCode(layer).toString()
sb.append(layerHash)
sb.append("-")
}
return "$sb"
}

internal companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import java.util.concurrent.RejectedExecutionException
import java.util.concurrent.ThreadPoolExecutor
import java.util.concurrent.TimeUnit

@Suppress("UndocumentedPublicClass")
@Suppress("TooManyFunctions")
internal class Base64Serializer private constructor(
private val threadPoolExecutor: ExecutorService,
private val drawableUtils: DrawableUtils,
Expand All @@ -50,45 +50,18 @@ internal class Base64Serializer private constructor(
applicationContext: Context,
displayMetrics: DisplayMetrics,
drawable: Drawable,
drawableWidth: Int,
drawableHeight: Int,
imageWireframe: MobileSegment.Wireframe.ImageWireframe
) {
registerCacheForCallbacks(applicationContext)
registerBitmapPoolForCallbacks(applicationContext)
registerCallbacks(applicationContext)

asyncImageProcessingCallback?.startProcessingImage()

var shouldCacheBitmap = false
val cachedBase64 = base64LRUCache?.get(drawable)
if (cachedBase64 != null) {
finalizeRecordedDataItem(cachedBase64, imageWireframe, asyncImageProcessingCallback)
return
}

val bitmap = if (
drawable is BitmapDrawable &&
drawable.bitmap != null &&
!drawable.bitmap.isRecycled
) {
drawable.bitmap
} else {
drawableUtils.createBitmapOfApproxSizeFromDrawable(
drawable,
displayMetrics
)?.let {
shouldCacheBitmap = true
it
}
}

if (bitmap == null) {
asyncImageProcessingCallback?.finishProcessingImage()
return
}

Runnable {
@Suppress("ThreadSafety") // this runs inside an executor
serialiseBitmap(drawable, bitmap, shouldCacheBitmap, imageWireframe, asyncImageProcessingCallback)
}.let { executeRunnable(it) }
tryToGetBase64FromCache(drawable, imageWireframe)
jonathanmos marked this conversation as resolved.
Show resolved Hide resolved
?: tryToGetBitmapFromBitmapDrawable(drawable, imageWireframe)
?: tryToDrawNewBitmap(drawable, drawableWidth, drawableHeight, displayMetrics, imageWireframe)
?: asyncImageProcessingCallback?.finishProcessingImage()
}

internal fun registerAsyncLoadingCallback(
Expand Down Expand Up @@ -172,6 +145,85 @@ internal class Base64Serializer private constructor(
return base64Result
}

@MainThread
private fun tryToDrawNewBitmap(
drawable: Drawable,
drawableWidth: Int,
drawableHeight: Int,
displayMetrics: DisplayMetrics,
imageWireframe: MobileSegment.Wireframe.ImageWireframe
): Bitmap? {
drawableUtils.createBitmapOfApproxSizeFromDrawable(
drawable,
drawableWidth,
drawableHeight,
displayMetrics
)?.let { resizedBitmap ->
serializeBitmapAsynchronously(
drawable,
bitmap = resizedBitmap,
shouldCacheBitmap = true,
imageWireframe
)
return resizedBitmap
}

return null
}

@MainThread
private fun tryToGetBitmapFromBitmapDrawable(
drawable: Drawable,
imageWireframe: MobileSegment.Wireframe.ImageWireframe
): Bitmap? {
var result: Bitmap? = null
if (shouldUseDrawableBitmap(drawable)) {
drawableUtils.createScaledBitmap(
(drawable as BitmapDrawable).bitmap
)?.let { scaledBitmap ->
val shouldCacheBitmap = scaledBitmap != drawable.bitmap

serializeBitmapAsynchronously(
drawable,
scaledBitmap,
shouldCacheBitmap,
imageWireframe
)

result = scaledBitmap
}
}
return result
}

private fun tryToGetBase64FromCache(
drawable: Drawable,
imageWireframe: MobileSegment.Wireframe.ImageWireframe
): String? {
return base64LRUCache?.get(drawable)?.let { base64String ->
finalizeRecordedDataItem(base64String, imageWireframe, asyncImageProcessingCallback)
base64String
}
}

private fun serializeBitmapAsynchronously(
drawable: Drawable,
bitmap: Bitmap,
shouldCacheBitmap: Boolean,
imageWireframe: MobileSegment.Wireframe.ImageWireframe
) {
Runnable {
@Suppress("ThreadSafety") // this runs inside an executor
serialiseBitmap(
drawable,
bitmap,
shouldCacheBitmap,
imageWireframe,
asyncImageProcessingCallback
)
}.let { executeRunnable(it) }
}

private fun finalizeRecordedDataItem(
base64String: String,
wireframe: MobileSegment.Wireframe.ImageWireframe,
Expand All @@ -197,6 +249,20 @@ internal class Base64Serializer private constructor(
}
}

private fun shouldUseDrawableBitmap(drawable: Drawable): Boolean {
return drawable is BitmapDrawable &&
drawable.bitmap != null &&
!drawable.bitmap.isRecycled &&
drawable.bitmap.width > 0 &&
drawable.bitmap.height > 0
}

@MainThread
private fun registerCallbacks(applicationContext: Context) {
registerCacheForCallbacks(applicationContext)
registerBitmapPoolForCallbacks(applicationContext)
}

// endregion

// region builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ internal class BitmapPool(
val cacheIndex = bitmapIndex.incrementAndGet()
val cacheKey = "$key-$cacheIndex"

cache.put(cacheKey, bitmap)
@Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block
bitmapPoolHelper.safeCall {
cache.put(cacheKey, bitmap)
}

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
bitmapPoolHelper.safeCall {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@
package com.datadog.android.sessionreplay.internal.recorder.base64

import android.graphics.drawable.Drawable
import android.graphics.drawable.GradientDrawable
import android.graphics.drawable.InsetDrawable
import android.graphics.drawable.LayerDrawable
import android.view.View
import android.widget.TextView
import androidx.annotation.MainThread
import androidx.annotation.VisibleForTesting
import com.datadog.android.sessionreplay.internal.recorder.MappingContext
import com.datadog.android.sessionreplay.internal.recorder.ViewUtilsInternal
import com.datadog.android.sessionreplay.internal.recorder.densityNormalized
import com.datadog.android.sessionreplay.internal.recorder.safeGetDrawable
import com.datadog.android.sessionreplay.model.MobileSegment
import com.datadog.android.sessionreplay.utils.UniqueIdentifierGenerator

Expand All @@ -37,16 +41,9 @@ internal class ImageWireframeHelper(
prefix: String = DRAWABLE_CHILD_NAME
): MobileSegment.Wireframe.ImageWireframe? {
val id = uniqueIdentifierGenerator.resolveChildUniqueIdentifier(view, prefix + currentWireframeIndex)
val drawableProperties = resolveDrawableProperties(view, drawable)

@Suppress("ComplexCondition")
if (
drawable == null ||
id == null ||
drawable.intrinsicWidth <= 0 ||
drawable.intrinsicHeight <= 0
) {
return null
}
if (id == null || !drawableProperties.isValid()) return null

val displayMetrics = view.resources.displayMetrics
val applicationContext = view.context.applicationContext
Expand All @@ -66,10 +63,13 @@ internal class ImageWireframeHelper(
isEmpty = true
)

@Suppress("UnsafeCallOnNullableType") // drawable already checked for null in isValid
base64Serializer.handleBitmap(
applicationContext = applicationContext,
displayMetrics = displayMetrics,
drawable = drawable,
drawable = drawableProperties.drawable!!,
drawableWidth = drawableProperties.drawableWidth,
drawableHeight = drawableProperties.drawableHeight,
imageWireframe = imageWireframe
)

Expand Down Expand Up @@ -129,6 +129,23 @@ internal class ImageWireframeHelper(
return result
}

private fun resolveDrawableProperties(view: View, drawable: Drawable?): DrawableProperties {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice ;)

if (drawable == null) return DrawableProperties(null, 0, 0)

return when (drawable) {
is LayerDrawable -> {
if (drawable.numberOfLayers > 0) {
resolveDrawableProperties(view, drawable.safeGetDrawable(0))
} else {
DrawableProperties(drawable, drawable.intrinsicWidth, drawable.intrinsicHeight)
jonathanmos marked this conversation as resolved.
Show resolved Hide resolved
}
}
is InsetDrawable -> resolveDrawableProperties(view, drawable.drawable)
jonathanmos marked this conversation as resolved.
Show resolved Hide resolved
is GradientDrawable -> DrawableProperties(drawable, view.width, view.height)
else -> DrawableProperties(drawable, drawable.intrinsicWidth, drawable.intrinsicHeight)
}
}

@Suppress("MagicNumber")
private fun convertIndexToCompoundDrawablePosition(compoundDrawableIndex: Int): CompoundDrawablePositions? {
return when (compoundDrawableIndex) {
Expand All @@ -147,6 +164,16 @@ internal class ImageWireframeHelper(
BOTTOM
}

private data class DrawableProperties(
val drawable: Drawable?,
val drawableWidth: Int,
val drawableHeight: Int
) {
fun isValid(): Boolean {
return drawable != null && drawableWidth > 0 && drawableHeight > 0
}
}

internal companion object {
@VisibleForTesting internal const val DRAWABLE_CHILD_NAME = "drawable"
}
Expand Down
Loading
Loading