From ae8e87e9ce3a43c02b1be2923c1ccd503ae04bfc Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 7 Jul 2026 11:03:17 +0530 Subject: [PATCH 1/3] fix: [SDK-4845] always finish base notification trampoline via finally NotificationOpenedActivityBase called AndroidUtils.finishSafely only on the success path: an init failure early-returned and a thrown processFromContext both skipped it, stranding the transient trampoline activity. This is the same early-return-before-finish shape removed from the HMS trampoline in #2678. Wrap the coroutine body in try/finally so finishSafely always runs on the main thread after processing, an early return, or an exception. Keeping finish in finally (after processFromContext) preserves the Xiaomi startActivity()-before-finish() ordering. Co-authored-by: Cursor --- .../NotificationOpenedActivityBase.kt | 31 +++++---- .../NotificationOpenedActivityBaseTest.kt | 65 +++++++++++++++++++ 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt index 1080e064d..daedcb654 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt @@ -58,21 +58,24 @@ abstract class NotificationOpenedActivityBase : // warm dispatchers before the first suspendifyOnDefault dispatch. OneSignalDispatchers.prewarm() suspendifyOnDefault { - if (!OneSignal.initWithContext(applicationContext)) { - return@suspendifyOnDefault - } - - val openedProcessor = OneSignal.getService() - openedProcessor.processFromContext(this, intent) - // KEEP: Xiaomi Compatibility: - // Must keep this Activity alive while trampolining, that is - // startActivity() must be called BEFORE finish(), otherwise - // the app is never foregrounded. + try { + if (!OneSignal.initWithContext(applicationContext)) { + return@suspendifyOnDefault + } - // Safely finish the activity on the main thread after processing is complete. - // This gives the system enough time to complete rendering before closing the Trampoline activity. - runOnUiThread { - AndroidUtils.finishSafely(this) + val openedProcessor = OneSignal.getService() + openedProcessor.processFromContext(this, intent) + // KEEP: Xiaomi Compatibility: + // Must keep this Activity alive while trampolining, that is + // startActivity() must be called BEFORE finish(), otherwise + // the app is never foregrounded. + } finally { + // Finish in finally, on the main thread, so the trampoline is always dismissed — + // including when init fails, we early-return, or processing throws. Running after + // processing (not before) preserves the Xiaomi startActivity()-before-finish() ordering. + runOnUiThread { + AndroidUtils.finishSafely(this) + } } } } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt index d7032f78b..8c6a30b2f 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt @@ -1,18 +1,29 @@ package com.onesignal.notifications.activities +import android.content.Context import android.content.Intent import android.os.Bundle import android.os.Handler import android.os.Looper import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest +import com.onesignal.OneSignal +import com.onesignal.common.services.IServiceProvider import com.onesignal.common.threading.OneSignalDispatchers import com.onesignal.common.threading.suspendifyOnDefault import com.onesignal.mocks.IOMockHelper +import com.onesignal.notifications.internal.open.INotificationOpenedProcessor import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.clearMocks +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkObject import io.mockk.verify import io.mockk.verifyOrder +import kotlinx.coroutines.runBlocking import org.robolectric.Robolectric import org.robolectric.shadows.ShadowLooper @@ -78,4 +89,58 @@ class NotificationOpenedActivityTest : FunSpec({ activity.wasProcessIntentCalled shouldBe true } + + // Regression for SDK-4845: the base/GMS trampoline must always be finished after processing, + // and must not be finished before processing runs. suspendifyOnDefault is stubbed to capture + // (not run) its block so we can control exactly when the coroutine body executes. + test("does not finish the base trampoline until after open processing runs") { + val serviceProvider = mockk(relaxed = true) + val processor = mockk(relaxed = true) + every { serviceProvider.getService(INotificationOpenedProcessor::class.java) } returns processor + mockkObject(OneSignal) + try { + every { OneSignal.services } returns serviceProvider + coEvery { OneSignal.initWithContext(any()) } returns true + + var capturedBlock: (suspend () -> Unit)? = null + every { suspendifyOnDefault(any()) } answers { capturedBlock = firstArg() } + + val activity = Robolectric.buildActivity(TestNotificationOpenedActivity::class.java).setup().get() + + // The synchronous part of processIntent must NOT finish the activity or run processing yet. + activity.isFinishing shouldBe false + coVerify(exactly = 0) { processor.processFromContext(any(), any()) } + + runBlocking { capturedBlock!!.invoke() } + + coVerify(exactly = 1) { processor.processFromContext(activity, any()) } + activity.isFinishing shouldBe true + } finally { + unmockkObject(OneSignal) + } + } + + test("always finishes the base trampoline even when initialization fails") { + val serviceProvider = mockk(relaxed = true) + val processor = mockk(relaxed = true) + every { serviceProvider.getService(INotificationOpenedProcessor::class.java) } returns processor + mockkObject(OneSignal) + try { + every { OneSignal.services } returns serviceProvider + coEvery { OneSignal.initWithContext(any()) } returns false + + var capturedBlock: (suspend () -> Unit)? = null + every { suspendifyOnDefault(any()) } answers { capturedBlock = firstArg() } + + val activity = Robolectric.buildActivity(TestNotificationOpenedActivity::class.java).setup().get() + + runBlocking { capturedBlock!!.invoke() } + + // Early return on init failure must still dismiss the trampoline via the finally block. + coVerify(exactly = 0) { processor.processFromContext(any(), any()) } + activity.isFinishing shouldBe true + } finally { + unmockkObject(OneSignal) + } + } }) From f1247d03ba84dc0101bf2c213f507316780af996 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 7 Jul 2026 11:33:58 +0530 Subject: [PATCH 2/3] test: [SDK-4845] add throws-path regression and clarify trampoline test intent Address review feedback on the base-trampoline finally fix: - Rename the success-path test to reflect it is an ordering guard (finish after processFromContext), not the core regression. - Add a regression test asserting the trampoline is still finished when processFromContext throws (the exception propagates, but finally must have finished the activity first). - Drain the main-thread looper before asserting isFinishing so the assertions don't rely on runOnUiThread happening to run synchronously. Co-authored-by: Cursor --- .../NotificationOpenedActivityBaseTest.kt | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt index 8c6a30b2f..d9749501e 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt @@ -90,10 +90,11 @@ class NotificationOpenedActivityTest : FunSpec({ activity.wasProcessIntentCalled shouldBe true } - // Regression for SDK-4845: the base/GMS trampoline must always be finished after processing, - // and must not be finished before processing runs. suspendifyOnDefault is stubbed to capture - // (not run) its block so we can control exactly when the coroutine body executes. - test("does not finish the base trampoline until after open processing runs") { + // Ordering guard (not the SDK-4845 regression itself): the trampoline must not be finished by + // the synchronous part of processIntent, and on the success path finish() must run only after + // processFromContext (preserving the Xiaomi startActivity()-before-finish() ordering). + // suspendifyOnDefault is stubbed to capture (not run) its block so we control when it executes. + test("finishes the base trampoline only after open processing completes") { val serviceProvider = mockk(relaxed = true) val processor = mockk(relaxed = true) every { serviceProvider.getService(INotificationOpenedProcessor::class.java) } returns processor @@ -112,6 +113,7 @@ class NotificationOpenedActivityTest : FunSpec({ coVerify(exactly = 0) { processor.processFromContext(any(), any()) } runBlocking { capturedBlock!!.invoke() } + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() coVerify(exactly = 1) { processor.processFromContext(activity, any()) } activity.isFinishing shouldBe true @@ -120,6 +122,7 @@ class NotificationOpenedActivityTest : FunSpec({ } } + // Core SDK-4845 regression: the init-failure early-return path must still dismiss the trampoline. test("always finishes the base trampoline even when initialization fails") { val serviceProvider = mockk(relaxed = true) val processor = mockk(relaxed = true) @@ -135,6 +138,7 @@ class NotificationOpenedActivityTest : FunSpec({ val activity = Robolectric.buildActivity(TestNotificationOpenedActivity::class.java).setup().get() runBlocking { capturedBlock!!.invoke() } + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() // Early return on init failure must still dismiss the trampoline via the finally block. coVerify(exactly = 0) { processor.processFromContext(any(), any()) } @@ -143,4 +147,33 @@ class NotificationOpenedActivityTest : FunSpec({ unmockkObject(OneSignal) } } + + // Core SDK-4845 regression: when open processing throws, the trampoline must still be dismissed. + // The exception propagates (suspendifyWithCompletion catches/logs it in production), but the + // finally block must have finished the activity before it unwinds. + test("always finishes the base trampoline even when open processing throws") { + val serviceProvider = mockk(relaxed = true) + val processor = mockk(relaxed = true) + every { serviceProvider.getService(INotificationOpenedProcessor::class.java) } returns processor + coEvery { processor.processFromContext(any(), any()) } throws RuntimeException("boom") + mockkObject(OneSignal) + try { + every { OneSignal.services } returns serviceProvider + coEvery { OneSignal.initWithContext(any()) } returns true + + var capturedBlock: (suspend () -> Unit)? = null + every { suspendifyOnDefault(any()) } answers { capturedBlock = firstArg() } + + val activity = Robolectric.buildActivity(TestNotificationOpenedActivity::class.java).setup().get() + + val result = runCatching { runBlocking { capturedBlock!!.invoke() } } + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() + + result.isFailure shouldBe true + coVerify(exactly = 1) { processor.processFromContext(activity, any()) } + activity.isFinishing shouldBe true + } finally { + unmockkObject(OneSignal) + } + } }) From 15064e0bc9bc6c6b56c0a527a5efba57f714b863 Mon Sep 17 00:00:00 2001 From: AR Abdul Azeez Date: Tue, 7 Jul 2026 11:39:38 +0530 Subject: [PATCH 3/3] docs: trim trampoline comments and drop ticket refs Keep the comments concise and free of ticket numbers. Co-authored-by: Cursor --- .../activities/NotificationOpenedActivityBase.kt | 5 ++--- .../NotificationOpenedActivityBaseTest.kt | 13 +++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt index daedcb654..2b3beb5b6 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/activities/NotificationOpenedActivityBase.kt @@ -70,9 +70,8 @@ abstract class NotificationOpenedActivityBase : // startActivity() must be called BEFORE finish(), otherwise // the app is never foregrounded. } finally { - // Finish in finally, on the main thread, so the trampoline is always dismissed — - // including when init fails, we early-return, or processing throws. Running after - // processing (not before) preserves the Xiaomi startActivity()-before-finish() ordering. + // Always dismiss the trampoline, even if init fails or processing throws. + // Finishing after processing (not before) keeps the app foregrounding correctly on Xiaomi. runOnUiThread { AndroidUtils.finishSafely(this) } diff --git a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt index d9749501e..c580ec47f 100644 --- a/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt +++ b/OneSignalSDK/onesignal/notifications/src/test/java/com/onesignal/notifications/activities/NotificationOpenedActivityBaseTest.kt @@ -90,10 +90,8 @@ class NotificationOpenedActivityTest : FunSpec({ activity.wasProcessIntentCalled shouldBe true } - // Ordering guard (not the SDK-4845 regression itself): the trampoline must not be finished by - // the synchronous part of processIntent, and on the success path finish() must run only after - // processFromContext (preserving the Xiaomi startActivity()-before-finish() ordering). - // suspendifyOnDefault is stubbed to capture (not run) its block so we control when it executes. + // Ordering guard: finish() must run only after processFromContext, never from the synchronous + // part of processIntent. suspendifyOnDefault is captured (not run) so we control when it executes. test("finishes the base trampoline only after open processing completes") { val serviceProvider = mockk(relaxed = true) val processor = mockk(relaxed = true) @@ -122,7 +120,7 @@ class NotificationOpenedActivityTest : FunSpec({ } } - // Core SDK-4845 regression: the init-failure early-return path must still dismiss the trampoline. + // The init-failure early return must still dismiss the trampoline. test("always finishes the base trampoline even when initialization fails") { val serviceProvider = mockk(relaxed = true) val processor = mockk(relaxed = true) @@ -148,9 +146,8 @@ class NotificationOpenedActivityTest : FunSpec({ } } - // Core SDK-4845 regression: when open processing throws, the trampoline must still be dismissed. - // The exception propagates (suspendifyWithCompletion catches/logs it in production), but the - // finally block must have finished the activity before it unwinds. + // When processing throws, the exception still propagates (production logs it via + // suspendifyWithCompletion), but the trampoline must be finished before it unwinds. test("always finishes the base trampoline even when open processing throws") { val serviceProvider = mockk(relaxed = true) val processor = mockk(relaxed = true)