Skip to content

Commit 96c4451

Browse files
leotianlizhanjeremyjiang-dev
authored andcommitted
Fireperf: start screen trace in onResume (#3532)
* move screen trace start to onResum * gjf * comment * fix tests
1 parent f6203da commit 96c4451

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
@@ -169,20 +169,7 @@ public void onActivityCreated(Activity activity, Bundle savedInstanceState) {
169169
public void onActivityDestroyed(Activity activity) {}
170170

171171
@Override
172-
public synchronized void onActivityStarted(Activity activity) {
173-
if (isScreenTraceSupported() && configResolver.isPerformanceMonitoringEnabled()) {
174-
// Starts recording frame metrics for this activity.
175-
/**
176-
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
177-
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
178-
*/
179-
frameMetricsAggregator.add(activity);
180-
// Start the Trace
181-
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
182-
screenTrace.start();
183-
activityToScreenTraceMap.put(activity, screenTrace);
184-
}
185-
}
172+
public synchronized void onActivityStarted(Activity activity) {}
186173

187174
@Override
188175
public synchronized void onActivityStopped(Activity activity) {
@@ -204,7 +191,8 @@ public synchronized void onActivityResumed(Activity activity) {
204191
// cases:
205192
// 1. At app startup, first activity comes to foreground.
206193
// 2. app switch from background to foreground.
207-
// 3. app already in foreground, current activity is replaced by another activity.
194+
// 3. app already in foreground, current activity is replaced by another activity, or the
195+
// current activity was paused then resumed without onStop, for example by an AlertDialog
208196
if (activityToResumedMap.isEmpty()) {
209197
// The first resumed activity means app comes to foreground.
210198
resumeTime = clock.getTime();
@@ -221,9 +209,24 @@ public synchronized void onActivityResumed(Activity activity) {
221209
updateAppState(ApplicationProcessState.FOREGROUND);
222210
}
223211
} else {
224-
// case 3: app already in foreground, current activity is replaced by another activity.
212+
// case 3: app already in foreground, current activity is replaced by another activity, or the
213+
// current activity was paused then resumed without onStop, for example by an AlertDialog
225214
activityToResumedMap.put(activity, true);
226215
}
216+
217+
// Screen trace is after session update so the sessionId is not added twice to the Trace
218+
if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
219+
// Starts recording frame metrics for this activity.
220+
/**
221+
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
222+
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
223+
*/
224+
frameMetricsAggregator.add(activity);
225+
// Start the Trace
226+
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
227+
screenTrace.start();
228+
activityToScreenTraceMap.put(activity, screenTrace);
229+
}
227230
}
228231

229232
/** 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)