Appboy / appboy-android-sdk

Public repo for the Braze Android SDK
https://www.braze.com
Other
153 stars 102 forks source link

RaceCondition NotificationTrampolineActivity? #204

Closed michaeltiket closed 2 years ago

michaeltiket commented 2 years ago

Standard Reporting

What version of the Braze Android SDK are you using?

Braze -> 13.1.2 Test on Android P. Xiaomi mi 8

What did you do?

val config = AppboyConfig.Builder()
            .setIsFirebaseCloudMessagingRegistrationEnabled(true)
            .setFirebaseCloudMessagingSenderIdKey(/*..*/)
            .setApiKey(/*..*/)
            .setHandlePushDeepLinksAutomatically(true)
            .setCustomEndpoint(/*..*/)
            .setPushHtmlRenderingEnabled(true)
            .setPushDeepLinkBackStackActivityEnabled(false)
            .build()

plus registering AppboyLifecycleCallbackListener(true, true)

What did you expect to happen?

Should open app (Deeplink Handler or home). Or can we override/replace NotificationTrampolineActivity?

What happened instead?

Doesn't open app. I think there is some race condition on when NotificationTrampolineActivity sendBroadcast and finish, because the NotificationTrampolineActivity is finish, there is no root and when startActivity from UriAction nothing happen? because when i set breakpoint on finish and wait a little time, the notif works fine. when i see new update on braze 17.0.0 that changes by adding delay 200 ms, in my case solve, but i doubt the solution because may cause some intermittent same bug. is there any proper fix?

Steps to reproduce

Are you doing any feature customizations that may relate to the issue? Can you share the code snippet?

nope but i try to simulate the behaviour on sample app.

Project that demonstrates the issue

https://github.com/cyrax1800/sampe_broadcast On my sample app should open the Activity with text "Second"

ANR Reporting

Entire ANR Report

Braze Related Threads In ANR

radixdev commented 2 years ago

Hi @michaeltiket ,

You are correct that the original logic in 13.1.2 could result in no root activity being available when startActivity was called. The fix for that behavior was to call BrazePushReceiver.handleReceivedIntent(this, sendIntent, false); which runs the push handler code synchronously so the NotificationTrampolineActivity is still in the foreground when startActivity is eventually called. For this reason, being on 17.0.0 is perfectly fine if you were seeing that bug occur on that OS. This is indeed the proper fix.

The 200 ms delay has nothing to do with the startActivity issue you were seeing. At that point in the code, the target intent was already fired and thus the NotificationTrampolineActivity.onPause() method also called. The delay is just to ensure that the NotificationTrampolineActivity is finally closed no matter what.