-
Notifications
You must be signed in to change notification settings - Fork 3
hotfix/cp-10490-android-sdk-issue-while-testing-in-app-banners #328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,6 +123,7 @@ private AppBannerModule(String channel, boolean showDrafts, SharedPreferences sh | |
| handlerThread.start(); | ||
| handler = new Handler(handlerThread.getLooper()); | ||
| editor.putBoolean(CleverPushPreferences.APP_BANNER_SHOWING, false); | ||
| editor.putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, false); | ||
| editor.apply(); | ||
| } | ||
|
|
||
|
|
@@ -1856,7 +1857,8 @@ void showBanner(AppBannerPopup bannerPopup, boolean force, AppBannerClosedListen | |
| Banner banner = bannerPopup.getData(); | ||
| setAppBannerClosedListener(appBannerClosedListener); | ||
| if (!force) { | ||
| if (sharedPreferences.getBoolean(CleverPushPreferences.APP_BANNER_SHOWING, false)) { | ||
| if (sharedPreferences.getBoolean(CleverPushPreferences.APP_BANNER_SHOWING, false) | ||
| || sharedPreferences.getBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, false)) { | ||
| pendingFilteredBanners.add(bannerPopup); | ||
| Logger.d(TAG, "Skipping Banner " + banner.getId() + " because: A Banner is already on the screen"); | ||
| return; | ||
|
|
@@ -1905,6 +1907,8 @@ void showBanner(AppBannerPopup bannerPopup, boolean force, AppBannerClosedListen | |
| bannerPopup.init(); | ||
| bannerPopup.show(); | ||
|
|
||
| sharedPreferences.edit().putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, true).apply(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flag stuck true when async banner display failsThe Additional Locations (1) |
||
|
|
||
| getActivityLifecycleListener().setActivityInitializedListener(new ActivityInitializedListener() { | ||
| @Override | ||
| public void initialized() { | ||
|
|
@@ -2006,6 +2010,7 @@ public void initialized() { | |
| getCleverPushInstance().getAppBannerShownListener().shown(banner); | ||
| } | ||
| } catch (Exception ex) { | ||
| sharedPreferences.edit().putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, false).apply(); | ||
| Logger.e(TAG, "Error in showBanner. " + ex.getMessage(), ex); | ||
| } | ||
| } | ||
|
|
@@ -2014,6 +2019,9 @@ void showBanner(AppBannerPopup bannerPopup) { | |
| try { | ||
| showBanner(bannerPopup, false, null, null); | ||
| } catch (Exception ex) { | ||
| try { | ||
| sharedPreferences.edit().putBoolean(CleverPushPreferences.APP_BANNER_SHOWING_IN_PROGRESS, false).apply(); | ||
| } catch (Exception ignore) { } | ||
| Logger.e(TAG, ex.getMessage(), ex); | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition: flag set after show instead of before
The
APP_BANNER_SHOWING_IN_PROGRESSflag is set at line 1910 afterbannerPopup.show()is called, but it's checked at lines 1860-1861 to prevent concurrent banner display. Sinceshow()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 callingshow()to properly protect the critical section.Additional Locations (1)
cleverpush/src/main/java/com/cleverpush/banner/AppBannerModule.java#L1859-L1861