Skip to content

Commit b37b2eb

Browse files
romtsnclaude
andcommitted
refactor(screenshot): Per-call MaskRenderer, main-thread VH capture, and don't leak unmasked screenshots
- Use per-call MaskRenderer via try-with-resources instead of shared instance - Remove Closeable from ScreenshotEventProcessor (nothing to clean up) - Capture view hierarchy on main thread via runOnUiThread + CountDownLatch - Apply masking on the calling thread (only VH traversal needs main thread) - Return null on masking failure to avoid sending unmasked screenshots - Fix setMaskViewContainerClass to not trigger trackCustomMasking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5084ed2 commit b37b2eb

File tree

4 files changed

+122
-40
lines changed

4 files changed

+122
-40
lines changed

sentry-android-core/api/sentry-android-core.api

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,8 @@ public final class io/sentry/android/core/NetworkBreadcrumbsIntegration : io/sen
325325
public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
326326
}
327327

328-
public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor, java/io/Closeable {
328+
public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor {
329329
public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;Z)V
330-
public fun close ()V
331330
public fun getOrder ()Ljava/lang/Long;
332331
public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
333332
public fun process (Lio/sentry/protocol/SentryTransaction;Lio/sentry/Hint;)Lio/sentry/protocol/SentryTransaction;

sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java

Lines changed: 75 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@
2121
import io.sentry.protocol.SentryTransaction;
2222
import io.sentry.util.HintUtils;
2323
import io.sentry.util.Objects;
24-
import java.io.Closeable;
25-
import java.io.IOException;
24+
import java.util.concurrent.CountDownLatch;
25+
import java.util.concurrent.TimeUnit;
26+
import java.util.concurrent.atomic.AtomicReference;
2627
import org.jetbrains.annotations.ApiStatus;
2728
import org.jetbrains.annotations.NotNull;
2829
import org.jetbrains.annotations.Nullable;
@@ -32,16 +33,17 @@
3233
* captured.
3334
*/
3435
@ApiStatus.Internal
35-
public final class ScreenshotEventProcessor implements EventProcessor, Closeable {
36+
public final class ScreenshotEventProcessor implements EventProcessor {
3637

3738
private final @NotNull SentryAndroidOptions options;
3839
private final @NotNull BuildInfoProvider buildInfoProvider;
3940

4041
private final @NotNull Debouncer debouncer;
4142
private static final long DEBOUNCE_WAIT_TIME_MS = 2000;
4243
private static final int DEBOUNCE_MAX_EXECUTIONS = 3;
44+
private static final long MASKING_TIMEOUT_MS = 2000;
4345

44-
private @Nullable MaskRenderer maskRenderer = null;
46+
private final boolean isReplayAvailable;
4547

4648
public ScreenshotEventProcessor(
4749
final @NotNull SentryAndroidOptions options,
@@ -56,9 +58,7 @@ public ScreenshotEventProcessor(
5658
DEBOUNCE_WAIT_TIME_MS,
5759
DEBOUNCE_MAX_EXECUTIONS);
5860

59-
if (isReplayAvailable) {
60-
maskRenderer = new MaskRenderer();
61-
}
61+
this.isReplayAvailable = isReplayAvailable;
6262

6363
if (options.isAttachScreenshot()) {
6464
addIntegrationToSdkVersion("Screenshot");
@@ -69,7 +69,7 @@ private boolean isMaskingEnabled() {
6969
if (options.getScreenshotOptions().getMaskViewClasses().isEmpty()) {
7070
return false;
7171
}
72-
if (maskRenderer == null) {
72+
if (!isReplayAvailable) {
7373
options
7474
.getLogger()
7575
.log(SentryLevel.WARNING, "Screenshot masking requires sentry-android-replay module");
@@ -124,14 +124,13 @@ private boolean isMaskingEnabled() {
124124

125125
// Apply masking if enabled and replay module is available
126126
if (isMaskingEnabled()) {
127-
final @Nullable View rootView =
128-
activity.getWindow() != null
129-
&& activity.getWindow().peekDecorView() != null
130-
&& activity.getWindow().peekDecorView().getRootView() != null
131-
? activity.getWindow().peekDecorView().getRootView()
132-
: null;
133-
if (rootView != null) {
134-
screenshot = applyMasking(screenshot, rootView);
127+
final @Nullable ViewHierarchyNode rootNode = captureViewHierarchy(activity);
128+
if (rootNode == null) {
129+
return event;
130+
}
131+
screenshot = applyMasking(screenshot, rootNode);
132+
if (screenshot == null) {
133+
return event;
135134
}
136135
}
137136

@@ -146,11 +145,65 @@ private boolean isMaskingEnabled() {
146145
return event;
147146
}
148147

149-
private @NotNull Bitmap applyMasking(
150-
final @NotNull Bitmap screenshot, final @NotNull View rootView) {
148+
/**
149+
* Captures the view hierarchy on the main thread, since view traversal requires it. If already on
150+
* the main thread, captures directly; otherwise posts to the main thread and waits.
151+
*/
152+
private @Nullable ViewHierarchyNode captureViewHierarchy(final @NotNull Activity activity) {
153+
if (options.getThreadChecker().isMainThread()) {
154+
return buildViewHierarchy(activity);
155+
}
156+
157+
final AtomicReference<ViewHierarchyNode> result = new AtomicReference<>(null);
158+
final CountDownLatch latch = new CountDownLatch(1);
159+
160+
activity.runOnUiThread(
161+
() -> {
162+
try {
163+
result.set(buildViewHierarchy(activity));
164+
} finally {
165+
latch.countDown();
166+
}
167+
});
168+
169+
try {
170+
if (!latch.await(MASKING_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
171+
options
172+
.getLogger()
173+
.log(
174+
SentryLevel.WARNING, "Timed out waiting for view hierarchy capture on main thread");
175+
return null;
176+
}
177+
} catch (Throwable e) {
178+
options.getLogger().log(SentryLevel.ERROR, "Failed to capture view hierarchy", e);
179+
return null;
180+
}
181+
182+
return result.get();
183+
}
184+
185+
private @Nullable ViewHierarchyNode buildViewHierarchy(final @NotNull Activity activity) {
186+
final @Nullable View rootView =
187+
activity.getWindow() != null
188+
&& activity.getWindow().peekDecorView() != null
189+
&& activity.getWindow().peekDecorView().getRootView() != null
190+
? activity.getWindow().peekDecorView().getRootView()
191+
: null;
192+
if (rootView == null) {
193+
return null;
194+
}
195+
196+
final ViewHierarchyNode rootNode =
197+
ViewHierarchyNode.Companion.fromView(rootView, null, 0, options.getScreenshotOptions());
198+
ViewsKt.traverse(rootView, rootNode, options.getScreenshotOptions(), options.getLogger());
199+
return rootNode;
200+
}
201+
202+
private @Nullable Bitmap applyMasking(
203+
final @NotNull Bitmap screenshot, final @NotNull ViewHierarchyNode rootNode) {
151204
Bitmap mutableBitmap = screenshot;
152205
boolean createdCopy = false;
153-
try {
206+
try (final MaskRenderer maskRenderer = new MaskRenderer()) {
154207
// Make bitmap mutable if needed
155208
if (!screenshot.isMutable()) {
156209
mutableBitmap = screenshot.copy(Bitmap.Config.ARGB_8888, true);
@@ -160,16 +213,7 @@ private boolean isMaskingEnabled() {
160213
createdCopy = true;
161214
}
162215

163-
// we can access it here, since it's "internal" only for Kotlin
164-
165-
// Build view hierarchy and apply masks
166-
final ViewHierarchyNode rootNode =
167-
ViewHierarchyNode.Companion.fromView(rootView, null, 0, options.getScreenshotOptions());
168-
ViewsKt.traverse(rootView, rootNode, options.getScreenshotOptions(), options.getLogger());
169-
170-
if (maskRenderer != null) {
171-
maskRenderer.renderMasks(mutableBitmap, rootNode, null);
172-
}
216+
maskRenderer.renderMasks(mutableBitmap, rootNode, null);
173217

174218
// Recycle original if we created a copy
175219
if (createdCopy && !screenshot.isRecycled()) {
@@ -183,19 +227,13 @@ private boolean isMaskingEnabled() {
183227
if (createdCopy && !mutableBitmap.isRecycled()) {
184228
mutableBitmap.recycle();
185229
}
186-
return screenshot;
230+
// Don't return unmasked screenshot when masking is configured
231+
return null;
187232
}
188233
}
189234

190235
@Override
191236
public @Nullable Long getOrder() {
192237
return 10000L;
193238
}
194-
195-
@Override
196-
public void close() throws IOException {
197-
if (maskRenderer != null) {
198-
maskRenderer.close();
199-
}
200-
}
201239
}

sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,51 @@ class ScreenshotEventProcessorTest {
323323
assertNotNull(hint.screenshot)
324324
}
325325

326+
@Test
327+
fun `when masking is configured and VH capture fails, no screenshot is attached`() {
328+
val sut = fixture.getSut(attachScreenshot = true, isReplayAvailable = true)
329+
fixture.options.screenshotOptions.setMaskAllText(true)
330+
val hint = Hint()
331+
332+
// No activity set, so VH capture will return null (no rootView)
333+
CurrentActivityHolder.getInstance().clearActivity()
334+
335+
val event = fixture.mainProcessor.process(getEvent(), hint)
336+
sut.process(event, hint)
337+
338+
assertNull(hint.screenshot)
339+
}
340+
341+
@Test
342+
fun `when masking is configured but replay is not available, screenshot is still captured without masking`() {
343+
val sut = fixture.getSut(attachScreenshot = true, isReplayAvailable = false)
344+
fixture.options.screenshotOptions.setMaskAllText(true)
345+
val hint = Hint()
346+
347+
CurrentActivityHolder.getInstance().setActivity(fixture.activity)
348+
349+
val event = fixture.mainProcessor.process(getEvent(), hint)
350+
sut.process(event, hint)
351+
352+
assertNotNull(hint.screenshot)
353+
}
354+
355+
@Test
356+
fun `when masking is configured from background thread, VH is captured on main thread`() {
357+
fixture.options.screenshotOptions.setMaskAllText(true)
358+
val sut = fixture.getSut(attachScreenshot = true, isReplayAvailable = true)
359+
whenever(fixture.threadChecker.isMainThread).thenReturn(false)
360+
361+
CurrentActivityHolder.getInstance().setActivity(fixture.activity)
362+
363+
val hint = Hint()
364+
val event = fixture.mainProcessor.process(getEvent(), hint)
365+
sut.process(event, hint)
366+
367+
shadowOf(Looper.getMainLooper()).idle()
368+
assertNotNull(hint.screenshot)
369+
}
370+
326371
// region Snapshot Tests
327372

328373
@Test

sentry/src/main/java/io/sentry/SentryMaskingOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public void addUnmaskViewClass(final @NotNull String className) {
109109
}
110110

111111
public void setMaskViewContainerClass(@NotNull String containerClass) {
112-
addMaskViewClass(containerClass);
112+
maskViewClasses.add(containerClass);
113113
maskViewContainerClass = containerClass;
114114
}
115115

0 commit comments

Comments
 (0)