fix: [SDK-4746] register Android click listener lazily to replay cold-start clicks

[sherwinski]

Description

One Line Summary

Defer registering the native Android notification click listener until the first C# Clicked subscriber, so cold-start notification clicks are replayed instead of silently dropped.

Details

Motivation

Fixes #873. When an Android app is opened by tapping a push notification from a killed state (cold start), the click event — and its Additional Data — never reached the app.

The Android bridge registered the native click listener eagerly inside OneSignal.Initialize(). The native Android SDK queues a cold-start click and replays it the moment a listener is added, but at that point no C# Clicked handler was subscribed yet (apps subscribe after Initialize(), as our samples do), so the bridge discarded the replayed event.

This PR adopts the same lazy-registration pattern the iOS bridge already uses: the native addClickListener call now happens on the first C# Clicked subscription, after the handler is attached, so the native SDK's queued cold-start click replays into a live handler.

Scope

Android only, notification click events only. ForegroundWillDisplay and PermissionChanged registration is unchanged. No public API changes — OneSignal.Notifications.Clicked behaves the same, except cold-start clicks now fire. iOS is untouched (it already worked this way).

Testing

Unit testing

None added — the repo has no unit test infrastructure, and this change is a thin JNI-registration ordering change.

Manual testing

Ran a before-and-after comparison with the demo app (examples/demo), built with Unity 6000.3.6f1 (IL2CPP/ARM64) and installed on a Pixel 9 emulator (API 37, Google Play image). "Before" = main, "After" = this branch. To simulate real apps that subscribe to SDK events later than Initialize() (the stock demo subscribes ~100ms after, a window small enough that the replayed event usually sneaks through), a temporary 5-second delay was added before RegisterSdkListeners() in both builds.

Reproduction steps

1. Simulate late subscription. In examples/demo/Assets/Scripts/AppBootstrapper.cs, add a delay before listener registration in Start() (this mimics apps that subscribe to Clicked in a later scene/frame, like the reporter's):

   await System.Threading.Tasks.Task.Delay(5000);
   RegisterSdkListeners();

   Optionally, log the click payload in OnNotificationClicked via Newtonsoft.Json.JsonConvert.SerializeObject(e.Notification.AdditionalData) (note: JsonUtility.ToJson won't show AdditionalData — it can't serialize dictionaries).

2. Build and install on a booted emulator (Google Play image required for FCM):

   cd examples/demo && ./run-android.sh

3. Subscribe the device. Launch the app, accept the notification permission prompt, and wait for the push subscription to appear. Grab the subscription ID from the app UI (Push ID) or from logcat (adb logcat | grep -o 'OneSignal-Subscription-Id=\[[a-f0-9-]*\]').

4. Kill the app process (kill only — force-stop would put the app in stopped state, which blocks FCM delivery):

   adb shell input keyevent KEYCODE_HOME
   adb shell am kill com.onesignal.example
   adb shell pidof com.onesignal.example   # should print nothing

5. Send a push with Additional Data to the subscription from step 3:

   curl -X POST https://onesignal.com/api/v1/notifications \
     -H "Content-Type: application/json" \
     -d '{"app_id":"<APP_ID>","include_subscription_ids":["<SUBSCRIPTION_ID>"],
          "headings":{"en":"Cold start test"},"contents":{"en":"Tap me"},
          "data":{"playback_data":"episode-42"}}'

6. Tap the notification from the shade (this cold-starts the app) and watch logcat:

   adb logcat | grep -E 'addClickListener|Notification clicked'

   • On main: addClickListener logs during Initialize(), then the listener registration log 5s later — but Notification clicked never appears. The event is dropped.
   • On this branch: addClickListener logs right after the 5s delay (first Clicked +=), immediately followed by Notification clicked with the additionalData payload.

Results

Before (bug reproduced) — the native listener registers during Initialize(), the queued cold-start click replays into a null C# handler and is silently discarded:

