Conversation
fix HTML banner crash and remove deprecated ConstraintLayout attr
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| bannerPopup.init(); | ||
| bannerPopup.show(); | ||
|
|
||
| sharedPreferences.edit().putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, true).apply(); |
There was a problem hiding this comment.
Race condition: flag set after show instead of before
The APP_BANNER_SHOWING_IN_PROGRESS flag is set at line 1910 after bannerPopup.show() is called, but it's checked at lines 1860-1861 to prevent concurrent banner display. Since show() starts an AsyncTask and the flag is set afterward, there's a window where multiple concurrent calls could pass the check before any of them sets the flag. This defeats the intended purpose of preventing overlapping banner displays. The flag needs to be set before calling show() to properly protect the critical section.
Additional Locations (1)
| bannerPopup.init(); | ||
| bannerPopup.show(); | ||
|
|
||
| sharedPreferences.edit().putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, true).apply(); |
There was a problem hiding this comment.
Flag stuck true when async banner display fails
The APP_BANNER_SHOWING_IN_PROGRESS flag is set to true at line 1910 but only reset in synchronous catch blocks and toggleShowing(false). If the tryShowSafe AsyncTask fails — either because the activity becomes invalid (finishing/destroyed) before the banner loads, or an exception occurs in displayBanner/animateBody — the popup never displays, the dismiss listener never fires, and the flag remains true. This blocks all future non-forced banners until app restart when the flag is reset in the constructor.
fix HTML banner crash and remove deprecated ConstraintLayout attr
Note
Prevents concurrent in-app banner display with a new
APP_BANNER_SHOWING_IN_PROGRESSflag, fixes HTML banner JS injection using safe regex replacement, and removes a deprecated ConstraintLayout attribute.CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESSto gate showing banners (AppBannerModule,AppBannerPopup).showBannerlifecycle and on exceptions; skip when showing or in-progress.String.replaceAllwithPattern/Matcher.replaceAll+Matcher.quoteReplacementwhen injecting JS before</body>inAppBannerCarouselAdapterandInboxDetailBannerCarouselAdapter.app:layout_constraintWidth_defaultfromres/layout/app_banner_html.xml.Written by Cursor Bugbot for commit 2085db7. This will update automatically on new commits. Configure here.
Summary by cubic
Fix crash when rendering HTML app banners and prevent overlapping banner displays. Addresses Linear CP-10490 during in-app banner testing.
Written for commit 2085db7. Summary will update automatically on new commits.