Fix: Remove onActivityPreCreated call in favor of onActivityCreated (#…

…1661) (#1661)
getsentry · Aug 19, 2021 · 41654d8 · 41654d8
1 parent bcdb061
commit 41654d8
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* Fix: Remove onActivityPreCreated call in favor of onActivityCreated (#1661)
 * Fix: Do not crash if SENSOR_SERVICE throws (#1655)
 * Feat: Add support for async methods in Spring MVC (#1652)
 * Feat: Add secondary constructor taking IHub to SentryOkHttpInterceptor (#1657)

diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api
@@ -5,7 +5,6 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
 	public fun onActivityDestroyed (Landroid/app/Activity;)V
 	public fun onActivityPaused (Landroid/app/Activity;)V
 	public fun onActivityPostResumed (Landroid/app/Activity;)V
-	public fun onActivityPreCreated (Landroid/app/Activity;Landroid/os/Bundle;)V
 	public fun onActivityResumed (Landroid/app/Activity;)V
 	public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V
 	public fun onActivityStarted (Landroid/app/Activity;)V

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java
@@ -224,40 +224,14 @@ private void finishTransaction(final @Nullable ITransaction transaction) {
     }
   }
 
-  @Override
-  public synchronized void onActivityPreCreated(
-      final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) {
-
-    // only executed if API >= 29 otherwise it happens on onActivityCreated
-    if (isAllActivityCallbacksAvailable) {
-      // start collecting frame metrics for transaction
-      activityFramesTracker.addActivity(activity);
-
-      setColdStart(savedInstanceState);
-
-      // if activity has global fields being init. and
-      // they are slow, this won't count the whole fields/ctor initialization time, but only
-      // when onCreate is actually called.
-      startTracing(activity);
-    }
-  }
-
   @Override
   public synchronized void onActivityCreated(
       final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) {
-    if (!isAllActivityCallbacksAvailable) {
-      // start collecting frame metrics for transaction
-      activityFramesTracker.addActivity(activity);
-
-      setColdStart(savedInstanceState);
-    }
-
+    activityFramesTracker.addActivity(activity);
+    setColdStart(savedInstanceState);
     addBreadcrumb(activity, "created");
+    startTracing(activity);
 
-    // fallback call for API < 29 compatibility, otherwise it happens on onActivityPreCreated
-    if (!isAllActivityCallbacksAvailable) {
-      startTracing(activity);
-    }
     firstActivityCreated = true;
   }
 

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt
@@ -229,7 +229,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.hub, never()).startTransaction(any(), any(), anyOrNull(), any(), any())
     }
@@ -241,8 +241,8 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.hub).startTransaction(any(), any(), anyOrNull(), any(), any())
     }
@@ -256,7 +256,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime()
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.hub).startTransaction(any(), check {
             assertEquals("ui.load", it)
@@ -270,7 +270,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.activityFramesTracker).addActivity(eq(activity))
     }
@@ -284,7 +284,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime()
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.hub).startTransaction(check {
             assertEquals("Activity", it)
@@ -307,7 +307,7 @@ class ActivityLifecycleIntegrationTest {
         }
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
     }
 
     @Test
@@ -328,7 +328,7 @@ class ActivityLifecycleIntegrationTest {
         }
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
     }
 
     @Test
@@ -338,7 +338,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityPostResumed(activity)
 
         verify(fixture.hub).captureTransaction(check {
@@ -353,7 +353,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         fixture.transaction.status = SpanStatus.UNKNOWN_ERROR
 
@@ -372,7 +372,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityPostResumed(activity)
 
         verify(fixture.hub, never()).captureTransaction(any())
@@ -396,7 +396,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityDestroyed(activity)
 
         verify(fixture.hub).captureTransaction(any())
@@ -409,7 +409,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         assertFalse(sut.activitiesWithOngoingTransactions.isEmpty())
     }
@@ -421,7 +421,7 @@ class ActivityLifecycleIntegrationTest {
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityDestroyed(activity)
 
         assertTrue(sut.activitiesWithOngoingTransactions.isEmpty())
@@ -433,32 +433,20 @@ class ActivityLifecycleIntegrationTest {
         fixture.options.tracesSampleRate = 1.0
         sut.register(fixture.hub, fixture.options)
 
-        sut.onActivityPreCreated(mock(), mock())
+        sut.onActivityCreated(mock(), mock())
 
-        sut.onActivityPreCreated(mock(), fixture.bundle)
+        sut.onActivityCreated(mock(), fixture.bundle)
         verify(fixture.hub).captureTransaction(any())
     }
 
-    @Test
-    fun `do not start transaction on created if API 29`() {
-        val sut = fixture.getSut()
-        fixture.options.tracesSampleRate = 1.0
-        sut.register(fixture.hub, fixture.options)
-
-        val activity = mock<Activity>()
-        sut.onActivityCreated(activity, mock())
-
-        verify(fixture.hub, never()).startTransaction(any(), any(), anyOrNull(), any(), any())
-    }
-
     @Test
     fun `do not stop transaction on resumed if API 29`() {
         val sut = fixture.getSut()
         fixture.options.tracesSampleRate = 1.0
         sut.register(fixture.hub, fixture.options)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, mock())
+        sut.onActivityCreated(activity, mock())
         sut.onActivityResumed(activity)
 
         verify(fixture.hub, never()).captureTransaction(any())
@@ -556,7 +544,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime(date)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         // call only once
         verify(fixture.hub).startTransaction(any(), any(), eq(date), any(), any())
@@ -572,7 +560,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime(date)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         val span = fixture.transaction.children.first()
         assertEquals(span.operation, "app.start.warm")
@@ -589,7 +577,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime(date)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, null)
+        sut.onActivityCreated(activity, null)
 
         val span = fixture.transaction.children.first()
         assertEquals(span.operation, "app.start.cold")
@@ -606,7 +594,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime(date)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         val span = fixture.transaction.children.first()
         assertEquals(span.description, "Warm Start")
@@ -623,7 +611,7 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime(date)
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, null)
+        sut.onActivityCreated(activity, null)
 
         val span = fixture.transaction.children.first()
         assertEquals(span.description, "Cold Start")
@@ -640,14 +628,14 @@ class ActivityLifecycleIntegrationTest {
         setAppStartTime()
 
         val activity = mock<Activity>()
-        sut.onActivityPreCreated(activity, fixture.bundle)
+        sut.onActivityCreated(activity, fixture.bundle)
 
         verify(fixture.hub).startTransaction(any(), any(), eq(date), any(), any())
         sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityPostResumed(activity)
 
         val newActivity = mock<Activity>()
-        sut.onActivityPreCreated(newActivity, fixture.bundle)
+        sut.onActivityCreated(newActivity, fixture.bundle)
 
         val nullDate: Date? = null
         verify(fixture.hub).startTransaction(any(), any(), eq(nullDate), any(), any())