Skip to content

Commit a9ffa8b

Browse files
Fireperf: start screen trace in onResume (#3532)
* move screen trace start to onResum * gjf * comment * fix tests
1 parent e1fcfc3 commit a9ffa8b

File tree

2 files changed

+21
-18
lines changed

2 files changed

+21
-18
lines changed

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,20 +146,7 @@ public void onActivityCreated(Activity activity, Bundle savedInstanceState) {}
146146
public void onActivityDestroyed(Activity activity) {}
147147

148148
@Override
149-
public synchronized void onActivityStarted(Activity activity) {
150-
if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
151-
// Starts recording frame metrics for this activity.
152-
/**
153-
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
154-
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
155-
*/
156-
frameMetricsAggregator.add(activity);
157-
// Start the Trace
158-
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
159-
screenTrace.start();
160-
activityToScreenTraceMap.put(activity, screenTrace);
161-
}
162-
}
149+
public synchronized void onActivityStarted(Activity activity) {}
163150

164151
@Override
165152
public synchronized void onActivityStopped(Activity activity) {
@@ -181,7 +168,8 @@ public synchronized void onActivityResumed(Activity activity) {
181168
// cases:
182169
// 1. At app startup, first activity comes to foreground.
183170
// 2. app switch from background to foreground.
184-
// 3. app already in foreground, current activity is replaced by another activity.
171+
// 3. app already in foreground, current activity is replaced by another activity, or the
172+
// current activity was paused then resumed without onStop, for example by an AlertDialog
185173
if (activityToResumedMap.isEmpty()) {
186174
// The first resumed activity means app comes to foreground.
187175
resumeTime = clock.getTime();
@@ -198,9 +186,24 @@ public synchronized void onActivityResumed(Activity activity) {
198186
updateAppState(ApplicationProcessState.FOREGROUND);
199187
}
200188
} else {
201-
// case 3: app already in foreground, current activity is replaced by another activity.
189+
// case 3: app already in foreground, current activity is replaced by another activity, or the
190+
// current activity was paused then resumed without onStop, for example by an AlertDialog
202191
activityToResumedMap.put(activity, true);
203192
}
193+
194+
// Screen trace is after session update so the sessionId is not added twice to the Trace
195+
if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
196+
// Starts recording frame metrics for this activity.
197+
/**
198+
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
199+
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
200+
*/
201+
frameMetricsAggregator.add(activity);
202+
// Start the Trace
203+
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
204+
screenTrace.start();
205+
activityToScreenTraceMap.put(activity, screenTrace);
206+
}
204207
}
205208

206209
/** Returns if this is the cold start of the app. */

firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ public void screenTrace_twoActivities_traceStartedAndStoppedWithActivityLifecycl
366366
int startTime = i * 100;
367367
int endTime = startTime + 10;
368368
currentTime = startTime;
369-
monitor.onActivityStarted(activity);
369+
monitor.onActivityResumed(activity);
370370
assertThat(monitor.getActivity2ScreenTrace()).hasSize(1);
371371
currentTime = endTime;
372372
monitor.onActivityPostPaused(activity);
@@ -418,7 +418,7 @@ public void screenTrace_perfMonEnabledSwitchAtRuntime_traceCreationDependsOnRunt
418418

419419
// Assert that screen trace has been created.
420420
currentTime = 100;
421-
monitor.onActivityStarted(activityWithNonHardwareAcceleratedView);
421+
monitor.onActivityResumed(activityWithNonHardwareAcceleratedView);
422422
assertThat(monitor.getActivity2ScreenTrace()).hasSize(1);
423423
currentTime = 200;
424424
monitor.onActivityPostPaused(activityWithNonHardwareAcceleratedView);

0 commit comments

Comments
 (0)