Fix activity holder access, return build info

krystofwoldrich · krystofwoldrich · commit 2e19e51ecf8a · 2022-11-16T15:18:15.000+01:00
diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api
@@ -122,7 +122,7 @@ public final class io/sentry/android/core/PhoneStateBreadcrumbsIntegration : io/
 }
 
 public final class io/sentry/android/core/ScreenshotEventProcessor : android/app/Application$ActivityLifecycleCallbacks, io/sentry/EventProcessor, java/io/Closeable {
-	public fun <init> (Landroid/app/Application;Lio/sentry/android/core/SentryAndroidOptions;)V
+	public fun <init> (Landroid/app/Application;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V
 	public fun close ()V
 	public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V
 	public fun onActivityDestroyed (Landroid/app/Activity;)V
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
@@ -209,7 +209,7 @@ private static void installDefaultIntegrations(
       if (isFragmentAvailable) {
         options.addIntegration(new FragmentLifecycleIntegration((Application) context, true, true));
       }
-      options.addEventProcessor(new ScreenshotEventProcessor((Application) context, options));
+      options.addEventProcessor(new ScreenshotEventProcessor((Application) context, options, buildInfoProvider));
     } else {
       options
           .getLogger()
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityHolder.java b/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityHolder.java
@@ -5,19 +5,16 @@
 import androidx.annotation.Nullable;
 import java.lang.ref.WeakReference;
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 
 @ApiStatus.Internal
 public class CurrentActivityHolder {
 
-  private static @Nullable CurrentActivityHolder instance;
+  private static @NotNull CurrentActivityHolder instance = new CurrentActivityHolder();
 
   private @Nullable WeakReference<Activity> currentActivity;
 
   public static CurrentActivityHolder getInstance() {
-    if (instance != null) {
-      return instance;
-    }
-    instance = new CurrentActivityHolder();
     return instance;
   }
 
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
@@ -30,14 +30,14 @@ public final class ScreenshotEventProcessor
 
   private final @NotNull Application application;
   private final @NotNull SentryAndroidOptions options;
-  private final @NotNull CurrentActivityHolder currentActivityHolder;
+  private final @NotNull BuildInfoProvider buildInfoProvider;
   private boolean lifecycleCallbackInstalled = true;
 
   public ScreenshotEventProcessor(
-      final @NotNull Application application, final @NotNull SentryAndroidOptions options) {
+      final @NotNull Application application, final @NotNull SentryAndroidOptions options, final @NotNull BuildInfoProvider buildInfoProvider) {
     this.application = Objects.requireNonNull(application, "Application is required");
     this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required");
-    this.currentActivityHolder = CurrentActivityHolder.getInstance();
+    this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
 
     application.registerActivityLifecycleCallbacks(this);
   }
@@ -60,24 +60,24 @@ public ScreenshotEventProcessor(
 
       return event;
     }
-    if (currentActivityHolder.getActivity() == null || HintUtils.isFromHybridSdk(hint)) {
+    if (CurrentActivityHolder.getInstance().getActivity() == null || HintUtils.isFromHybridSdk(hint)) {
       return event;
     }
 
     final byte[] screenshot =
-        takeScreenshot(currentActivityHolder.getActivity(), options.getLogger());
+        takeScreenshot(CurrentActivityHolder.getInstance().getActivity(), options.getLogger(), buildInfoProvider);
     if (screenshot == null) {
       return event;
     }
 
     hint.setScreenshot(Attachment.fromScreenshot(screenshot));
-    hint.set(ANDROID_ACTIVITY, currentActivityHolder.getActivity());
+    hint.set(ANDROID_ACTIVITY, CurrentActivityHolder.getInstance().getActivity());
     return event;
   }
 
   @Override
   public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) {
-    currentActivityHolder.setActivity(activity);
+    CurrentActivityHolder.getInstance().setActivity(activity);
   }
 
   @Override
@@ -112,17 +112,17 @@ public void onActivityDestroyed(@NonNull Activity activity) {
   public void close() throws IOException {
     if (options.isAttachScreenshot()) {
       application.unregisterActivityLifecycleCallbacks(this);
-      currentActivityHolder.clearActivity();
+      CurrentActivityHolder.getInstance().clearActivity();
     }
   }
 
   private void cleanCurrentActivity(@NonNull Activity activity) {
-    if (currentActivityHolder.getActivity() == activity) {
-      currentActivityHolder.clearActivity();
+    if (CurrentActivityHolder.getInstance().getActivity() == activity) {
+      CurrentActivityHolder.getInstance().clearActivity();
     }
   }
 
   private void setCurrentActivity(@NonNull Activity activity) {
-    currentActivityHolder.setActivity(activity);
+    CurrentActivityHolder.getInstance().setActivity(activity);
   }
 }
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ActivityUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ActivityUtils.java
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
@@ -1,27 +1,28 @@
 package io.sentry.android.core.internal.util;
 
-import static io.sentry.android.core.internal.util.ActivityUtils.isActivityValid;
-
 import android.app.Activity;
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
+import android.os.Build;
 import android.view.View;
 import androidx.annotation.Nullable;
 import io.sentry.ILogger;
 import io.sentry.SentryLevel;
+import io.sentry.android.core.BuildInfoProvider;
+
 import java.io.ByteArrayOutputStream;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 
 @ApiStatus.Internal
 public class ScreenshotUtils {
   public static @Nullable byte[] takeScreenshot(
-      final @Nullable Activity activity, final @NotNull ILogger logger) {
+      final @Nullable Activity activity, final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider) {
     if (activity == null) {
       return null;
     }
 
-    if (!isActivityValid(activity)
+    if (!isActivityValid(activity, buildInfoProvider)
         || activity.getWindow() == null
         || activity.getWindow().getDecorView() == null
         || activity.getWindow().getDecorView().getRootView() == null) {
@@ -61,4 +62,15 @@ public class ScreenshotUtils {
     }
     return null;
   }
+
+  public static boolean isActivityValid(final @Nullable Activity activity, final @NotNull BuildInfoProvider buildInfoProvider) {
+    if (activity == null) {
+      return false;
+    }
+    if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
+      return !activity.isFinishing() && !activity.isDestroyed();
+    } else {
+      return !activity.isFinishing();
+    }
+  }
 }
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
@@ -28,6 +28,7 @@ class ScreenshotEventProcessorTest {
 
     private class Fixture {
         val application = mock<Application>()
+        val buildInfo = mock<BuildInfoProvider>()
         val activity = mock<Activity>()
         val window = mock<Window>()
         val view = mock<View>()
@@ -48,7 +49,7 @@ class ScreenshotEventProcessorTest {
         fun getSut(attachScreenshot: Boolean = false): ScreenshotEventProcessor {
             options.isAttachScreenshot = attachScreenshot
 
-            return ScreenshotEventProcessor(application, options)
+            return ScreenshotEventProcessor(application, options, buildInfo)
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,7 @@ public final class io/sentry/android/core/PhoneStateBreadcrumbsIntegration : io/`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`public final class io/sentry/android/core/ScreenshotEventProcessor : android/app/Application$ActivityLifecycleCallbacks, io/sentry/EventProcessor, java/io/Closeable {`
`125`		`- public fun <init> (Landroid/app/Application;Lio/sentry/android/core/SentryAndroidOptions;)V`
	`125`	`+ public fun <init> (Landroid/app/Application;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V`
`126`	`126`	`public fun close ()V`
`127`	`127`	`public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V`
`128`	`128`	`public fun onActivityDestroyed (Landroid/app/Activity;)V`
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ private static void installDefaultIntegrations(`
`209`	`209`	`if (isFragmentAvailable) {`
`210`	`210`	`options.addIntegration(new FragmentLifecycleIntegration((Application) context, true, true));`
`211`	`211`	`}`
`212`		`- options.addEventProcessor(new ScreenshotEventProcessor((Application) context, options));`
	`212`	`+ options.addEventProcessor(new ScreenshotEventProcessor((Application) context, options, buildInfoProvider));`
`213`	`213`	`} else {`
`214`	`214`	`options`
`215`	`215`	`.getLogger()`
Original file line number	Diff line number	Diff line change
`@@ -30,14 +30,14 @@ public final class ScreenshotEventProcessor`
`30`	`30`
`31`	`31`	`private final @NotNull Application application;`
`32`	`32`	`private final @NotNull SentryAndroidOptions options;`
`33`		`- private final @NotNull CurrentActivityHolder currentActivityHolder;`
	`33`	`+ private final @NotNull BuildInfoProvider buildInfoProvider;`
`34`	`34`	`private boolean lifecycleCallbackInstalled = true;`
`35`	`35`
`36`	`36`	`public ScreenshotEventProcessor(`
`37`		`- final @NotNull Application application, final @NotNull SentryAndroidOptions options) {`
	`37`	`+ final @NotNull Application application, final @NotNull SentryAndroidOptions options, final @NotNull BuildInfoProvider buildInfoProvider) {`
`38`	`38`	`this.application = Objects.requireNonNull(application, "Application is required");`
`39`	`39`	`this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required");`
`40`		`- this.currentActivityHolder = CurrentActivityHolder.getInstance();`
	`40`	`+ this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");`
`41`	`41`
`42`	`42`	`application.registerActivityLifecycleCallbacks(this);`
`43`	`43`	`}`
`@@ -60,24 +60,24 @@ public ScreenshotEventProcessor(`
`60`	`60`
`61`	`61`	`return event;`
`62`	`62`	`}`
`63`		`- if (currentActivityHolder.getActivity() == null \|\| HintUtils.isFromHybridSdk(hint)) {`
	`63`	`+ if (CurrentActivityHolder.getInstance().getActivity() == null \|\| HintUtils.isFromHybridSdk(hint)) {`
`64`	`64`	`return event;`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`final byte[] screenshot =`
`68`		`- takeScreenshot(currentActivityHolder.getActivity(), options.getLogger());`
	`68`	`+ takeScreenshot(CurrentActivityHolder.getInstance().getActivity(), options.getLogger(), buildInfoProvider);`
`69`	`69`	`if (screenshot == null) {`
`70`	`70`	`return event;`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`hint.setScreenshot(Attachment.fromScreenshot(screenshot));`
`74`		`- hint.set(ANDROID_ACTIVITY, currentActivityHolder.getActivity());`
	`74`	`+ hint.set(ANDROID_ACTIVITY, CurrentActivityHolder.getInstance().getActivity());`
`75`	`75`	`return event;`
`76`	`76`	`}`
`77`	`77`
`78`	`78`	`@Override`
`79`	`79`	`public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) {`
`80`		`- currentActivityHolder.setActivity(activity);`
	`80`	`+ CurrentActivityHolder.getInstance().setActivity(activity);`
`81`	`81`	`}`
`82`	`82`
`83`	`83`	`@Override`
`@@ -112,17 +112,17 @@ public void onActivityDestroyed(@NonNull Activity activity) {`
`112`	`112`	`public void close() throws IOException {`
`113`	`113`	`if (options.isAttachScreenshot()) {`
`114`	`114`	`application.unregisterActivityLifecycleCallbacks(this);`
`115`		`- currentActivityHolder.clearActivity();`
	`115`	`+ CurrentActivityHolder.getInstance().clearActivity();`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`	`119`	`private void cleanCurrentActivity(@NonNull Activity activity) {`
`120`		`- if (currentActivityHolder.getActivity() == activity) {`
`121`		`- currentActivityHolder.clearActivity();`
	`120`	`+ if (CurrentActivityHolder.getInstance().getActivity() == activity) {`
	`121`	`+ CurrentActivityHolder.getInstance().clearActivity();`
`122`	`122`	`}`
`123`	`123`	`}`
`124`	`124`
`125`	`125`	`private void setCurrentActivity(@NonNull Activity activity) {`
`126`		`- currentActivityHolder.setActivity(activity);`
	`126`	`+ CurrentActivityHolder.getInstance().setActivity(activity);`
`127`	`127`	`}`
`128`	`128`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ class ScreenshotEventProcessorTest {`
`28`	`28`
`29`	`29`	`private class Fixture {`
`30`	`30`	`val application = mock<Application>()`
	`31`	`+ val buildInfo = mock<BuildInfoProvider>()`
`31`	`32`	`val activity = mock<Activity>()`
`32`	`33`	`val window = mock<Window>()`
`33`	`34`	`val view = mock<View>()`
`@@ -48,7 +49,7 @@ class ScreenshotEventProcessorTest {`
`48`	`49`	`fun getSut(attachScreenshot: Boolean = false): ScreenshotEventProcessor {`
`49`	`50`	`options.isAttachScreenshot = attachScreenshot`
`50`	`51`
`51`		`- return ScreenshotEventProcessor(application, options)`
	`52`	`+ return ScreenshotEventProcessor(application, options, buildInfo)`
`52`	`53`	`}`
`53`	`54`	`}`
`54`	`55`