diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1d09eae08..53915c1880c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Remove null keys/values before creating concurrent hashmap in order to avoid NPE ([#2708](https://github.com/getsentry/sentry-java/pull/2708)) - Exclude SentryOptions from R8/ProGuard obfuscation ([#2699](https://github.com/getsentry/sentry-java/pull/2699)) - This fixes AGP 8.+ incompatibility, where full R8 mode is enforced +- Ensure screenshots and view hierarchies are captured on the main thread ([#2712](https://github.com/getsentry/sentry-java/pull/2712)) ### Dependencies diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 512e53f2c71..d28fcfab8b3 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -317,8 +317,9 @@ public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentr public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; public static fun snapshotViewHierarchy (Landroid/app/Activity;Lio/sentry/ILogger;)Lio/sentry/protocol/ViewHierarchy; + public static fun snapshotViewHierarchy (Landroid/app/Activity;Lio/sentry/util/thread/IMainThreadChecker;Lio/sentry/ILogger;)Lio/sentry/protocol/ViewHierarchy; public static fun snapshotViewHierarchy (Landroid/view/View;)Lio/sentry/protocol/ViewHierarchy; - public static fun snapshotViewHierarchyAsData (Landroid/app/Activity;Lio/sentry/ISerializer;Lio/sentry/ILogger;)[B + public static fun snapshotViewHierarchyAsData (Landroid/app/Activity;Lio/sentry/util/thread/IMainThreadChecker;Lio/sentry/ISerializer;Lio/sentry/ILogger;)[B } public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java index 45245bb8978..85cb1347781 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java @@ -52,7 +52,9 @@ public ScreenshotEventProcessor( return event; } - final byte[] screenshot = takeScreenshot(activity, options.getLogger(), buildInfoProvider); + final byte[] screenshot = + takeScreenshot( + activity, options.getMainThreadChecker(), options.getLogger(), buildInfoProvider); if (screenshot == null) { return event; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index ba5c8bbfdb6..6c051cb7961 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -13,13 +13,18 @@ import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.android.core.internal.gestures.ViewUtils; +import io.sentry.android.core.internal.util.AndroidMainThreadChecker; import io.sentry.protocol.ViewHierarchy; import io.sentry.protocol.ViewHierarchyNode; import io.sentry.util.HintUtils; import io.sentry.util.JsonSerializationUtils; import io.sentry.util.Objects; +import io.sentry.util.thread.IMainThreadChecker; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -29,6 +34,7 @@ public final class ViewHierarchyEventProcessor implements EventProcessor, IntegrationName { private final @NotNull SentryAndroidOptions options; + private static final long CAPTURE_TIMEOUT_MS = 1000; public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required"); @@ -54,7 +60,7 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); final @Nullable ViewHierarchy viewHierarchy = - snapshotViewHierarchy(activity, options.getLogger()); + snapshotViewHierarchy(activity, options.getMainThreadChecker(), options.getLogger()); if (viewHierarchy != null) { hint.setViewHierarchy(Attachment.fromViewHierarchy(viewHierarchy)); @@ -63,10 +69,13 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) return event; } - @Nullable public static byte[] snapshotViewHierarchyAsData( - @Nullable Activity activity, @NotNull ISerializer serializer, @NotNull ILogger logger) { - @Nullable ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity, logger); + @Nullable Activity activity, + @NotNull IMainThreadChecker mainThreadChecker, + @NotNull ISerializer serializer, + @NotNull ILogger logger) { + @Nullable + ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity, mainThreadChecker, logger); if (viewHierarchy == null) { logger.log(SentryLevel.ERROR, "Could not get ViewHierarchy."); @@ -90,6 +99,14 @@ public static byte[] snapshotViewHierarchyAsData( @Nullable public static ViewHierarchy snapshotViewHierarchy( @Nullable Activity activity, @NotNull ILogger logger) { + return snapshotViewHierarchy(activity, AndroidMainThreadChecker.getInstance(), logger); + } + + @Nullable + public static ViewHierarchy snapshotViewHierarchy( + @Nullable Activity activity, + @NotNull IMainThreadChecker mainThreadChecker, + @NotNull ILogger logger) { if (activity == null) { logger.log(SentryLevel.INFO, "Missing activity for view hierarchy snapshot."); return null; @@ -108,12 +125,28 @@ public static ViewHierarchy snapshotViewHierarchy( } try { - final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(decorView); - return viewHierarchy; + if (mainThreadChecker.isMainThread()) { + return snapshotViewHierarchy(decorView); + } else { + final CountDownLatch latch = new CountDownLatch(1); + final AtomicReference viewHierarchy = new AtomicReference<>(null); + activity.runOnUiThread( + () -> { + try { + viewHierarchy.set(snapshotViewHierarchy(decorView)); + latch.countDown(); + } catch (Throwable t) { + logger.log(SentryLevel.ERROR, "Failed to process view hierarchy.", t); + } + }); + if (latch.await(CAPTURE_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + return viewHierarchy.get(); + } + } } catch (Throwable t) { logger.log(SentryLevel.ERROR, "Failed to process view hierarchy.", t); - return null; } + return null; } @NotNull diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ScreenshotUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ScreenshotUtils.java index b9a528382e8..88f3e126dee 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ScreenshotUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ScreenshotUtils.java @@ -10,16 +10,32 @@ import io.sentry.ILogger; import io.sentry.SentryLevel; import io.sentry.android.core.BuildInfoProvider; +import io.sentry.util.thread.IMainThreadChecker; import java.io.ByteArrayOutputStream; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @ApiStatus.Internal public class ScreenshotUtils { + + private static final long CAPTURE_TIMEOUT_MS = 1000; + + public static @Nullable byte[] takeScreenshot( + final @NotNull Activity activity, + final @NotNull ILogger logger, + final @NotNull BuildInfoProvider buildInfoProvider) { + return takeScreenshot( + activity, AndroidMainThreadChecker.getInstance(), logger, buildInfoProvider); + } + public static @Nullable byte[] takeScreenshot( final @NotNull Activity activity, + final @NotNull IMainThreadChecker mainThreadChecker, final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider) { + if (!isActivityValid(activity, buildInfoProvider) || activity.getWindow() == null || activity.getWindow().getDecorView() == null @@ -40,7 +56,23 @@ public class ScreenshotUtils { Bitmap.createBitmap(view.getWidth(), view.getHeight(), Bitmap.Config.ARGB_8888); final Canvas canvas = new Canvas(bitmap); - view.draw(canvas); + if (mainThreadChecker.isMainThread()) { + view.draw(canvas); + } else { + final @NotNull CountDownLatch latch = new CountDownLatch(1); + activity.runOnUiThread( + () -> { + try { + view.draw(canvas); + latch.countDown(); + } catch (Throwable e) { + logger.log(SentryLevel.ERROR, "Taking screenshot failed (view.draw).", e); + } + }); + if (!latch.await(CAPTURE_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + return null; + } + } // 0 meaning compress for small size, 100 meaning compress for max quality. // Some formats, like PNG which is lossless, will ignore the quality setting. diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt index 5fa20184b52..60d61394f21 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt @@ -10,13 +10,17 @@ import io.sentry.MainEventProcessor import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage import io.sentry.TypeCheckHint.ANDROID_ACTIVITY +import io.sentry.util.thread.IMainThreadChecker import org.junit.runner.RunWith +import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -30,6 +34,7 @@ class ScreenshotEventProcessorTest { val window = mock() val view = mock() val rootView = mock() + val mainThreadChecker = mock() val options = SentryAndroidOptions().apply { dsn = "https://key@sentry.io/proj" } @@ -41,10 +46,16 @@ class ScreenshotEventProcessorTest { whenever(view.rootView).thenReturn(rootView) whenever(window.decorView).thenReturn(view) whenever(activity.window).thenReturn(window) + whenever(activity.runOnUiThread(any())).then { + it.getArgument(0).run() + } + + whenever(mainThreadChecker.isMainThread).thenReturn(true) } fun getSut(attachScreenshot: Boolean = false): ScreenshotEventProcessor { options.isAttachScreenshot = attachScreenshot + options.mainThreadChecker = mainThreadChecker return ScreenshotEventProcessor(options, buildInfo) } @@ -156,6 +167,20 @@ class ScreenshotEventProcessorTest { } @Test + fun `when screenshot event processor is called from background thread it executes on main thread`() { + val sut = fixture.getSut(true) + whenever(fixture.mainThreadChecker.isMainThread).thenReturn(false) + + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + + val hint = Hint() + val event = fixture.mainProcessor.process(getEvent(), hint) + sut.process(event, hint) + + verify(fixture.activity).runOnUiThread(any()) + assertNotNull(hint.screenshot) + } + fun `when enabled, the feature is added to the integration list`() { SentryIntegrationPackageStorage.getInstance().clearStorage() val hint = Hint() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index a377032c9a6..c00bb08b3e5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -12,10 +12,12 @@ import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage import io.sentry.TypeCheckHint import io.sentry.protocol.SentryException +import io.sentry.util.thread.IMainThreadChecker import org.junit.runner.RunWith import org.mockito.invocation.InvocationOnMock import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.io.Writer import kotlin.test.BeforeTest @@ -44,6 +46,7 @@ class ViewHierarchyEventProcessorTest { } } val activity = mock() + val mainThreadChecker = mock() val window = mock() val view = mock() val options = SentryAndroidOptions().apply { @@ -56,12 +59,17 @@ class ViewHierarchyEventProcessorTest { whenever(window.decorView).thenReturn(view) whenever(window.peekDecorView()).thenReturn(view) whenever(activity.window).thenReturn(window) + whenever(activity.runOnUiThread(any())).then { + it.getArgument(0).run() + } + whenever(mainThreadChecker.isMainThread).thenReturn(true) CurrentActivityHolder.getInstance().setActivity(activity) } fun getSut(attachViewHierarchy: Boolean = false): ViewHierarchyEventProcessor { options.isAttachViewHierarchy = attachViewHierarchy + options.mainThreadChecker = mainThreadChecker return ViewHierarchyEventProcessor(options) } @@ -88,6 +96,7 @@ class ViewHierarchyEventProcessorTest { fun `should return a view hierarchy as byte array`() { val viewHierarchy = ViewHierarchyEventProcessor.snapshotViewHierarchyAsData( fixture.activity, + fixture.mainThreadChecker, fixture.serializer, fixture.logger ) @@ -100,6 +109,7 @@ class ViewHierarchyEventProcessorTest { fun `should return null as bytes are empty array`() { val viewHierarchy = ViewHierarchyEventProcessor.snapshotViewHierarchyAsData( fixture.activity, + fixture.mainThreadChecker, fixture.emptySerializer, fixture.logger ) @@ -149,6 +159,22 @@ class ViewHierarchyEventProcessorTest { assertNotNull(hint.viewHierarchy) } + @Test + fun `when an event errored in the background, the view hierarchy should captured on the main thread`() { + whenever(fixture.mainThreadChecker.isMainThread).thenReturn(false) + + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + verify(fixture.activity).runOnUiThread(any()) + assertNotNull(event) + assertNotNull(hint.viewHierarchy) + } + @Test fun `when an event did not error, the view hierarchy should be attached if the feature is enabled`() { val (event, hint) = fixture.process(