fix: [SDK-4732] track activity lifecycle from process start to fix late-init focus#2655
Open
fadi-george wants to merge 9 commits into
Open
fix: [SDK-4732] track activity lifecycle from process start to fix late-init focus#2655fadi-george wants to merge 9 commits into
fadi-george wants to merge 9 commits into
Conversation
… observer When the lifecycle observer is registered at process start, late init skips focus seeding, leaving nextResumeIsFirstActivity false. On a cold start from a notification the trampoline is ignored and the notification module sets entryState=NOTIFICATION_CLICK before the host activity launches, so the wasInBackground guard evaluates false and onFocus is never delivered to SessionService, IAM, outcomes, and other consumers. Arm the first-foreground flag when no activity has been observed yet so the first real start fires focus while preserving the NOTIFICATION_CLICK entry state. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…rt notifications The previous fix armed the first-foreground flag in start(), which only covers the cold-start path. On a warm start from a notification the SDK is already initialized, so start() never runs: the app was backgrounded (current is null), the notification module sets entryState=NOTIFICATION_CLICK, and the host activity's onActivityStarted reads wasInBackground as false, so onFocus is never delivered. Move the arming to onActivityStarted — the single choke point for every foreground entry — so the first non-internal activity start after no foreground activity (cold or warm) fires focus. handleFocus still preserves the NOTIFICATION_CLICK entry state. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)0/75 touched executable lines covered (0.0% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 0.0% (minimum 80%). |
… taking focus Excluding OneSignalInternalActivity from all tracking regressed two notification paths. (1) Foreground tap: the trampoline launches in its own task (taskAffinity=""), so the host stops while it is on top; without the trampoline's prior contribution the reference count crossed zero, firing a spurious onUnfocused and letting the async open handler set NOTIFICATION_CLICK on an organic foreground tap. (2) URL/browser notifications never launch the host, so the trampoline finished alone and left entryState stuck at NOTIFICATION_CLICK, mis-attributing the next organic open as a direct notification session. Track started internal activities separately: they still never become current, take focus, or set entry state, but while one is on top a real activity's stop is treated as transient (the host resumes when the trampoline finishes), and when the trampoline finishes with no current activity a stale NOTIFICATION_CLICK is reset to APP_CLOSE. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@claude review |
Contributor
Author
|
@claude review |
8 tasks
… was uncounted A notification-open trampoline can reach onStop without its onStart ever being counted (e.g. a wrapper SDK that disabled the process-start initializer, so the trampoline started before the lifecycle observer attached). That hit the !wasCounted early return in decrementStartedActivity and skipped the NOTIFICATION_CLICK -> APP_CLOSE reset, leaving entry state stale and mis-attributing the next organic launch as a direct notification session. Extract resetStaleNotificationEntryIfBackgrounded and also run it in the early-return path (guarded on internal + no current + NOTIFICATION_CLICK, no onUnfocused). Real uncounted activity stops are unaffected. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
Register the activity lifecycle observer at process start (via
androidx.startup) and make the activity reference counter membership- and config-change-aware, so focus/entry-state tracking is correct regardless of when the SDK initializes.Details
Motivation
Wrapper SDKs (Flutter, React Native, Capacitor) have to add their own workarounds because
ApplicationServiceonly begins observing the activity lifecycle onceOneSignal.initWithContextruns. When init happens after the first activity is already started, the SDK misses lifecycle callbacks, which produces:entryState(e.g.NOTIFICATION_CLICKreported for an organic cold start),activityReferencescount that can underflow and falsely drop focus, andThis consolidates the fix in the native SDK so wrappers can drop their nudges.
Scope
androidx.startupInitializerattaches theActivityLifecycleCallbacksto theApplicationbefore init runs. The same process-wideApplicationServiceinstance is then resolved by DI, so lifecycle observed before init is visible to the running SDK. If the startup provider is disabled (or in unit tests), DI falls back to a fresh instance andstart()seeds focus state from the init context as before.WeakHashMap-backed set): a stop only decrements for an activity whose start was actually counted, preventing underflow when a late-init SDK observes a transient activity's stop without its start.OneSignalInternalActivitymarker and are excluded from focus/entry-state tracking.Testing
Unit testing
Added
ApplicationServiceTestscases covering: internal trampoline activities are ignored for focus/current/entry-state; stopping an activity whose start was never counted does not drop focus (no underflow); and configuration-change recreation does not inflate the reference count.Manual testing
Built the SDK to
mavenLocaland ran the OneSignal example app (gms variant) on an Android 16 emulator, driving scenarios via a helper script:entryState=APP_OPEN(notNOTIFICATION_CLICK), oneonFocus, count settles at 1.onUnfocusedthen oneonFocus, count returns to 1 with no underflow.onUnfocused/onFocusandentryStatepreserved.Affected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor