-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix app_start trace not firing in SwiftUI apps using @UIApplicationDelegateAdaptor (#15802) #15912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
92a8fa1
02915e8
ae249f3
4eedd53
21f96ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,6 +34,7 @@ | |||||||
| static FPRCPUGaugeData *gAppStartCPUGaugeData = nil; | ||||||||
| static FPRMemoryGaugeData *gAppStartMemoryGaugeData = nil; | ||||||||
| static BOOL isActivePrewarm = NO; | ||||||||
| static BOOL sLaunchedInBackgroundState = NO; | ||||||||
|
|
||||||||
| NSString *const kFPRAppStartTraceName = @"_as"; | ||||||||
| NSString *const kFPRAppStartStageNameTimeToUI = @"_astui"; | ||||||||
|
|
@@ -118,15 +119,18 @@ + (void)windowDidBecomeVisible:(NSNotification *)notification { | |||||||
| + (void)applicationDidFinishLaunching:(NSNotification *)notification { | ||||||||
| applicationDidFinishLaunchTime = [NSDate date]; | ||||||||
|
|
||||||||
| // Detect a background launch and invalidate app start time | ||||||||
| // this prevents we measure duration from background launch | ||||||||
| UIApplicationState state = [UIApplication sharedApplication].applicationState; | ||||||||
| if (state == UIApplicationStateBackground) { | ||||||||
| // App launched in background so we invalidate the captured app start time | ||||||||
| // to prevent incorrect measurement when user later opens the app | ||||||||
| appStartTime = nil; | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, | ||||||||
| @"Background launch detected. App start measurement will be skipped."); | ||||||||
| NSDictionary *sceneManifest = | ||||||||
| [[NSBundle mainBundle] objectForInfoDictionaryKey:@"UIApplicationSceneManifest"]; | ||||||||
| if (sceneManifest) { | ||||||||
| // Scene-based apps may report .background here even on foreground launches | ||||||||
| // because scenes haven't connected yet. Defer decision to didBecomeActive. | ||||||||
| sLaunchedInBackgroundState = YES; | ||||||||
| } else { | ||||||||
| // Non-scene app - .background is reliable, invalidate app start. | ||||||||
| appStartTime = nil; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| [[NSNotificationCenter defaultCenter] removeObserver:self | ||||||||
|
|
@@ -259,15 +263,25 @@ - (void)appDidBecomeActiveNotification:(NSNotification *)notification { | |||||||
|
|
||||||||
| static dispatch_once_t onceToken; | ||||||||
| dispatch_once(&onceToken, ^{ | ||||||||
| // Early bailout if background launch was detected, appStartTime will be nil if the app was | ||||||||
| // launched in background | ||||||||
| if (appStartTime == nil) { | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, | ||||||||
| @"App start trace skipped due to background launch. " | ||||||||
| @"This prevents reporting incorrect multi-minute/hour durations."); | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, @"App start trace skipped due to background launch."); | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| // Distinguish genuine background launches by checking how long ago +load captured appStartTime. | ||||||||
| // A real foreground launch reaches didBecomeActive within seconds; a background launch that is | ||||||||
| // later opened by the user will have a much larger gap. | ||||||||
| if (sLaunchedInBackgroundState) { | ||||||||
| NSTimeInterval timeSinceStart = [[NSDate date] timeIntervalSinceDate:appStartTime]; | ||||||||
| // App launched in background so we invalidate the captured app start time | ||||||||
| // to prevent incorrect measurement when user later opens the app | ||||||||
|
Comment on lines
+276
to
+277
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is a bit misleading as it appears to be a leftover from the previous implementation. The code no longer invalidates
Suggested change
|
||||||||
| if (timeSinceStart > gAppStartMaxValidDuration) { | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, | ||||||||
| @"Background launch detected. App start measurement will be skipped."); | ||||||||
| return; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| self.appStartTrace = [[FIRTrace alloc] initInternalTraceWithName:kFPRAppStartTraceName]; | ||||||||
| [self.appStartTrace startWithStartTime:appStartTime]; | ||||||||
| [self.appStartTrace startStageNamed:kFPRAppStartStageNameTimeToUI startTime:appStartTime]; | ||||||||
|
|
@@ -281,8 +295,7 @@ - (void)appDidBecomeActiveNotification:(NSNotification *)notification { | |||||||
| self.appStartTrace.backgroundTraceState != FPRTraceStateForegroundOnly) { | ||||||||
| [self.appStartTrace cancel]; | ||||||||
| self.appStartTrace = nil; | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, | ||||||||
| @"App start trace cancelled due to background state contamination."); | ||||||||
| FPRLogDebug(kFPRTraceNotCreated, @"App start trace cancelled. Trace had background state."); | ||||||||
| } | ||||||||
|
|
||||||||
| // Stop the active background session trace. | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worried about a scenario where a non-switftUI app starts itself in background, but the app gets launched before the max app start duration value - this code would account for that app start time.
I think the ideal would be differentiate if this is a SwiftUI app and make some exceptions rather than impacting all the scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a commit that checks for UIApplicationSceneManifest in the info.plist before deferring to keep the original behavior in the rest of the escenarios
LMK what you think 😄