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..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 @@ -58,21 +58,23 @@ 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 { + // 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 d7032f78b..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 @@ -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,88 @@ class NotificationOpenedActivityTest : FunSpec({ activity.wasProcessIntentCalled shouldBe true } + + // 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) + 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() } + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() + + coVerify(exactly = 1) { processor.processFromContext(activity, any()) } + activity.isFinishing shouldBe true + } finally { + unmockkObject(OneSignal) + } + } + + // 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) + 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() } + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() + + // 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) + } + } + + // 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) + 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) + } + } })