Skip to content

Commit 45e4c1d

Browse files
leotianlizhanjeremyjiang-dev
authored andcommitted
Fireperf: fix incorrect sessionIds for _fs and _bs traces (#3122)
* simple fix by changing order of methods * add test * comments * rewording
1 parent 490f1bc commit 45e4c1d

File tree

3 files changed

+44
-2
lines changed

3 files changed

+44
-2
lines changed

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,9 @@ public synchronized void onActivityStopped(Activity activity) {
340340
if (activityToResumedMap.isEmpty()) {
341341
// no more activity in foreground, app goes to background.
342342
stopTime = clock.getTime();
343-
updateAppState(ApplicationProcessState.BACKGROUND);
344343
sendSessionLog(Constants.TraceNames.FOREGROUND_TRACE_NAME.toString(), resumeTime, stopTime);
344+
// order is important to complete _fs before triggering a sessionId change b/204362742
345+
updateAppState(ApplicationProcessState.BACKGROUND);
345346
}
346347
}
347348
}
@@ -356,14 +357,16 @@ public synchronized void onActivityResumed(Activity activity) {
356357
// The first resumed activity means app comes to foreground.
357358
resumeTime = clock.getTime();
358359
activityToResumedMap.put(activity, true);
359-
updateAppState(ApplicationProcessState.FOREGROUND);
360360
if (isColdStart) {
361361
// case 1: app startup.
362+
updateAppState(ApplicationProcessState.FOREGROUND);
362363
sendAppColdStartUpdate();
363364
isColdStart = false;
364365
} else {
365366
// case 2: app switch from background to foreground.
366367
sendSessionLog(Constants.TraceNames.BACKGROUND_TRACE_NAME.toString(), stopTime, resumeTime);
368+
// order is important to complete _bs before triggering a sessionId change b/204362742
369+
updateAppState(ApplicationProcessState.FOREGROUND);
367370
}
368371
} else {
369372
// case 3: app already in foreground, current activity is replaced by another activity.
@@ -626,6 +629,11 @@ Timer getPauseTime() {
626629
return stopTime;
627630
}
628631

632+
@VisibleForTesting
633+
void setStopTime(Timer timer) {
634+
stopTime = timer;
635+
}
636+
629637
@VisibleForTesting
630638
Timer getResumeTime() {
631639
return resumeTime;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.firebase.perf.application;
1616

1717
import androidx.annotation.NonNull;
18+
import com.google.android.gms.common.util.VisibleForTesting;
1819
import com.google.firebase.perf.application.AppStateMonitor.AppStateCallback;
1920
import com.google.firebase.perf.v1.ApplicationProcessState;
2021
import java.lang.ref.WeakReference;
@@ -102,4 +103,9 @@ public void onUpdateAppState(ApplicationProcessState newState) {
102103
public ApplicationProcessState getAppState() {
103104
return currentAppState;
104105
}
106+
107+
@VisibleForTesting
108+
public WeakReference<AppStateCallback> getAppStateCallback() {
109+
return appStateCallback;
110+
}
105111
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,15 @@
3838
import com.google.firebase.perf.config.DeviceCacheManager;
3939
import com.google.firebase.perf.metrics.NetworkRequestMetricBuilder;
4040
import com.google.firebase.perf.metrics.Trace;
41+
import com.google.firebase.perf.session.SessionManager;
4142
import com.google.firebase.perf.session.gauges.GaugeManager;
4243
import com.google.firebase.perf.transport.TransportManager;
4344
import com.google.firebase.perf.util.Clock;
4445
import com.google.firebase.perf.util.Constants;
4546
import com.google.firebase.perf.util.ImmutableBundle;
4647
import com.google.firebase.perf.util.Timer;
4748
import com.google.firebase.perf.v1.ApplicationProcessState;
49+
import com.google.firebase.perf.v1.PerfSession;
4850
import com.google.firebase.perf.v1.TraceMetric;
4951
import com.google.testing.timing.FakeDirectExecutorService;
5052
import java.lang.ref.WeakReference;
@@ -766,6 +768,32 @@ public void appColdStart_singleSubscriberRegistersForMultipleTimes_oneCallbackIs
766768
verify(mockInitializer1, times(1)).onAppColdStart();
767769
}
768770

771+
@Test
772+
public void updatePerfSession_isAfterSendingForegroundOrBackgroundSession() {
773+
AppStateMonitor monitor = new AppStateMonitor(transportManager, clock);
774+
monitor.registerForAppState(SessionManager.getInstance().getAppStateCallback());
775+
monitor.setStopTime(new Timer(currentTime));
776+
monitor.setIsColdStart(false);
777+
// Mandatory due to circular dependencies of singletons AppStateMonitor and SessionManager
778+
AppStateMonitor.getInstance().setIsColdStart(false);
779+
780+
// Foreground -> Background, sends _fs
781+
PerfSession currentSession = SessionManager.getInstance().perfSession().build();
782+
monitor.onActivityResumed(activity1);
783+
784+
verify(transportManager, times(1)).log(argTraceMetric.capture(), eq(FOREGROUND_BACKGROUND));
785+
PerfSession sentSession = argTraceMetric.getValue().getPerfSessions(0);
786+
Assert.assertEquals(currentSession, sentSession);
787+
788+
// Background -> Foreground, sends _bs
789+
currentSession = SessionManager.getInstance().perfSession().build();
790+
monitor.onActivityStopped(activity1);
791+
792+
verify(transportManager, times(2)).log(argTraceMetric.capture(), eq(FOREGROUND_BACKGROUND));
793+
sentSession = argTraceMetric.getValue().getPerfSessions(0);
794+
Assert.assertEquals(currentSession, sentSession);
795+
}
796+
769797
private static Activity createFakeActivity(boolean isHardwareAccelerated) {
770798
ActivityController<Activity> fakeActivityController = Robolectric.buildActivity(Activity.class);
771799

0 commit comments

Comments
 (0)