09:28:44.165  NotificationsManager.addClickListener(...)   <- during Initialize()
09:28:49.373  [AppBootstrapper] Registering SDK listeners after 5s delay
              (no "Notification clicked" — event lost)

After (fixed) — the native listener registers on first Clicked +=, and the queued cold-start click replays immediately with Additional Data intact:

09:31:45.254  [AppBootstrapper] Registering SDK listeners after 5s delay
09:31:45.256  NotificationsManager.addClickListener(...)   <- lazy, after handler attached
09:31:45.296  [AppBootstrapper] Notification clicked: , additionalData: {"playback_data":"episode-42","test_key":"after-fix-delayed"}

Regression checks on the fixed build (all passing):

• Foreground receive: ForegroundWillDisplay fired and the notification displayed.
• Warm-start click (process alive, app backgrounded): Clicked fired immediately with additionalData: {"test_key":"foreground"}.
• Cold-start click: fired with data, as shown above.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

[sherwinski]

@claude review once

[claude]

LGTM — small, well-scoped fix that mirrors the iOS lazy-registration pattern to unblock cold-start click replay.

Extended reasoning...

Overview

Single-file change in com.onesignal.unity.android/Runtime/AndroidNotificationsManager.cs. The native Android addClickListener call is moved out of Initialize() and into the Clicked event's add accessor, guarded by a _clickNativeListenerSet flag so registration happens exactly once. The event field is backed by a private _clicked delegate, and the internal click handler dispatches through it. ForegroundWillDisplay and PermissionChanged are untouched.

Security risks

None. This is a JNI registration ordering change — no auth, crypto, permissions, network, or data-handling surface is touched. Public API shape is unchanged.

Level of scrutiny

Low. The fix is a mechanical reordering that adopts a pattern already proven on the iOS side, with a clear explanation in both the code comment and the PR description. Cold-start click replay is a documented native-SDK behavior, so the reasoning is verifiable. No new abstractions, no changes to threading semantics beyond what the existing event dispatch already assumes.

Other factors

• The bug hunting system found no issues.
• Manual verification on an Android emulator is documented (cold-start tap now fires Clicked with AdditionalData; warm-start and foreground paths unchanged).
• Non-thread-safe check on _clickNativeListenerSet is acceptable here because Unity event subscription conventionally occurs on the main thread; this matches the surrounding code's assumptions.
• If Clicked never gets a subscriber, the native listener is never registered — this is intentional and matches iOS.

[fadi-george]

Had to do:
adb shell input keyevent KEYCODE_HOME
sleep 2
adb shell am kill com.onesignal.example
adb shell pidof com.onesignal.example

[fadi-george]

Does seem to fix it though I do some small changes.
I’d address these two things in the PR since they’re directly introduced by the lazy registration change:

1. Guard the custom Clicked accessor so concurrent first subscriptions can’t register duplicate native listeners.
2. Set _clickNativeListenerSet = true only after addClickListener succeeds, so a transient JNI failure can retry on the next subscription.

[sherwinski]

Addressed both points in e2db6d9:

1. The Clicked add/remove accessors now synchronize on a private lock, so concurrent first subscriptions can't register duplicate native listeners.
2. _clickNativeListenerSet is now set only after addClickListener returns successfully, so a transient native failure will retry on the next subscription.

The iOS bridge has the same pattern but I can address it in a follow-up PR.

[fadi-george]

Nit: if addClickListener throws, _clicked += value has already succeeded even though the native registration did not. Retrying the same subscription could add the same handler twice.
Could wrap the native registration and roll back _clicked -= value before rethrowing, so failed subscriptions don’t leave partial managed state behind.

[sherwinski]

Applied the nit in f3d5d48 — the native registration is now wrapped in a try/catch that rolls back _clicked -= value before rethrowing, so a failed subscription leaves no partial managed state and a retry can't add the same handler twice.