From 592ef91c9c9e09acf99393f090480f19d8b636d2 Mon Sep 17 00:00:00 2001 From: Joseph Rodiz Date: Thu, 21 May 2026 19:22:34 -0600 Subject: [PATCH 1/2] firebase-perf: fix API 34+ background-start heuristic in AppStartTrace (#8103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the timing-window heuristic in resolveIsStartedFromBackground on API 34+ with an OS-reported process start cause. Pre-API-34 keeps the legacy runnable-before-onActivityCreated check. API < 34: mainThreadRunnableTime set first ⇒ suppress (legacy, unchanged). API 34+: ProcessStartCause.cause FOREGROUND ⇒ log. UNKNOWN / null ⇒ suppress. New ProcessStartCause helper reads RunningAppProcessInfo.importance at first capture via ActivityManager.getMyMemoryState. IMPORTANCE_FOREGROUND indicates an activity-driven start; anything else maps to UNKNOWN. Pre-API-34 returns UNKNOWN and the legacy AppStartTrace logic owns the decision. FirebasePerfEarly stops scheduling StartFromBackgroundRunnable on API 34+ since its output is no longer consumed there. :firebase-perf:testReleaseUnitTest — 0 failures / 0 errors. AppStartTraceTest 9/9, ProcessStartCauseTest 6/6. --- firebase-perf/CHANGELOG.md | 5 + .../firebase/perf/FirebasePerfEarly.java | 8 +- .../firebase/perf/metrics/AppStartTrace.java | 67 +++++----- .../perf/metrics/ProcessStartCause.java | 102 +++++++++++++++ .../perf/metrics/AppStartTraceTest.java | 118 ++++++++++++++---- .../perf/metrics/ProcessStartCauseTest.java | 113 +++++++++++++++++ 6 files changed, 360 insertions(+), 53 deletions(-) create mode 100644 firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java create mode 100644 firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java diff --git a/firebase-perf/CHANGELOG.md b/firebase-perf/CHANGELOG.md index fd06467ade2..68d1466de0d 100644 --- a/firebase-perf/CHANGELOG.md +++ b/firebase-perf/CHANGELOG.md @@ -1,5 +1,10 @@ # Unreleased +- [fixed] Fixed `_app_start` traces being suppressed on API 34+ devices for typical + real-world apps. The previous timing-window heuristic has been replaced on API 34+ by + `RunningAppProcessInfo.importance` at first capture, which indicates whether the + process was forked to launch an activity. Pre-API-34 behavior is unchanged. [#8103] + # 22.0.5 - [changed] Bumped internal dependencies. diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java index 5b89deaad82..ccceb9a9330 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java @@ -15,6 +15,7 @@ package com.google.firebase.perf; import android.content.Context; +import android.os.Build; import androidx.annotation.Nullable; import com.google.firebase.FirebaseApp; import com.google.firebase.StartupTime; @@ -48,7 +49,12 @@ public FirebasePerfEarly( if (startupTime != null) { AppStartTrace appStartTrace = AppStartTrace.getInstance(); appStartTrace.registerActivityLifecycleCallbacks(context); - uiExecutor.execute(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace)); + // The posted runnable feeds AppStartTrace's pre-API-34 background-start check. + // On API 34+ the runnable's output is unused (the causal signal owns the + // decision), so we skip the main-thread post. + if (Build.VERSION.SDK_INT < 34) { + uiExecutor.execute(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace)); + } } // TODO: Bring back Firebase Sessions dependency to watch for updates to sessions. diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index 813c8988383..e2769d58184 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -74,11 +74,6 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser private static final @NonNull Timer PERF_CLASS_LOAD_TIME = new Clock().getTime(); private static final long MAX_LATENCY_BEFORE_UI_INIT = TimeUnit.MINUTES.toMicros(1); - // If the `mainThreadRunnableTime` was set within this duration, the assumption - // is that it was called immediately before `onActivityCreated` in foreground starts on API 34+. - // See b/339891952. - private static final long MAX_BACKGROUND_RUNNABLE_DELAY = TimeUnit.MILLISECONDS.toMicros(50); - // Core pool size 0 allows threads to shut down if they're idle private static final int CORE_POOL_SIZE = 0; private static final int MAX_POOL_SIZE = 1; // Only need single thread @@ -134,6 +129,11 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser private final DrawCounter onDrawCounterListener = new DrawCounter(); private boolean systemForegroundCheck = false; + // OS-reported reason this process was forked. Captured once during + // registerActivityLifecycleCallbacks; consulted by resolveIsStartedFromBackground on + // API 34+. + private @Nullable ProcessStartCause processStartCause = null; + /** * Called from onCreate() method of an activity by instrumented byte code. * @@ -224,6 +224,9 @@ public synchronized void registerActivityLifecycleCallbacks(@NonNull Context con if (appContext instanceof Application) { ((Application) appContext).registerActivityLifecycleCallbacks(this); systemForegroundCheck = systemForegroundCheck || isAnyAppProcessInForeground(appContext); + // Capture the OS-reported start cause as early as possible (this method runs from + // FirebasePerfEarly during the ContentProvider init chain). + processStartCause = ProcessStartCause.capture(appContext); isRegisteredForLifecycleCallbacks = true; this.appContext = appContext; } @@ -327,37 +330,30 @@ private void recordOnDrawFrontOfQueue() { } /** - * Sets the `isStartedFromBackground` flag to `true` if the `mainThreadRunnableTime` was set - * from the `StartFromBackgroundRunnable`. - *

- * If it's prior to API 34, it's always set to true if `mainThreadRunnableTime` was set. - *

- * If it's on or after API 34, and it was called less than `MAX_BACKGROUND_RUNNABLE_DELAY` - * before `onActivityCreated`, the - * assumption is that it was called immediately before the activity lifecycle callbacks in a - * foreground start. - * See b/339891952. + * Decide whether this process was background-only and, if so, set + * {@link #isStartedFromBackground} so the activity-lifecycle callbacks suppress the + * {@code _app_start} trace. + * + * API < 34: legacy pre-bug ordering. If {@link StartFromBackgroundRunnable} fired + * before the first {@code onActivityCreated}, suppress. + * + * API 34+: {@link ProcessStartCause} owns the decision. {@code FOREGROUND} lets the + * trace through; {@code UNKNOWN} or null suppresses. + * + * See b/339891952 and https://github.com/firebase/firebase-android-sdk/issues/8103. */ private void resolveIsStartedFromBackground() { - // If the mainThreadRunnableTime is null, either the runnable hasn't run, or this check has - // already been made. - if (mainThreadRunnableTime == null) { + if (Build.VERSION.SDK_INT < 34) { + if (mainThreadRunnableTime != null) { + isStartedFromBackground = true; + mainThreadRunnableTime = null; + } return; } - - // If the `mainThreadRunnableTime` was set prior to API 34, it's always assumed that's it's - // a background start. - // Otherwise it's assumed to be a background start if the runnable was set more than - // `MAX_BACKGROUND_RUNNABLE_DELAY` - // before the first `onActivityCreated` call. - // TODO(b/339891952): Investigate removing the API check. - if ((Build.VERSION.SDK_INT < 34) - || (mainThreadRunnableTime.getDurationMicros() > MAX_BACKGROUND_RUNNABLE_DELAY)) { + if (processStartCause == null + || processStartCause.cause != ProcessStartCause.Cause.FOREGROUND) { isStartedFromBackground = true; } - - // Set this to null to prevent additional checks. - mainThreadRunnableTime = null; } @Override @@ -633,4 +629,15 @@ Timer getOnResumeTime() { void setMainThreadRunnableTime(Timer timer) { mainThreadRunnableTime = timer; } + + @VisibleForTesting + void setProcessStartCauseForTest(@Nullable ProcessStartCause cause) { + this.processStartCause = cause; + } + + @VisibleForTesting + @Nullable + ProcessStartCause getProcessStartCauseForTest() { + return processStartCause; + } } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java new file mode 100644 index 00000000000..ca52a65c863 --- /dev/null +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java @@ -0,0 +1,102 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.perf.metrics; + +import android.app.ActivityManager; +import android.content.Context; +import android.os.Build; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; + +/** + * OS-reported reason this process was forked, used by {@link AppStartTrace} to decide + * whether to emit the {@code _app_start} trace. + * + * API 34+: {@link ActivityManager#getMyMemoryState} importance. + * {@code IMPORTANCE_FOREGROUND} at first capture indicates an activity-driven start. + * + * API < 34: returns {@link Cause#UNKNOWN}; legacy logic in {@link AppStartTrace} owns + * the decision on these versions. + * + * @hide + */ +final class ProcessStartCause { + + /** Classification of why the process was forked. */ + enum Cause { + /** Process forked to satisfy an activity launch. */ + FOREGROUND, + /** Couldn't decide — caller falls back to its own heuristic. */ + UNKNOWN + } + + /** OS classification. Never null. */ + final @NonNull Cause cause; + + /** {@code RunningAppProcessInfo.importance} at capture, or {@code -1} if unread. */ + final int importance; + + /** {@link Build.VERSION#SDK_INT} at capture. */ + final int apiLevel; + + @VisibleForTesting + ProcessStartCause(@NonNull Cause cause, int importance, int apiLevel) { + this.cause = cause; + this.importance = importance; + this.apiLevel = apiLevel; + } + + /** + * Capture the cause for the current process. Call as early as possible (during + * {@code AppStartTrace.registerActivityLifecycleCallbacks}) so the OS-set values still + * reflect the original fork reason rather than transient state mid-init. + */ + static @NonNull ProcessStartCause capture(@Nullable Context appContext) { + final int apiLevel = Build.VERSION.SDK_INT; + if (appContext == null) { + return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); + } + + final ActivityManager activityManager = + (ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE); + if (activityManager == null) { + return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); + } + + final int importance = readImportance(); + + if (apiLevel >= 34) { + Cause cause = + importance == ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND + ? Cause.FOREGROUND + : Cause.UNKNOWN; + return new ProcessStartCause(cause, importance, apiLevel); + } + + // API < 34: legacy AppStartTrace logic owns the decision. + return new ProcessStartCause(Cause.UNKNOWN, importance, apiLevel); + } + + private static int readImportance() { + try { + ActivityManager.RunningAppProcessInfo info = new ActivityManager.RunningAppProcessInfo(); + ActivityManager.getMyMemoryState(info); + return info.importance; + } catch (Throwable t) { + return -1; + } + } +} diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java index daa80ad29a2..cf744531d8a 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java @@ -238,55 +238,129 @@ public void testDelayedAppStart() { ArgumentMatchers.nullable(ApplicationProcessState.class)); } + // --- Pre-API-34 regression tests for the legacy pre-bug-ordering path --- + // + // Pre-API-34 still detects background-only starts via the StartFromBackgroundRunnable + // firing before any activity. These tests pin that behavior on the still-active code path. + @Test - public void testStartFromBackground_within50ms() { + @Config(sdk = 33) + public void preApi34_runnableFiredBeforeActivity_marksAsBackground() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - Timer fakeTimer = spy(new Timer(currentTime)); AppStartTrace trace = new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); trace.registerActivityLifecycleCallbacks(appContext); - trace.setMainThreadRunnableTime(fakeTimer); + // Simulate StartFromBackgroundRunnable having fired before any activity was created. + trace.setMainThreadRunnableTime(spy(new Timer(currentTime))); - // See AppStartTrace.MAX_BACKGROUND_RUNNABLE_DELAY. - when(fakeTimer.getDurationMicros()).thenReturn(TimeUnit.MILLISECONDS.toMicros(50) - 1); trace.onActivityCreated(activity1, bundle); - Assert.assertNotNull(trace.getOnCreateTime()); + Assert.assertNull(trace.getOnCreateTime()); ++currentTime; trace.onActivityStarted(activity1); - Assert.assertNotNull(trace.getOnStartTime()); + Assert.assertNull(trace.getOnStartTime()); ++currentTime; trace.onActivityResumed(activity1); - Assert.assertNotNull(trace.getOnResumeTime()); + Assert.assertNull(trace.getOnResumeTime()); fakeExecutorService.runAll(); - // There should be a trace sent since the delay between the main thread and onActivityCreated - // is limited. - verify(transportManager, times(1)) + + // Trace suppressed — pre-bug ordering says background. + verify(transportManager, times(0)) .log( traceArgumentCaptor.capture(), ArgumentMatchers.nullable(ApplicationProcessState.class)); } @Test - public void testStartFromBackground_moreThan50ms() { + @Config(sdk = 33) + public void preApi34_runnableNotFired_traceLogs() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - Timer fakeTimer = spy(new Timer(currentTime)); AppStartTrace trace = new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); trace.registerActivityLifecycleCallbacks(appContext); - trace.setMainThreadRunnableTime(fakeTimer); + // mainThreadRunnableTime is NOT set — i.e., the runnable hasn't fired yet, which is + // the normal pre-bug-ordering state on a cold foreground start. - // See AppStartTrace.MAX_BACKGROUND_RUNNABLE_DELAY. - when(fakeTimer.getDurationMicros()).thenReturn(TimeUnit.MILLISECONDS.toMicros(50) + 1); + currentTime = 1; trace.onActivityCreated(activity1, bundle); - Assert.assertNull(trace.getOnCreateTime()); - ++currentTime; + currentTime = 2; trace.onActivityStarted(activity1); - Assert.assertNull(trace.getOnStartTime()); - ++currentTime; + currentTime = 3; trace.onActivityResumed(activity1); - Assert.assertNull(trace.getOnResumeTime()); - // There should be no trace sent. fakeExecutorService.runAll(); + + // Trace logs — runnable-before-activity didn't happen. + verify(transportManager, times(1)) + .log( + traceArgumentCaptor.capture(), + ArgumentMatchers.nullable(ApplicationProcessState.class)); + } + + // --- API 34+ causal-signal decision tests --- + // ProcessStartCause is the only decision input on API 34+; exercise each Cause value. + + /** Builds an {@link AppStartTrace} and registers callbacks. */ + private AppStartTrace newTrace(FakeScheduledExecutorService executor) { + AppStartTrace trace = new AppStartTrace(transportManager, clock, configResolver, executor); + trace.registerActivityLifecycleCallbacks(appContext); + return trace; + } + + @Test + public void api34Plus_foregroundCause_traceLogs() { + FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); + AppStartTrace trace = newTrace(executor); + trace.setProcessStartCauseForTest( + new ProcessStartCause(ProcessStartCause.Cause.FOREGROUND, 100, 35)); + + currentTime = 1; + trace.onActivityCreated(activity1, bundle); + Assert.assertNotNull(trace.getOnCreateTime()); + currentTime = 2; + trace.onActivityStarted(activity1); + Assert.assertNotNull(trace.getOnStartTime()); + currentTime = 3; + trace.onActivityResumed(activity1); + Assert.assertNotNull(trace.getOnResumeTime()); + executor.runAll(); + + verify(transportManager, times(1)) + .log( + traceArgumentCaptor.capture(), + ArgumentMatchers.nullable(ApplicationProcessState.class)); + } + + @Test + public void api34Plus_unknownCause_traceSuppressed() { + // UNKNOWN means importance != FOREGROUND at capture — typically a warm-start + // scenario. Suppress to keep _app_start measuring real cold foreground launches. + FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); + AppStartTrace trace = newTrace(executor); + trace.setProcessStartCauseForTest( + new ProcessStartCause(ProcessStartCause.Cause.UNKNOWN, 200, 34)); + + trace.onActivityCreated(activity1, bundle); + + Assert.assertNull(trace.getOnCreateTime()); + executor.runAll(); + verify(transportManager, times(0)) + .log( + traceArgumentCaptor.capture(), + ArgumentMatchers.nullable(ApplicationProcessState.class)); + } + + @Test + public void api34Plus_nullProcessStartCause_traceSuppressed() { + // Defensive: if processStartCause is somehow null at decision time (e.g. the + // capture didn't run), suppress — better to miss a trace than emit one with no + // provenance. + FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); + AppStartTrace trace = newTrace(executor); + trace.setProcessStartCauseForTest(null); + + trace.onActivityCreated(activity1, bundle); + + Assert.assertNull(trace.getOnCreateTime()); + executor.runAll(); verify(transportManager, times(0)) .log( traceArgumentCaptor.capture(), diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java new file mode 100644 index 00000000000..fb5b63333d6 --- /dev/null +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java @@ -0,0 +1,113 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.perf.metrics; + +import static com.google.common.truth.Truth.assertThat; +import static org.robolectric.Shadows.shadowOf; + +import android.app.ActivityManager; +import android.app.ActivityManager.RunningAppProcessInfo; +import android.content.Context; +import android.os.Process; +import androidx.test.core.app.ApplicationProvider; +import java.util.Collections; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowActivityManager; + +/** Unit tests for {@link ProcessStartCause}. */ +@RunWith(RobolectricTestRunner.class) +public class ProcessStartCauseTest { + + private final Context appContext = ApplicationProvider.getApplicationContext(); + + /** Seeds {@code getMyMemoryState}'s reply with the desired importance for this process. */ + private void setProcessImportance(int importance) { + ActivityManager am = (ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE); + ShadowActivityManager shadow = shadowOf(am); + RunningAppProcessInfo info = new RunningAppProcessInfo(); + info.pid = Process.myPid(); + info.importance = importance; + shadow.setProcesses(Collections.singletonList(info)); + } + + @Test + public void capture_nullContext_returnsUnknown() { + ProcessStartCause cause = ProcessStartCause.capture(null); + + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.importance).isEqualTo(-1); + } + + @Test + @Config(sdk = 33) + public void capture_preApi34_returnsUnknownButRecordsImportance() { + setProcessImportance(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); + + ProcessStartCause cause = ProcessStartCause.capture(appContext); + + // Pre-API-34: classification is owned by the legacy AppStartTrace path. + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.apiLevel).isEqualTo(33); + assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); + } + + @Test + @Config(sdk = 34) + public void capture_api34_foregroundImportance_classifiesForeground() { + setProcessImportance(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); + + ProcessStartCause cause = ProcessStartCause.capture(appContext); + + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.FOREGROUND); + assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); + assertThat(cause.apiLevel).isEqualTo(34); + } + + @Test + @Config(sdk = 34) + public void capture_api34_serviceImportance_classifiesUnknown() { + setProcessImportance(RunningAppProcessInfo.IMPORTANCE_SERVICE); + + ProcessStartCause cause = ProcessStartCause.capture(appContext); + + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_SERVICE); + } + + @Test + @Config(sdk = 34) + public void capture_api34_cachedImportance_classifiesUnknown() { + setProcessImportance(RunningAppProcessInfo.IMPORTANCE_CACHED); + + ProcessStartCause cause = ProcessStartCause.capture(appContext); + + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_CACHED); + } + + @Test + public void constructor_visibleForTesting_preservesAllFields() { + ProcessStartCause cause = + new ProcessStartCause( + ProcessStartCause.Cause.FOREGROUND, RunningAppProcessInfo.IMPORTANCE_FOREGROUND, 35); + + assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.FOREGROUND); + assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); + assertThat(cause.apiLevel).isEqualTo(35); + } +} From ff71dfa5baf38e3eb4113a6f8090f497bafcaa8f Mon Sep 17 00:00:00 2001 From: Joseph Rodiz Date: Mon, 15 Jun 2026 12:17:54 -0600 Subject: [PATCH 2/2] Address PR #8202 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename ProcessStartCause → AppStartCause (class, field, test helpers/class) per review, and document resolveIsStartedFromBackground: the pre-API-34 branch is the only one that checks/clears mainThreadRunnable; the API 34+ branch is driven by the OS-reported cause. --- ...cessStartCause.java => AppStartCause.java} | 14 ++++---- .../firebase/perf/metrics/AppStartTrace.java | 24 ++++++++------ ...tCauseTest.java => AppStartCauseTest.java} | 32 +++++++++---------- .../perf/metrics/AppStartTraceTest.java | 14 ++++---- 4 files changed, 44 insertions(+), 40 deletions(-) rename firebase-perf/src/main/java/com/google/firebase/perf/metrics/{ProcessStartCause.java => AppStartCause.java} (86%) rename firebase-perf/src/test/java/com/google/firebase/perf/metrics/{ProcessStartCauseTest.java => AppStartCauseTest.java} (76%) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartCause.java similarity index 86% rename from firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java rename to firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartCause.java index ca52a65c863..65705e3ec08 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/ProcessStartCause.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartCause.java @@ -33,7 +33,7 @@ * * @hide */ -final class ProcessStartCause { +final class AppStartCause { /** Classification of why the process was forked. */ enum Cause { @@ -53,7 +53,7 @@ enum Cause { final int apiLevel; @VisibleForTesting - ProcessStartCause(@NonNull Cause cause, int importance, int apiLevel) { + AppStartCause(@NonNull Cause cause, int importance, int apiLevel) { this.cause = cause; this.importance = importance; this.apiLevel = apiLevel; @@ -64,16 +64,16 @@ enum Cause { * {@code AppStartTrace.registerActivityLifecycleCallbacks}) so the OS-set values still * reflect the original fork reason rather than transient state mid-init. */ - static @NonNull ProcessStartCause capture(@Nullable Context appContext) { + static @NonNull AppStartCause capture(@Nullable Context appContext) { final int apiLevel = Build.VERSION.SDK_INT; if (appContext == null) { - return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); + return new AppStartCause(Cause.UNKNOWN, -1, apiLevel); } final ActivityManager activityManager = (ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE); if (activityManager == null) { - return new ProcessStartCause(Cause.UNKNOWN, -1, apiLevel); + return new AppStartCause(Cause.UNKNOWN, -1, apiLevel); } final int importance = readImportance(); @@ -83,11 +83,11 @@ enum Cause { importance == ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND ? Cause.FOREGROUND : Cause.UNKNOWN; - return new ProcessStartCause(cause, importance, apiLevel); + return new AppStartCause(cause, importance, apiLevel); } // API < 34: legacy AppStartTrace logic owns the decision. - return new ProcessStartCause(Cause.UNKNOWN, importance, apiLevel); + return new AppStartCause(Cause.UNKNOWN, importance, apiLevel); } private static int readImportance() { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index e2769d58184..f592008f86c 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -132,7 +132,7 @@ public class AppStartTrace implements ActivityLifecycleCallbacks, LifecycleObser // OS-reported reason this process was forked. Captured once during // registerActivityLifecycleCallbacks; consulted by resolveIsStartedFromBackground on // API 34+. - private @Nullable ProcessStartCause processStartCause = null; + private @Nullable AppStartCause appStartCause = null; /** * Called from onCreate() method of an activity by instrumented byte code. @@ -226,7 +226,7 @@ public synchronized void registerActivityLifecycleCallbacks(@NonNull Context con systemForegroundCheck = systemForegroundCheck || isAnyAppProcessInForeground(appContext); // Capture the OS-reported start cause as early as possible (this method runs from // FirebasePerfEarly during the ContentProvider init chain). - processStartCause = ProcessStartCause.capture(appContext); + appStartCause = AppStartCause.capture(appContext); isRegisteredForLifecycleCallbacks = true; this.appContext = appContext; } @@ -337,12 +337,16 @@ private void recordOnDrawFrontOfQueue() { * API < 34: legacy pre-bug ordering. If {@link StartFromBackgroundRunnable} fired * before the first {@code onActivityCreated}, suppress. * - * API 34+: {@link ProcessStartCause} owns the decision. {@code FOREGROUND} lets the + * API 34+: {@link AppStartCause} owns the decision. {@code FOREGROUND} lets the * trace through; {@code UNKNOWN} or null suppresses. * * See b/339891952 and https://github.com/firebase/firebase-android-sdk/issues/8103. */ private void resolveIsStartedFromBackground() { + // Only on API < 34 do we consult/consume mainThreadRunnableTime: if the + // StartFromBackgroundRunnable already fired (time is set), this was a background-only + // start, so suppress. Clear it afterwards so the check runs once. On API 34+ this field + // is never read or written here — appStartCause owns the decision below. if (Build.VERSION.SDK_INT < 34) { if (mainThreadRunnableTime != null) { isStartedFromBackground = true; @@ -350,8 +354,10 @@ private void resolveIsStartedFromBackground() { } return; } - if (processStartCause == null - || processStartCause.cause != ProcessStartCause.Cause.FOREGROUND) { + // API 34+: the timing-window heuristic is gone. The OS-reported start cause decides: + // only a FOREGROUND cause (process forked to launch an activity) lets the trace + // through. A null cause (capture failed) or any non-FOREGROUND cause suppresses. + if (appStartCause == null || appStartCause.cause != AppStartCause.Cause.FOREGROUND) { isStartedFromBackground = true; } } @@ -631,13 +637,13 @@ void setMainThreadRunnableTime(Timer timer) { } @VisibleForTesting - void setProcessStartCauseForTest(@Nullable ProcessStartCause cause) { - this.processStartCause = cause; + void setAppStartCauseForTest(@Nullable AppStartCause cause) { + this.appStartCause = cause; } @VisibleForTesting @Nullable - ProcessStartCause getProcessStartCauseForTest() { - return processStartCause; + AppStartCause getAppStartCauseForTest() { + return appStartCause; } } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartCauseTest.java similarity index 76% rename from firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java rename to firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartCauseTest.java index fb5b63333d6..8b817ef5b56 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/ProcessStartCauseTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartCauseTest.java @@ -29,9 +29,9 @@ import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowActivityManager; -/** Unit tests for {@link ProcessStartCause}. */ +/** Unit tests for {@link AppStartCause}. */ @RunWith(RobolectricTestRunner.class) -public class ProcessStartCauseTest { +public class AppStartCauseTest { private final Context appContext = ApplicationProvider.getApplicationContext(); @@ -47,9 +47,9 @@ private void setProcessImportance(int importance) { @Test public void capture_nullContext_returnsUnknown() { - ProcessStartCause cause = ProcessStartCause.capture(null); + AppStartCause cause = AppStartCause.capture(null); - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.UNKNOWN); assertThat(cause.importance).isEqualTo(-1); } @@ -58,10 +58,10 @@ public void capture_nullContext_returnsUnknown() { public void capture_preApi34_returnsUnknownButRecordsImportance() { setProcessImportance(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); - ProcessStartCause cause = ProcessStartCause.capture(appContext); + AppStartCause cause = AppStartCause.capture(appContext); // Pre-API-34: classification is owned by the legacy AppStartTrace path. - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.UNKNOWN); assertThat(cause.apiLevel).isEqualTo(33); assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); } @@ -71,9 +71,9 @@ public void capture_preApi34_returnsUnknownButRecordsImportance() { public void capture_api34_foregroundImportance_classifiesForeground() { setProcessImportance(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); - ProcessStartCause cause = ProcessStartCause.capture(appContext); + AppStartCause cause = AppStartCause.capture(appContext); - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.FOREGROUND); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.FOREGROUND); assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); assertThat(cause.apiLevel).isEqualTo(34); } @@ -83,9 +83,9 @@ public void capture_api34_foregroundImportance_classifiesForeground() { public void capture_api34_serviceImportance_classifiesUnknown() { setProcessImportance(RunningAppProcessInfo.IMPORTANCE_SERVICE); - ProcessStartCause cause = ProcessStartCause.capture(appContext); + AppStartCause cause = AppStartCause.capture(appContext); - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.UNKNOWN); assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_SERVICE); } @@ -94,19 +94,19 @@ public void capture_api34_serviceImportance_classifiesUnknown() { public void capture_api34_cachedImportance_classifiesUnknown() { setProcessImportance(RunningAppProcessInfo.IMPORTANCE_CACHED); - ProcessStartCause cause = ProcessStartCause.capture(appContext); + AppStartCause cause = AppStartCause.capture(appContext); - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.UNKNOWN); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.UNKNOWN); assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_CACHED); } @Test public void constructor_visibleForTesting_preservesAllFields() { - ProcessStartCause cause = - new ProcessStartCause( - ProcessStartCause.Cause.FOREGROUND, RunningAppProcessInfo.IMPORTANCE_FOREGROUND, 35); + AppStartCause cause = + new AppStartCause( + AppStartCause.Cause.FOREGROUND, RunningAppProcessInfo.IMPORTANCE_FOREGROUND, 35); - assertThat(cause.cause).isEqualTo(ProcessStartCause.Cause.FOREGROUND); + assertThat(cause.cause).isEqualTo(AppStartCause.Cause.FOREGROUND); assertThat(cause.importance).isEqualTo(RunningAppProcessInfo.IMPORTANCE_FOREGROUND); assertThat(cause.apiLevel).isEqualTo(35); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java index cf744531d8a..5037db9a25d 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java @@ -296,7 +296,7 @@ public void preApi34_runnableNotFired_traceLogs() { } // --- API 34+ causal-signal decision tests --- - // ProcessStartCause is the only decision input on API 34+; exercise each Cause value. + // AppStartCause is the only decision input on API 34+; exercise each Cause value. /** Builds an {@link AppStartTrace} and registers callbacks. */ private AppStartTrace newTrace(FakeScheduledExecutorService executor) { @@ -309,8 +309,7 @@ private AppStartTrace newTrace(FakeScheduledExecutorService executor) { public void api34Plus_foregroundCause_traceLogs() { FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); AppStartTrace trace = newTrace(executor); - trace.setProcessStartCauseForTest( - new ProcessStartCause(ProcessStartCause.Cause.FOREGROUND, 100, 35)); + trace.setAppStartCauseForTest(new AppStartCause(AppStartCause.Cause.FOREGROUND, 100, 35)); currentTime = 1; trace.onActivityCreated(activity1, bundle); @@ -335,8 +334,7 @@ public void api34Plus_unknownCause_traceSuppressed() { // scenario. Suppress to keep _app_start measuring real cold foreground launches. FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); AppStartTrace trace = newTrace(executor); - trace.setProcessStartCauseForTest( - new ProcessStartCause(ProcessStartCause.Cause.UNKNOWN, 200, 34)); + trace.setAppStartCauseForTest(new AppStartCause(AppStartCause.Cause.UNKNOWN, 200, 34)); trace.onActivityCreated(activity1, bundle); @@ -349,13 +347,13 @@ public void api34Plus_unknownCause_traceSuppressed() { } @Test - public void api34Plus_nullProcessStartCause_traceSuppressed() { - // Defensive: if processStartCause is somehow null at decision time (e.g. the + public void api34Plus_nullAppStartCause_traceSuppressed() { + // Defensive: if appStartCause is somehow null at decision time (e.g. the // capture didn't run), suppress — better to miss a trace than emit one with no // provenance. FakeScheduledExecutorService executor = new FakeScheduledExecutorService(); AppStartTrace trace = newTrace(executor); - trace.setProcessStartCauseForTest(null); + trace.setAppStartCauseForTest(null); trace.onActivityCreated(activity1, bundle);