Skip to content

Commit 3c6bed3

Browse files
authored
Revert "Make a change to prevent false positive background app starts." (#7278)
Reverts #7274 Reverting this if the current change is going to be included in M169. An additional change is needed to verify that the `mainThreadRunnableTime` should be set only if no activity's onCreate has been called - otherwise this can cause a regression of *genuine* background starts.
1 parent be8f480 commit 3c6bed3

File tree

3 files changed

+10
-62
lines changed

3 files changed

+10
-62
lines changed

firebase-perf/CHANGELOG.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# Unreleased
22
* [fixed] Fixed an ANR on app launch. [#4831]
3-
* [fixed] Fixed app start traces on API 34+. [#5920]
43

54
# 22.0.0
65
* [changed] **Breaking Change**: Updated minSdkVersion to API level 23 or higher.

firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser
7575
private static final @NonNull Timer PERF_CLASS_LOAD_TIME = new Clock().getTime();
7676
private static final long MAX_LATENCY_BEFORE_UI_INIT = TimeUnit.MINUTES.toMicros(1);
7777

78-
private static final long MAX_BACKGROUND_RUNNABLE_DELAY = TimeUnit.MILLISECONDS.toMicros(100);
79-
8078
// Core pool size 0 allows threads to shut down if they're idle
8179
private static final int CORE_POOL_SIZE = 0;
8280
private static final int MAX_POOL_SIZE = 1; // Only need single thread
@@ -113,8 +111,6 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser
113111
private final @Nullable Timer processStartTime;
114112
private final @Nullable Timer firebaseClassLoadTime;
115113
private Timer onCreateTime = null;
116-
117-
private Timer mainThreadRunnableTime = null;
118114
private Timer onStartTime = null;
119115
private Timer onResumeTime = null;
120116
private Timer firstForegroundTime = null;
@@ -323,26 +319,8 @@ private void recordOnDrawFrontOfQueue() {
323319
logExperimentTrace(this.experimentTtid);
324320
}
325321

326-
private void resolveIsStartedFromBackground() {
327-
// If the mainThreadRunnableTime is null, either the runnable hasn't run, or this check has
328-
// already been made.
329-
if (mainThreadRunnableTime == null) {
330-
return;
331-
}
332-
333-
// Set it to true if the runnable ran more than 100ms prior to onActivityCreated()
334-
if (mainThreadRunnableTime.getDurationMicros() > MAX_BACKGROUND_RUNNABLE_DELAY) {
335-
isStartedFromBackground = true;
336-
}
337-
338-
// Set this to null to prevent additional checks if `onActivityCreated()` is called again.
339-
mainThreadRunnableTime = null;
340-
}
341-
342322
@Override
343323
public synchronized void onActivityCreated(Activity activity, Bundle savedInstanceState) {
344-
resolveIsStartedFromBackground();
345-
346324
if (isStartedFromBackground || onCreateTime != null // An activity already called onCreate()
347325
) {
348326
return;
@@ -582,7 +560,8 @@ public static boolean isScreenOn(Context appContext) {
582560
* We use StartFromBackgroundRunnable to detect if app is started from background or foreground.
583561
* If app is started from background, we do not generate AppStart trace. This runnable is posted
584562
* to main UI thread from FirebasePerfEarly. If app is started from background, this runnable will
585-
* be executed earlier than 100ms of any activity's onCreate() method.
563+
* be executed before any activity's onCreate() method. If app is started from foreground,
564+
* activity's onCreate() method is executed before this runnable.
586565
*/
587566
public static class StartFromBackgroundRunnable implements Runnable {
588567
private final AppStartTrace trace;
@@ -593,7 +572,10 @@ public StartFromBackgroundRunnable(final AppStartTrace trace) {
593572

594573
@Override
595574
public void run() {
596-
trace.mainThreadRunnableTime = new Timer();
575+
// if no activity has ever been created.
576+
if (trace.onCreateTime == null) {
577+
trace.isStartedFromBackground = true;
578+
}
597579
}
598580
}
599581

@@ -632,7 +614,7 @@ Timer getOnResumeTime() {
632614
}
633615

634616
@VisibleForTesting
635-
void setMainThreadRunnableTime(Timer timer) {
636-
mainThreadRunnableTime = timer;
617+
void setIsStartFromBackground() {
618+
isStartedFromBackground = true;
637619
}
638620
}

firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static org.mockito.ArgumentMatchers.isA;
1919
import static org.mockito.Mockito.doAnswer;
2020
import static org.mockito.Mockito.mock;
21-
import static org.mockito.Mockito.spy;
2221
import static org.mockito.Mockito.times;
2322
import static org.mockito.Mockito.verify;
2423
import static org.mockito.Mockito.when;
@@ -239,42 +238,11 @@ public void testDelayedAppStart() {
239238
}
240239

241240
@Test
242-
public void testStartFromBackground_within100ms() {
241+
public void testStartFromBackground() {
243242
FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService();
244-
Timer fakeTimer = spy(new Timer(currentTime));
245243
AppStartTrace trace =
246244
new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService);
247-
trace.registerActivityLifecycleCallbacks(appContext);
248-
trace.setMainThreadRunnableTime(fakeTimer);
249-
250-
when(fakeTimer.getDurationMicros()).thenReturn(99L);
251-
trace.onActivityCreated(activity1, bundle);
252-
Assert.assertNotNull(trace.getOnCreateTime());
253-
++currentTime;
254-
trace.onActivityStarted(activity1);
255-
Assert.assertNotNull(trace.getOnStartTime());
256-
++currentTime;
257-
trace.onActivityResumed(activity1);
258-
Assert.assertNotNull(trace.getOnResumeTime());
259-
fakeExecutorService.runAll();
260-
// There should be a trace sent since the delay between the main thread and onActivityCreated
261-
// is limited.
262-
verify(transportManager, times(1))
263-
.log(
264-
traceArgumentCaptor.capture(),
265-
ArgumentMatchers.nullable(ApplicationProcessState.class));
266-
}
267-
268-
@Test
269-
public void testStartFromBackground_moreThan100ms() {
270-
FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService();
271-
Timer fakeTimer = spy(new Timer(currentTime));
272-
AppStartTrace trace =
273-
new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService);
274-
trace.registerActivityLifecycleCallbacks(appContext);
275-
trace.setMainThreadRunnableTime(fakeTimer);
276-
277-
when(fakeTimer.getDurationMicros()).thenReturn(TimeUnit.MILLISECONDS.toMicros(100) + 1);
245+
trace.setIsStartFromBackground();
278246
trace.onActivityCreated(activity1, bundle);
279247
Assert.assertNull(trace.getOnCreateTime());
280248
++currentTime;
@@ -284,7 +252,6 @@ public void testStartFromBackground_moreThan100ms() {
284252
trace.onActivityResumed(activity1);
285253
Assert.assertNull(trace.getOnResumeTime());
286254
// There should be no trace sent.
287-
fakeExecutorService.runAll();
288255
verify(transportManager, times(0))
289256
.log(
290257
traceArgumentCaptor.capture(),

0 commit comments

Comments
 (0)