Skip to content

Commit d89b3cd

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 37fa4d2 commit d89b3cd

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

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

185172
@Override
186173
public synchronized void onActivityStopped(Activity activity) {
@@ -202,7 +189,8 @@ public synchronized void onActivityResumed(Activity activity) {
202189
// cases:
203190
// 1. At app startup, first activity comes to foreground.
204191
// 2. app switch from background to foreground.
205-
// 3. app already in foreground, current activity is replaced by another activity.
192+
// 3. app already in foreground, current activity is replaced by another activity, or the
193+
// current activity was paused then resumed without onStop, for example by an AlertDialog
206194
if (activityToResumedMap.isEmpty()) {
207195
// The first resumed activity means app comes to foreground.
208196
resumeTime = clock.getTime();
@@ -219,9 +207,24 @@ public synchronized void onActivityResumed(Activity activity) {
219207
updateAppState(ApplicationProcessState.FOREGROUND);
220208
}
221209
} else {
222-
// case 3: app already in foreground, current activity is replaced by another activity.
210+
// case 3: app already in foreground, current activity is replaced by another activity, or the
211+
// current activity was paused then resumed without onStop, for example by an AlertDialog
223212
activityToResumedMap.put(activity, true);
224213
}
214+
215+
// Screen trace is after session update so the sessionId is not added twice to the Trace
216+
if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
217+
// Starts recording frame metrics for this activity.
218+
/**
219+
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
220+
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
221+
*/
222+
frameMetricsAggregator.add(activity);
223+
// Start the Trace
224+
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
225+
screenTrace.start();
226+
activityToScreenTraceMap.put(activity, screenTrace);
227+
}
225228
}
226229

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