Compress Screenshots on a background thread (#4295)

* Compress Screenshots on a background thread

* Update Changelog

* Recover APIs used by hybrid SDKs

* Recycle bitmap after compression

Author: markushi

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Compress Screenshots on a background thread ([#4295](https://github.com/getsentry/sentry-java/pull/4295))
+
 ## 7.22.5
 
 ### Fixes

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

@@ -1,17 +1,19 @@
 package io.sentry.android.core;
 
 import static io.sentry.TypeCheckHint.ANDROID_ACTIVITY;
-import static io.sentry.android.core.internal.util.ScreenshotUtils.takeScreenshot;
+import static io.sentry.android.core.internal.util.ScreenshotUtils.captureScreenshot;
 import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion;
 
 import android.app.Activity;
+import android.graphics.Bitmap;
 import io.sentry.Attachment;
 import io.sentry.EventProcessor;
 import io.sentry.Hint;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
 import io.sentry.android.core.internal.util.AndroidCurrentDateProvider;
 import io.sentry.android.core.internal.util.Debouncer;
+import io.sentry.android.core.internal.util.ScreenshotUtils;
 import io.sentry.protocol.SentryTransaction;
 import io.sentry.util.HintUtils;
 import io.sentry.util.Objects;
@@ -87,14 +89,19 @@ public ScreenshotEventProcessor(
       return event;
     }
 
-    final byte[] screenshot =
-        takeScreenshot(
+    final Bitmap screenshot =
+        captureScreenshot(
             activity, options.getMainThreadChecker(), options.getLogger(), buildInfoProvider);
     if (screenshot == null) {
       return event;
     }
 
-    hint.setScreenshot(Attachment.fromScreenshot(screenshot));
+    hint.setScreenshot(
+        Attachment.fromByteProvider(
+            () -> ScreenshotUtils.compressBitmapToPng(screenshot, options.getLogger()),
+            "screenshot.png",
+            "image/png",
+            false));
     hint.set(ANDROID_ACTIVITY, activity);
     return event;
   }

sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ScreenshotUtils.java

@@ -27,6 +27,10 @@ public class ScreenshotUtils {
 
   private static final long CAPTURE_TIMEOUT_MS = 1000;
 
+  // Used by Hybrid SDKs
+  /**
+   * @noinspection unused
+   */
   public static @Nullable byte[] takeScreenshot(
       final @NotNull Activity activity,
       final @NotNull ILogger logger,
@@ -35,12 +39,33 @@ public class ScreenshotUtils {
         activity, AndroidMainThreadChecker.getInstance(), logger, buildInfoProvider);
   }
 
+  // Used by Hybrid SDKs
   @SuppressLint("NewApi")
   public static @Nullable byte[] takeScreenshot(
       final @NotNull Activity activity,
       final @NotNull IMainThreadChecker mainThreadChecker,
       final @NotNull ILogger logger,
       final @NotNull BuildInfoProvider buildInfoProvider) {
+
+    final @Nullable Bitmap screenshot =
+        captureScreenshot(activity, mainThreadChecker, logger, buildInfoProvider);
+    return compressBitmapToPng(screenshot, logger);
+  }
+
+  public static @Nullable Bitmap captureScreenshot(
+      final @NotNull Activity activity,
+      final @NotNull ILogger logger,
+      final @NotNull BuildInfoProvider buildInfoProvider) {
+    return captureScreenshot(
+        activity, AndroidMainThreadChecker.getInstance(), logger, buildInfoProvider);
+  }
+
+  @SuppressLint("NewApi")
+  public static @Nullable Bitmap captureScreenshot(
+      final @NotNull Activity activity,
+      final @NotNull IMainThreadChecker mainThreadChecker,
+      final @NotNull ILogger logger,
+      final @NotNull BuildInfoProvider buildInfoProvider) {
     // We are keeping BuildInfoProvider param for compatibility, as it's being used by
     // cross-platform SDKs
 
@@ -72,7 +97,7 @@ public class ScreenshotUtils {
       return null;
     }
 
-    try (final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+    try {
       // ARGB_8888 -> This configuration is very flexible and offers the best quality
       final Bitmap bitmap =
           Bitmap.createBitmap(view.getWidth(), view.getHeight(), Bitmap.Config.ARGB_8888);
@@ -133,10 +158,31 @@ public class ScreenshotUtils {
           return null;
         }
       }
+      return bitmap;
+    } catch (Throwable e) {
+      logger.log(SentryLevel.ERROR, "Taking screenshot failed.", e);
+    }
+    return null;
+  }
 
+  /**
+   * Compresses the supplied Bitmap to a PNG byte array. After compression, the Bitmap will be
+   * recycled.
+   *
+   * @param bitmap The bitmap to compress
+   * @param logger the logger
+   * @return the Bitmap in PNG format, or null if the bitmap was null, recycled or compressing faile
+   */
+  public static @Nullable byte[] compressBitmapToPng(
+      final @Nullable Bitmap bitmap, final @NotNull ILogger logger) {
+    if (bitmap == null || bitmap.isRecycled()) {
+      return null;
+    }
+    try (final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
       // 0 meaning compress for small size, 100 meaning compress for max quality.
       // Some formats, like PNG which is lossless, will ignore the quality setting.
       bitmap.compress(Bitmap.CompressFormat.PNG, 0, byteArrayOutputStream);
+      bitmap.recycle();
 
       if (byteArrayOutputStream.size() <= 0) {
         logger.log(SentryLevel.DEBUG, "Screenshot is 0 bytes, not attaching the image.");
@@ -146,7 +192,7 @@ public class ScreenshotUtils {
       // screenshot png is around ~100-150 kb
       return byteArrayOutputStream.toByteArray();
     } catch (Throwable e) {
-      logger.log(SentryLevel.ERROR, "Taking screenshot failed.", e);
+      logger.log(SentryLevel.ERROR, "Compressing bitmap failed.", e);
     }
     return null;
   }

sentry-android-core/src/test/java/io/sentry/android/core/internal/util/ScreenshotUtilTest.kt

@@ -1,12 +1,14 @@
 package io.sentry.android.core.internal.util
 
 import android.app.Activity
+import android.graphics.Bitmap
 import android.os.Build
 import android.os.Bundle
 import android.view.View
 import android.view.Window
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.ILogger
+import io.sentry.NoOpLogger
 import io.sentry.android.core.BuildInfoProvider
 import junit.framework.TestCase.assertNull
 import org.junit.runner.RunWith
@@ -16,7 +18,9 @@ import org.robolectric.Robolectric.buildActivity
 import org.robolectric.annotation.Config
 import org.robolectric.shadows.ShadowPixelCopy
 import kotlin.test.Test
+import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
 
 @Config(
     shadows = [ShadowPixelCopy::class],
@@ -32,7 +36,7 @@ class ScreenshotUtilTest {
         whenever(activity.isDestroyed).thenReturn(false)
 
         val data =
-            ScreenshotUtils.takeScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
+            ScreenshotUtils.captureScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
         assertNull(data)
     }
 
@@ -44,7 +48,7 @@ class ScreenshotUtilTest {
         whenever(activity.window).thenReturn(mock<Window>())
 
         val data =
-            ScreenshotUtils.takeScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
+            ScreenshotUtils.captureScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
         assertNull(data)
     }
 
@@ -60,7 +64,7 @@ class ScreenshotUtilTest {
         whenever(window.peekDecorView()).thenReturn(decorView)
 
         val data =
-            ScreenshotUtils.takeScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
+            ScreenshotUtils.captureScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
         assertNull(data)
     }
 
@@ -81,7 +85,7 @@ class ScreenshotUtilTest {
         whenever(rootView.height).thenReturn(0)
 
         val data =
-            ScreenshotUtils.takeScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
+            ScreenshotUtils.captureScreenshot(activity, mock<ILogger>(), mock<BuildInfoProvider>())
         assertNull(data)
     }
 
@@ -94,7 +98,7 @@ class ScreenshotUtilTest {
         val buildInfoProvider = mock<BuildInfoProvider>()
         whenever(buildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.O)
 
-        val data = ScreenshotUtils.takeScreenshot(controller.get(), logger, buildInfoProvider)
+        val data = ScreenshotUtils.captureScreenshot(controller.get(), logger, buildInfoProvider)
         assertNotNull(data)
     }
 
@@ -107,9 +111,40 @@ class ScreenshotUtilTest {
         val buildInfoProvider = mock<BuildInfoProvider>()
         whenever(buildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N)
 
-        val data = ScreenshotUtils.takeScreenshot(controller.get(), logger, buildInfoProvider)
+        val data = ScreenshotUtils.captureScreenshot(controller.get(), logger, buildInfoProvider)
         assertNotNull(data)
     }
+
+    @Test
+    fun `a null bitmap compresses into null`() {
+        val bytes = ScreenshotUtils.compressBitmapToPng(null, NoOpLogger.getInstance())
+        assertNull(bytes)
+    }
+
+    @Test
+    fun `a recycled bitmap compresses into null`() {
+        val bitmap = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
+        bitmap.recycle()
+
+        val bytes = ScreenshotUtils.compressBitmapToPng(bitmap, NoOpLogger.getInstance())
+        assertNull(bytes)
+    }
+
+    @Test
+    fun `a valid bitmap compresses into a valid bytearray`() {
+        val bitmap = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
+        val bytes = ScreenshotUtils.compressBitmapToPng(bitmap, NoOpLogger.getInstance())
+        assertNotNull(bytes)
+        assertTrue(bytes.isNotEmpty())
+    }
+
+    @Test
+    fun `compressBitmapToPng recycles the supplied bitmap`() {
+        val bitmap = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
+        assertFalse(bitmap.isRecycled)
+        ScreenshotUtils.compressBitmapToPng(bitmap, NoOpLogger.getInstance())
+        assertTrue(bitmap.isRecycled)
+    }
 }
 
 class ExampleActivity : Activity() {

sentry/api/sentry.api

@@ -11,14 +11,17 @@ public final class io/sentry/Attachment {
 	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
 	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
 	public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/lang/String;)V
+	public fun <init> (Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
 	public fun <init> ([BLjava/lang/String;)V
 	public fun <init> ([BLjava/lang/String;Ljava/lang/String;)V
 	public fun <init> ([BLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
 	public fun <init> ([BLjava/lang/String;Ljava/lang/String;Z)V
+	public static fun fromByteProvider (Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Z)Lio/sentry/Attachment;
 	public static fun fromScreenshot ([B)Lio/sentry/Attachment;
 	public static fun fromThreadDump ([B)Lio/sentry/Attachment;
 	public static fun fromViewHierarchy (Lio/sentry/protocol/ViewHierarchy;)Lio/sentry/Attachment;
 	public fun getAttachmentType ()Ljava/lang/String;
+	public fun getByteProvider ()Ljava/util/concurrent/Callable;
 	public fun getBytes ()[B
 	public fun getContentType ()Ljava/lang/String;
 	public fun getFilename ()Ljava/lang/String;

sentry/src/main/java/io/sentry/Attachment.java

@@ -2,6 +2,7 @@
 
 import io.sentry.protocol.ViewHierarchy;
 import java.io.File;
+import java.util.concurrent.Callable;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -10,6 +11,7 @@ public final class Attachment {
 
   private @Nullable byte[] bytes;
   private final @Nullable JsonSerializable serializable;
+  private final @Nullable Callable<byte[]> byteProvider;
   private @Nullable String pathname;
   private final @NotNull String filename;
   private final @Nullable String contentType;
@@ -84,6 +86,7 @@ public Attachment(
       final boolean addToTransactions) {
     this.bytes = bytes;
     this.serializable = null;
+    this.byteProvider = null;
     this.filename = filename;
     this.contentType = contentType;
     this.attachmentType = attachmentType;
@@ -109,6 +112,33 @@ public Attachment(
       final boolean addToTransactions) {
     this.bytes = null;
     this.serializable = serializable;
+    this.byteProvider = null;
+    this.filename = filename;
+    this.contentType = contentType;
+    this.attachmentType = attachmentType;
+    this.addToTransactions = addToTransactions;
+  }
+
+  /**
+   * Initializes an Attachment with bytes factory, a filename, a content type, and
+   * addToTransactions.
+   *
+   * @param byteProvider A provider holding the attachment payload
+   * @param filename The name of the attachment to display in Sentry.
+   * @param contentType The content type of the attachment.
+   * @param attachmentType the attachment type.
+   * @param addToTransactions <code>true</code> if the SDK should add this attachment to every
+   *     {@link ITransaction} or set to <code>false</code> if it shouldn't.
+   */
+  public Attachment(
+      final @NotNull Callable<byte[]> byteProvider,
+      final @NotNull String filename,
+      final @Nullable String contentType,
+      final @Nullable String attachmentType,
+      final boolean addToTransactions) {
+    this.bytes = null;
+    this.serializable = null;
+    this.byteProvider = byteProvider;
     this.filename = filename;
     this.contentType = contentType;
     this.attachmentType = attachmentType;
@@ -186,6 +216,7 @@ public Attachment(
     this.pathname = pathname;
     this.filename = filename;
     this.serializable = null;
+    this.byteProvider = null;
     this.contentType = contentType;
     this.attachmentType = attachmentType;
     this.addToTransactions = addToTransactions;
@@ -212,6 +243,7 @@ public Attachment(
     this.pathname = pathname;
     this.filename = filename;
     this.serializable = null;
+    this.byteProvider = null;
     this.contentType = contentType;
     this.addToTransactions = addToTransactions;
   }
@@ -240,6 +272,7 @@ public Attachment(
     this.pathname = pathname;
     this.filename = filename;
     this.serializable = null;
+    this.byteProvider = null;
     this.contentType = contentType;
     this.addToTransactions = addToTransactions;
     this.attachmentType = attachmentType;
@@ -310,16 +343,35 @@ boolean isAddToTransactions() {
     return attachmentType;
   }
 
+  public @Nullable Callable<byte[]> getByteProvider() {
+    return byteProvider;
+  }
+
   /**
    * Creates a new Screenshot Attachment
    *
-   * @param screenshotBytes the array bytes
+   * @param screenshotBytes the array bytes of the PNG screenshot
    * @return the Attachment
    */
   public static @NotNull Attachment fromScreenshot(final byte[] screenshotBytes) {
     return new Attachment(screenshotBytes, "screenshot.png", "image/png", false);
   }
 
+  /**
+   * Creates a new Screenshot Attachment
+   *
+   * @param provider the mechanism providing the screenshot payload
+   * @return the Attachment
+   */
+  public static @NotNull Attachment fromByteProvider(
+      final @NotNull Callable<byte[]> provider,
+      final @NotNull String filename,
+      final @Nullable String contentType,
+      final boolean addToTransactions) {
+    return new Attachment(
+        provider, filename, contentType, DEFAULT_ATTACHMENT_TYPE, addToTransactions);
+  }
+
   /**
    * Creates a new View Hierarchy Attachment
    *