Skip to content

Commit a83d57c

Browse files
authored
Merge pull request #2723 from DataDog/yl/fix-drawable-utils-crash
RUM-9527: Defer drawable copy to work thread in Session Replay
2 parents 87d7506 + 6ab4051 commit a83d57c

File tree

2 files changed

+71
-13
lines changed
  • features/dd-sdk-android-session-replay/src

2 files changed

+71
-13
lines changed

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -137,21 +137,24 @@ internal class ResourceResolver(
137137
return
138138
}
139139

140-
val copiedDrawable = drawableCopier.copy(originalDrawable, resources)
141-
if (copiedDrawable == null) {
142-
resourceResolverCallback.onFailure()
143-
return
144-
}
145-
146-
val bitmapFromDrawable =
147-
if (copiedDrawable is BitmapDrawable && shouldUseDrawableBitmap(copiedDrawable)) {
148-
copiedDrawable.bitmap // cannot be null - we already checked in shouldUseDrawableBitmap
149-
} else {
150-
null
151-
}
152-
153140
// do in the background
154141
threadPoolExecutor.executeSafe(RESOURCE_RESOLVER_ALIAS, logger) {
142+
// Copy the drawable on work thread in order to avoid the new MaterialShapeDrawable
143+
// instance to reuse the singleton of `ShapeAppearancePathProvider` from main thread,
144+
// causing UI thread drawing crash.
145+
val copiedDrawable = drawableCopier.copy(originalDrawable, resources)
146+
if (copiedDrawable == null) {
147+
resourceResolverCallback.onFailure()
148+
return@executeSafe
149+
}
150+
151+
val bitmapFromDrawable =
152+
if (copiedDrawable is BitmapDrawable && shouldUseDrawableBitmap(copiedDrawable)) {
153+
copiedDrawable.bitmap // cannot be null - we already checked in shouldUseDrawableBitmap
154+
} else {
155+
null
156+
}
157+
155158
createBitmapFromDrawable(
156159
drawable = originalDrawable,
157160
copiedDrawable = copiedDrawable,

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,4 +1090,59 @@ internal class ResourceResolverTest {
10901090
bitmapCreationCallback = any()
10911091
)
10921092
}
1093+
1094+
@Test
1095+
fun `M only copy the drawable in work thread W resolveResourceIdFromDrawable`() {
1096+
// Given
1097+
whenever(
1098+
mockExecutorService.execute(
1099+
any()
1100+
)
1101+
).then {
1102+
// do nothing to simulate that work thread doesn't execute the task.
1103+
mock<Future<Boolean>>()
1104+
}
1105+
1106+
// When
1107+
testedResourceResolver.resolveResourceIdFromDrawable(
1108+
resources = mockResources,
1109+
applicationContext = mockApplicationContext,
1110+
displayMetrics = mockDisplayMetrics,
1111+
originalDrawable = mockDrawable,
1112+
drawableCopier = mockDrawableCopier,
1113+
drawableWidth = mockDrawable.intrinsicWidth,
1114+
drawableHeight = mockDrawable.intrinsicHeight,
1115+
customResourceIdCacheKey = null,
1116+
resourceResolverCallback = mockSerializerCallback
1117+
)
1118+
1119+
// Then
1120+
verifyNoInteractions(mockDrawableCopier)
1121+
1122+
// Given
1123+
whenever(
1124+
mockExecutorService.execute(
1125+
any()
1126+
)
1127+
).then {
1128+
(it.arguments[0] as Runnable).run()
1129+
mock<Future<Boolean>>()
1130+
}
1131+
1132+
// When
1133+
testedResourceResolver.resolveResourceIdFromDrawable(
1134+
resources = mockResources,
1135+
applicationContext = mockApplicationContext,
1136+
displayMetrics = mockDisplayMetrics,
1137+
originalDrawable = mockDrawable,
1138+
drawableCopier = mockDrawableCopier,
1139+
drawableWidth = mockDrawable.intrinsicWidth,
1140+
drawableHeight = mockDrawable.intrinsicHeight,
1141+
customResourceIdCacheKey = null,
1142+
resourceResolverCallback = mockSerializerCallback
1143+
)
1144+
1145+
// Then
1146+
verify(mockDrawableCopier).copy(mockDrawable, mockResources)
1147+
}
10931148
}

0 commit comments

Comments
 (0)