fix: TTFD waits for next drawn frame (#3505)

The SDK waits for the next drawn frame after users call reportFullyDisplayed to reflect when UI changes are visible to the user correctly.
getsentry · Dec 19, 2023 · fac579e · fac579e
1 parent f0ce81c
commit fac579e
Show file tree

Hide file tree

Showing 8 changed files with 205 additions and 177 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,11 @@
 
 - Add frames delay to transactions and spans (#3487, #3496)
 - Add slow and frozen frames to spans (#3450, #3478)
+
+### Fixes
+
+- TTFD waits for next drawn frame (#3505)
+
 ## 8.17.2
 
 ### Fixes

diff --git a/Samples/iOS-Swift/iOS-Swift/ViewControllers/LoremIpsumViewController.swift b/Samples/iOS-Swift/iOS-Swift/ViewControllers/LoremIpsumViewController.swift
@@ -14,7 +14,10 @@ class LoremIpsumViewController: UIViewController {
                 if let contents = FileManager.default.contents(atPath: path) {
                     DispatchQueue.main.async {
                         self.textView.text = String(data: contents, encoding: .utf8)
-                        SentrySDK.reportFullyDisplayed()
+
+                        dispatchQueue.asyncAfter(deadline: .now() + 0.1) {
+                            SentrySDK.reportFullyDisplayed()
+                        }
                     }
                 }
             }

diff --git a/Sources/Sentry/SentryTimeToDisplayTracker.m b/Sources/Sentry/SentryTimeToDisplayTracker.m
@@ -26,7 +26,7 @@
 
 @implementation SentryTimeToDisplayTracker {
     BOOL _waitForFullDisplay;
-    BOOL _isReadyToDisplay;
+    BOOL _initialDisplayReported;
     BOOL _fullyDisplayedReported;
     NSString *_controllerName;
 }
@@ -37,8 +37,7 @@ - (instancetype)initForController:(UIViewController *)controller
     if (self = [super init]) {
         _controllerName = [SwiftDescriptor getObjectClassName:controller];
         _waitForFullDisplay = waitForFullDisplay;
-
-        _isReadyToDisplay = NO;
+        _initialDisplayReported = NO;
         _fullyDisplayedReported = NO;
     }
     return self;
@@ -66,33 +65,26 @@ - (void)startForTracer:(SentryTracer *)tracer
     self.initialDisplaySpan.startTimestamp = tracer.startTimestamp;
 
     [SentryDependencyContainer.sharedInstance.framesTracker addListener:self];
-    [tracer setFinishCallback:^(
-        SentryTracer *_tracer) { [self trimTTFDIdNecessaryForTracer:_tracer]; }];
+    [tracer setFinishCallback:^(SentryTracer *_tracer) {
+        if (self.fullDisplaySpan.status != kSentrySpanStatusDeadlineExceeded) {
+            return;
+        }
+
+        self.fullDisplaySpan.timestamp = self.initialDisplaySpan.timestamp;
+        self.fullDisplaySpan.spanDescription = [NSString
+            stringWithFormat:@"%@ - Deadline Exceeded", self.fullDisplaySpan.spanDescription];
+        [self addTimeToDisplayMeasurement:self.fullDisplaySpan name:@"time_to_full_display"];
+    }];
 }
 
-- (void)reportReadyToDisplay
+- (void)reportInitialDisplay
 {
-    _isReadyToDisplay = YES;
+    _initialDisplayReported = YES;
 }
 
 - (void)reportFullyDisplayed
 {
     _fullyDisplayedReported = YES;
-    if (self.waitForFullDisplay && _isReadyToDisplay) {
-        // We need the timestamp to be able to calculate the duration
-        // but we can't finish first and add measure later because
-        // finishing the span may trigger the tracer finishInternal.
-        self.fullDisplaySpan.timestamp =
-            [SentryDependencyContainer.sharedInstance.dateProvider date];
-        [self addTimeToDisplayMeasurement:self.fullDisplaySpan name:@"time_to_full_display"];
-        [self.fullDisplaySpan finish];
-    }
-}
-
-- (void)addTimeToDisplayMeasurement:(SentrySpan *)span name:(NSString *)name
-{
-    NSTimeInterval duration = [span.timestamp timeIntervalSinceDate:span.startTimestamp] * 1000;
-    [span setMeasurement:name value:@(duration) unit:SentryMeasurementUnitDuration.millisecond];
 }
 
 - (void)framesTrackerHasNewFrame
@@ -102,32 +94,35 @@ - (void)framesTrackerHasNewFrame
     // The purpose of TTID and TTFD is to measure how long
     // takes to the content of the screen to change.
     // Thats why we need to wait for the next frame to be drawn.
-    if (_isReadyToDisplay && self.initialDisplaySpan.isFinished == NO) {
+    if (_initialDisplayReported && self.initialDisplaySpan.isFinished == NO) {
         self.initialDisplaySpan.timestamp = finishTime;
 
         [self addTimeToDisplayMeasurement:self.initialDisplaySpan name:@"time_to_initial_display"];
 
         [self.initialDisplaySpan finish];
-        [SentryDependencyContainer.sharedInstance.framesTracker removeListener:self];
+
+        if (!_waitForFullDisplay) {
+            [SentryDependencyContainer.sharedInstance.framesTracker removeListener:self];
+        }
     }
-    if (_waitForFullDisplay && _fullyDisplayedReported && self.fullDisplaySpan.isFinished == NO) {
+    if (_waitForFullDisplay && _fullyDisplayedReported && self.fullDisplaySpan.isFinished == NO
+        && self.initialDisplaySpan.isFinished == YES) {
         self.fullDisplaySpan.timestamp = finishTime;
 
-        [self addTimeToDisplayMeasurement:self.initialDisplaySpan name:@"time_to_full_display"];
+        [self addTimeToDisplayMeasurement:self.fullDisplaySpan name:@"time_to_full_display"];
 
         [self.fullDisplaySpan finish];
     }
-}
 
-- (void)trimTTFDIdNecessaryForTracer:(SentryTracer *)tracer
-{
-    if (self.fullDisplaySpan.status != kSentrySpanStatusDeadlineExceeded) {
-        return;
+    if (self.initialDisplaySpan.isFinished == YES && self.fullDisplaySpan.isFinished == YES) {
+        [SentryDependencyContainer.sharedInstance.framesTracker removeListener:self];
     }
+}
 
-    self.fullDisplaySpan.timestamp = self.initialDisplaySpan.timestamp;
-    self.fullDisplaySpan.spanDescription =
-        [NSString stringWithFormat:@"%@ - Deadline Exceeded", self.fullDisplaySpan.spanDescription];
+- (void)addTimeToDisplayMeasurement:(SentrySpan *)span name:(NSString *)name
+{
+    NSTimeInterval duration = [span.timestamp timeIntervalSinceDate:span.startTimestamp] * 1000;
+    [span setMeasurement:name value:@(duration) unit:SentryMeasurementUnitDuration.millisecond];
 }
 
 @end

diff --git a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m
@@ -182,7 +182,7 @@ - (void)viewControllerViewWillAppear:(UIViewController *)controller
 
         SentryTimeToDisplayTracker *ttdTracker
             = objc_getAssociatedObject(controller, &SENTRY_UI_PERFORMANCE_TRACKER_TTD_TRACKER);
-        [ttdTracker reportReadyToDisplay];
+        [ttdTracker reportInitialDisplay];
     };
 
     [self limitOverride:@"viewWillAppear"

diff --git a/Sources/Sentry/include/SentryTimeToDisplayTracker.h b/Sources/Sentry/include/SentryTimeToDisplayTracker.h
@@ -29,7 +29,7 @@ SENTRY_NO_INIT
 
 - (void)startForTracer:(SentryTracer *)tracer;
 
-- (void)reportReadyToDisplay;
+- (void)reportInitialDisplay;
 
 - (void)reportFullyDisplayed;
 

diff --git a/Tests/SentryTests/Integrations/Performance/SentryPerformanceTrackingIntegrationTests.swift b/Tests/SentryTests/Integrations/Performance/SentryPerformanceTrackingIntegrationTests.swift
@@ -1,3 +1,4 @@
+import Nimble
 import SentryTestUtils
 import XCTest