Merge c642240 into 590fcb8

getsentry · Jul 19, 2023 · 8e39f54 · 8e39f54
2 parents 590fcb8 + c642240
commit 8e39f54
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@ Breaking changes:
 
 ### Fixes
 
+- Measure AppStart time till First Draw instead of `onResume` ([#2851](https://github.com/getsentry/sentry-java/pull/2851))
+
 Breaking changes:
 - Move enableNdk from SentryOptions to SentryAndroidOptions ([#2793](https://github.com/getsentry/sentry-java/pull/2793))
 
@@ -2302,4 +2304,4 @@ Features from the current SDK like `ANR` are also available (by default triggere
 Packages were released on [`bintray`](https://dl.bintray.com/getsentry/sentry-android/io/sentry/), [`jcenter`](https://jcenter.bintray.com/io/sentry/sentry-android/)
 
 We'd love to get feedback and we'll work in getting the GA `2.0.0` out soon.
-Until then, the [stable SDK offered by Sentry is at version 1.7.28](https://github.com/getsentry/sentry-java/releases/tag/v1.7.28)
+Until then, the [stable SDK offered by Sentry is at version 1.7.28](https://github.com/getsentry/sentry-java/releases/tag/v1.7.28)
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
@@ -389,17 +389,6 @@ public synchronized void onActivityStarted(final @NotNull Activity activity) {
   @Override
   public synchronized void onActivityResumed(final @NotNull Activity activity) {
     if (performanceEnabled) {
-      // app start span
-      @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime();
-      @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime();
-      // in case the SentryPerformanceProvider is disabled it does not set the app start times,
-      // and we need to set the end time manually here,
-      // the start time gets set manually in SentryAndroid.init()
-      if (appStartStartTime != null && appStartEndTime == null) {
-        AppStartState.getInstance().setAppStartEnd();
-      }
-      finishAppStartSpan();
-
       final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity);
       final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity);
       final View rootView = activity.findViewById(android.R.id.content);
@@ -529,6 +518,17 @@ private void cancelTtfdAutoClose() {
   }
 
   private void onFirstFrameDrawn(final @Nullable ISpan ttfdSpan, final @Nullable ISpan ttidSpan) {
+    // app start span
+    @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime();
+    @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime();
+    // in case the SentryPerformanceProvider is disabled it does not set the app start times,
+    // and we need to set the end time manually here,
+    // the start time gets set manually in SentryAndroid.init()
+    if (appStartStartTime != null && appStartEndTime == null) {
+      AppStartState.getInstance().setAppStartEnd();
+    }
+    finishAppStartSpan();
+
     if (options != null && ttidSpan != null) {
       final SentryDate endDate = options.getDateProvider().now();
       final long durationNanos = endDate.diff(ttidSpan.getStartDate());

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java
@@ -45,6 +45,10 @@ synchronized void setAppStartEnd() {
 
   @TestOnly
   void setAppStartEnd(final long appStartEndMillis) {
+    if (this.appStartEndMillis != null) {
+      // only set app start end once
+      return;
+    }
     this.appStartEndMillis = appStartEndMillis;
   }
 

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java
@@ -1,13 +1,18 @@
 package io.sentry.android.core;
 
+import android.annotation.SuppressLint;
 import android.app.Activity;
 import android.app.Application;
 import android.content.Context;
 import android.content.pm.ProviderInfo;
 import android.net.Uri;
+import android.os.Build;
 import android.os.Bundle;
 import android.os.SystemClock;
+import android.view.View;
+import io.sentry.NoOpLogger;
 import io.sentry.SentryDate;
+import io.sentry.android.core.internal.util.FirstDrawDoneListener;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -33,8 +38,22 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider
 
   private @Nullable Application application;
 
+  private final @NotNull BuildInfoProvider buildInfoProvider;
+
+  private final @NotNull MainLooperHandler mainHandler;
+
   public SentryPerformanceProvider() {
     AppStartState.getInstance().setAppStartTime(appStartMillis, appStartTime);
+    buildInfoProvider = new BuildInfoProvider(NoOpLogger.getInstance());
+    mainHandler = new MainLooperHandler();
+  }
+
+  SentryPerformanceProvider(
+      final @NotNull BuildInfoProvider buildInfoProvider,
+      final @NotNull MainLooperHandler mainHandler) {
+    AppStartState.getInstance().setAppStartTime(appStartMillis, appStartTime);
+    this.buildInfoProvider = buildInfoProvider;
+    this.mainHandler = mainHandler;
   }
 
   @Override
@@ -100,12 +119,22 @@ public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle saved
   @Override
   public void onActivityStarted(@NotNull Activity activity) {}
 
+  @SuppressLint("NewApi")
   @Override
   public void onActivityResumed(@NotNull Activity activity) {
     if (!firstActivityResumed) {
       // sets App start as finished when the very first activity calls onResume
       firstActivityResumed = true;
-      AppStartState.getInstance().setAppStartEnd();
+      final View rootView = activity.findViewById(android.R.id.content);
+      if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN
+          && rootView != null) {
+        FirstDrawDoneListener.registerForNextDraw(
+            rootView, () -> AppStartState.getInstance().setAppStartEnd(), buildInfoProvider);
+      } else {
+        // Posting a task to the main thread's handler will make it executed after it finished
+        // its current job. That is, right after the activity draws the layout.
+        mainHandler.post(() -> AppStartState.getInstance().setAppStartEnd());
+      }
     }
     if (application != null) {
       application.unregisterActivityLifecycleCallbacks(this);

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
@@ -889,13 +889,17 @@ class ActivityLifecycleIntegrationTest {
         AppStartState.getInstance().setColdStart(false)
 
         // when activity is created
+        val view = fixture.createView()
         val activity = mock<Activity>()
+        whenever(activity.findViewById<View>(any())).thenReturn(view)
         sut.onActivityCreated(activity, fixture.bundle)
         // then app-start end time should still be null
         assertNull(AppStartState.getInstance().appStartEndTime)
 
         // when activity is resumed
         sut.onActivityResumed(activity)
+        Thread.sleep(1)
+        runFirstDraw(view)
         // end-time should be set
         assertNotNull(AppStartState.getInstance().appStartEndTime)
     }
@@ -936,10 +940,14 @@ class ActivityLifecycleIntegrationTest {
         AppStartState.getInstance().setColdStart(false)
 
         // when activity is created, started and resumed multiple times
+        val view = fixture.createView()
         val activity = mock<Activity>()
+        whenever(activity.findViewById<View>(any())).thenReturn(view)
         sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityStarted(activity)
         sut.onActivityResumed(activity)
+        Thread.sleep(1)
+        runFirstDraw(view)
 
         val firstAppStartEndTime = AppStartState.getInstance().appStartEndTime
 
@@ -948,6 +956,8 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityStopped(activity)
         sut.onActivityStarted(activity)
         sut.onActivityResumed(activity)
+        Thread.sleep(1)
+        runFirstDraw(view)
 
         // then the end time should not be overwritten
         assertEquals(

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt
@@ -1,6 +1,7 @@
 package io.sentry.android.core
 
 import io.sentry.SentryInstantDate
+import io.sentry.SentryLongDate
 import io.sentry.SentryNanotimeDate
 import java.util.Date
 import kotlin.test.BeforeTest
@@ -58,6 +59,18 @@ class AppStartStateTest {
         assertSame(date, sut.appStartTime)
     }
 
+    @Test
+    fun `do not overwrite app start end time if already set`() {
+        val sut = AppStartState.getInstance()
+
+        sut.setColdStart(true)
+        sut.setAppStartTime(1, SentryLongDate(1000000))
+        sut.setAppStartEnd(2)
+        sut.setAppStartEnd(3)
+
+        assertEquals(0, SentryLongDate(2000000).compareTo(sut.appStartEndTime!!))
+    }
+
     @Test
     fun `do not overwrite cold start value if already set`() {
         val sut = AppStartState.getInstance()

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt
@@ -1,14 +1,21 @@
 package io.sentry.android.core
 
+import android.app.Activity
 import android.app.Application
 import android.content.pm.ProviderInfo
 import android.os.Bundle
+import android.os.Looper
+import android.view.View
+import android.view.ViewTreeObserver
+import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.SentryNanotimeDate
 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 org.robolectric.Shadows
 import java.util.Date
 import kotlin.test.BeforeTest
 import kotlin.test.Test
@@ -84,11 +91,21 @@ class SentryPerformanceProviderTest {
         val mockContext = ContextUtilsTest.createMockContext(true)
         providerInfo.authority = AUTHORITY
 
-        val provider = SentryPerformanceProvider()
+        val provider = SentryPerformanceProvider(
+            mock {
+                whenever(mock.sdkInfoVersion).thenReturn(29)
+            },
+            MainLooperHandler()
+        )
         provider.attachInfo(mockContext, providerInfo)
 
-        provider.onActivityCreated(mock(), Bundle())
-        provider.onActivityResumed(mock())
+        val view = createView()
+        val activity = mock<Activity>()
+        whenever(activity.findViewById<View>(any())).thenReturn(view)
+        provider.onActivityCreated(activity, Bundle())
+        provider.onActivityResumed(activity)
+        Thread.sleep(1)
+        runFirstDraw(view)
 
         assertNotNull(AppStartState.getInstance().appStartInterval)
         assertNotNull(AppStartState.getInstance().appStartEndTime)
@@ -97,6 +114,24 @@ class SentryPerformanceProviderTest {
             .unregisterActivityLifecycleCallbacks(any())
     }
 
+    private fun createView(): View {
+        val view = View(ApplicationProvider.getApplicationContext())
+
+        // Adding a listener forces ViewTreeObserver.mOnDrawListeners to be initialized and non-null.
+        val dummyListener = ViewTreeObserver.OnDrawListener {}
+        view.viewTreeObserver.addOnDrawListener(dummyListener)
+        view.viewTreeObserver.removeOnDrawListener(dummyListener)
+
+        return view
+    }
+
+    private fun runFirstDraw(view: View) {
+        // Removes OnDrawListener in the next OnGlobalLayout after onDraw
+        view.viewTreeObserver.dispatchOnDraw()
+        view.viewTreeObserver.dispatchOnGlobalLayout()
+        Shadows.shadowOf(Looper.getMainLooper()).idle()
+    }
+
     companion object {
         private const val AUTHORITY = "io.sentry.sample.SentryPerformanceProvider"
     }