fix(Android): app shows notifications in foreground state#7012
Draft
fix(Android): app shows notifications in foreground state#7012
Conversation
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (2)
249-257:⚠️ Potential issue | 🟠 MajorAvoid queueing notifications that are intentionally suppressed in foreground.
Lines 253-257 still append to
notificationMessageseven when Line 294 later suppresses display. This can retain stale data and inflate grouped notifications once app goes background.Suggested fix
- notificationMessages.get(notId).add(bundle); - if (ENABLE_VERBOSE_LOGS) { - Log.d(TAG, "[After add] notificationMessages[" + notId + "].size=" + notificationMessages.get(notId).size()); - } - postNotification(Integer.parseInt(notId)); + if (isAppInForeground()) { + if (ENABLE_VERBOSE_LOGS) { + Log.d(TAG, "App in foreground, skipping native queue/display for notId=" + notId); + } + return; + } + notificationMessages.get(notId).add(bundle); + if (ENABLE_VERBOSE_LOGS) { + Log.d(TAG, "[After add] notificationMessages[" + notId + "].size=" + notificationMessages.get(notId).size()); + } + postNotification(Integer.parseInt(notId));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java` around lines 249 - 257, The code is adding bundle entries to notificationMessages (keyed by notId) even for notifications that are later suppressed in foreground; update the block around notificationMessages.get(notId).add(bundle) to first check the suppression flag on the bundle (e.g., bundle.getBoolean("notificationLoaded", false) or the foreground-suppression flag used later) and only add and call postNotification(Integer.parseInt(notId)) when the bundle is not suppressed; keep the verbose Log.d(TAG, ...) calls but move them inside the guarded branch so suppressed notifications are neither queued nor cause postNotification to run.
51-91:⚠️ Potential issue | 🔴 CriticalMake
isAppInForegroundthread-safe.The field at line 51 is written from MainActivity's lifecycle callbacks (main thread) and read during Firebase message processing (background thread). Without synchronization, memory visibility is not guaranteed across threads.
Suggested fix
- private static boolean isAppInForeground = false; + private static volatile boolean isAppInForeground = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java` around lines 51 - 91, The static field isAppInForeground is accessed from multiple threads without synchronization; make it thread-safe by replacing the plain boolean with a thread-safe construct (e.g., java.util.concurrent.atomic.AtomicBoolean) and update all usages: initialize a private static final AtomicBoolean isAppInForeground = new AtomicBoolean(false), change setAppInForeground(...) to call isAppInForeground.set(inForeground) and change isAppInForeground() to return isAppInForeground.get(); ensure any other direct reads/writes to the old boolean are updated to use the AtomicBoolean methods (references: isAppInForeground field, setAppInForeground method, isAppInForeground() method).
🧹 Nitpick comments (1)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
38-48: Consider process-level lifecycle tracking for more robust foreground state management.Activity-level
onResume/onPausecan briefly mark the app as backgrounded during transitions to other activities (e.g., ShareActivity). This could allow native notifications to display during activity transitions. UseProcessLifecycleOwnerfrom androidx.lifecycle for process-level tracking instead—this survives activity transitions and only changes when the entire app moves to background.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt` around lines 38 - 48, The current foreground/background tracking in MainActivity via onResume/onPause (which calls CustomPushNotification.setAppInForeground) can misfire during activity-to-activity transitions; replace or supplement this with a process-level lifecycle observer using androidx.lifecycle.ProcessLifecycleOwner: create a LifecycleObserver (or implement DefaultLifecycleObserver) that calls CustomPushNotification.setAppInForeground(true/false) on onStart/onStop (process-level), register it in the Application class (e.g., in onCreate) so state changes reflect the whole app lifecycle rather than MainActivity transitions, and remove or keep MainActivity.onResume/onPause only if you need activity-specific behaviour.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java`:
- Around line 249-257: The code is adding bundle entries to notificationMessages
(keyed by notId) even for notifications that are later suppressed in foreground;
update the block around notificationMessages.get(notId).add(bundle) to first
check the suppression flag on the bundle (e.g.,
bundle.getBoolean("notificationLoaded", false) or the foreground-suppression
flag used later) and only add and call postNotification(Integer.parseInt(notId))
when the bundle is not suppressed; keep the verbose Log.d(TAG, ...) calls but
move them inside the guarded branch so suppressed notifications are neither
queued nor cause postNotification to run.
- Around line 51-91: The static field isAppInForeground is accessed from
multiple threads without synchronization; make it thread-safe by replacing the
plain boolean with a thread-safe construct (e.g.,
java.util.concurrent.atomic.AtomicBoolean) and update all usages: initialize a
private static final AtomicBoolean isAppInForeground = new AtomicBoolean(false),
change setAppInForeground(...) to call isAppInForeground.set(inForeground) and
change isAppInForeground() to return isAppInForeground.get(); ensure any other
direct reads/writes to the old boolean are updated to use the AtomicBoolean
methods (references: isAppInForeground field, setAppInForeground method,
isAppInForeground() method).
---
Nitpick comments:
In `@android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt`:
- Around line 38-48: The current foreground/background tracking in MainActivity
via onResume/onPause (which calls CustomPushNotification.setAppInForeground) can
misfire during activity-to-activity transitions; replace or supplement this with
a process-level lifecycle observer using
androidx.lifecycle.ProcessLifecycleOwner: create a LifecycleObserver (or
implement DefaultLifecycleObserver) that calls
CustomPushNotification.setAppInForeground(true/false) on onStart/onStop
(process-level), register it in the Application class (e.g., in onCreate) so
state changes reflect the whole app lifecycle rather than MainActivity
transitions, and remove or keep MainActivity.onResume/onPause only if you need
activity-specific behaviour.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-05T13:55:06.688Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
Applied to files:
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
🧬 Code graph analysis (1)
android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (1)
app/reducers/app.ts (1)
app(29-77)
Rohit3523
requested a deployment
to
upload_experimental_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat Experimental 4.70.0.108316 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTIczwBTkj4_LK_6nz0RjLp21gpXPIAP_oq_5GB1G4_7EBNxXXtqSU3uqXL7ZdSZmgwHLtgz1S1yz0zm52M |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
When we are using the app and someone sends a message in any channel, the app shows a blank notification without content. This PR adds a check at the native level to prevent triggering notifications when the app is in the foreground
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1856
Closes: #6999
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit