Skip to content

Commit ec86d97

Browse files
leotianlizhanqdpham13
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 8e6d69c commit ec86d97

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
@@ -173,8 +173,9 @@ public synchronized void onActivityStopped(Activity activity) {
173173
if (activityToResumedMap.isEmpty()) {
174174
// no more activity in foreground, app goes to background.
175175
stopTime = clock.getTime();
176-
updateAppState(ApplicationProcessState.BACKGROUND);
177176
sendSessionLog(Constants.TraceNames.FOREGROUND_TRACE_NAME.toString(), resumeTime, stopTime);
177+
// order is important to complete _fs before triggering a sessionId change b/204362742
178+
updateAppState(ApplicationProcessState.BACKGROUND);
178179
}
179180
}
180181
}
@@ -189,14 +190,16 @@ public synchronized void onActivityResumed(Activity activity) {
189190
// The first resumed activity means app comes to foreground.
190191
resumeTime = clock.getTime();
191192
activityToResumedMap.put(activity, true);
192-
updateAppState(ApplicationProcessState.FOREGROUND);
193193
if (isColdStart) {
194194
// case 1: app startup.
195+
updateAppState(ApplicationProcessState.FOREGROUND);
195196
sendAppColdStartUpdate();
196197
isColdStart = false;
197198
} else {
198199
// case 2: app switch from background to foreground.
199200
sendSessionLog(Constants.TraceNames.BACKGROUND_TRACE_NAME.toString(), stopTime, resumeTime);
201+
// order is important to complete _bs before triggering a sessionId change b/204362742
202+
updateAppState(ApplicationProcessState.FOREGROUND);
200203
}
201204
} else {
202205
// case 3: app already in foreground, current activity is replaced by another activity.
@@ -459,6 +462,11 @@ Timer getPauseTime() {
459462
return stopTime;
460463
}
461464

465+
@VisibleForTesting
466+
void setStopTime(Timer timer) {
467+
stopTime = timer;
468+
}
469+
462470
@VisibleForTesting
463471
Timer getResumeTime() {
464472
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)