Merge 0d44e90 into 2b0a22b

getsentry · May 17, 2023 · 8ac2eae · 8ac2eae
2 parents 2b0a22b + 0d44e90
commit 8ac2eae
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 10 deletions.
diff --git 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
@@ -317,8 +317,9 @@ public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentr
 	public fun <init> (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
@@ -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
@@ -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> 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
@@ -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
@@ -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<Window>()
         val view = mock<View>()
         val rootView = mock<View>()
+        val mainThreadChecker = mock<IMainThreadChecker>()
         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<Runnable>(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
@@ -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<Activity>()
+        val mainThreadChecker = mock<IMainThreadChecker>()
         val window = mock<Window>()
         val view = mock<View>()
         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<Runnable>(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(