ADFA-2841: show debugger only in editor activity and child app#1043
ADFA-2841: show debugger only in editor activity and child app#1043itsaky-adfa merged 4 commits intostagefrom
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
📝 WalkthroughRelease NotesOverview: Implemented activity-aware debugger toolbar visibility control to show the debugger only when the editor activity is in the foreground. Changes
Risks & Best Practices Violations
WalkthroughThe changes refactor the application's activity tracking mechanism, replacing direct IDEApplication-based activity registration with a state-flow-driven approach. A new ActivityLifecycleCallbacksDelegate class is introduced to handle lifecycle callbacks, IDEApplication now exposes foreground activity state through StateFlow, and consuming services (DebuggerService, CredentialProtectedApplicationLoader) are updated to leverage this new state-based API. Changes
Sequence DiagramsequenceDiagram
participant AC as Activity Lifecycle
participant APP as IDEApplication
participant SF as StateFlow
participant DS as DebuggerService
participant FAR as ForegroundAppReceiver
AC->>APP: onActivityPreResumed(activity)
APP->>SF: _foregroundActivity.value = activity
APP->>APP: Log activity
SF-->>DS: foregroundActivityState updates
FAR-->>DS: foregroundAppState updates
DS->>DS: Combine both states
DS->>DS: Derive isCotg & isEditorActivityInForeground
DS->>DS: Update overlay visibility
AC->>APP: onActivityPostPaused(activity)
APP->>SF: _foregroundActivity.value = null
APP->>APP: Log activity finished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt (1)
79-82: Consider removing the unused default parameter.The default value for
ourForegroundActivityis never used since the function is only called from thecombineflow on line 73 where the value is always passed explicitly. Default parameters that aren't used can be misleading to future readers.♻️ Suggested simplification
private fun onForegroundAppChanged( foregroundAppState: ForegroundAppState, - ourForegroundActivity: Activity? = IDEApplication.instance.foregroundActivity, + ourForegroundActivity: Activity?, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt` around lines 79 - 82, The function onForegroundAppChanged has an unused default parameter ourForegroundActivity set to IDEApplication.instance.foregroundActivity; remove the default value so the signature becomes onForegroundAppChanged(foregroundAppState: ForegroundAppState, ourForegroundActivity: Activity?) and update any call sites (e.g., the combine flow that currently passes the activity explicitly) to continue passing the second argument unchanged—ensure references to the function name onForegroundAppChanged and the parameter ourForegroundActivity are updated accordingly.app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt (1)
135-147: Consider clearing foreground activity on pause regardless ofisFinishing.Currently,
foregroundActivityis only cleared when the activity is finishing. If the user backgrounds the app (presses home) without finishing the activity, theActivityreference is retained in theMutableStateFlow. While the debugger overlay logic inDebuggerServicehandles this correctly (by also checkingisCotg), holding a reference to a pausedActivitycould prevent garbage collection until another activity resumes.If the intent is to track only truly visible activities, consider clearing on any pause and setting on resume:
💡 Optional refactor
override fun onActivityPostPaused(activity: Activity) { super.onActivityPostPaused(activity) - if (foregroundActivity == activity && activity.isFinishing) { + if (foregroundActivity == activity) { logger.debug("foregroundActivity = null") _foregroundActivity.update { null } } }However, if the current behavior is intentional for the combined stream logic in
DebuggerService, the existing implementation is acceptable sinceForegroundAppReceiverwill correctly detect when the app is no longer in the foreground.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt` around lines 135 - 147, The onActivityPostPaused currently only clears the foregroundActivity when activity.isFinishing, which can retain a paused Activity reference and prevent GC; change onActivityPostPaused (the method that calls _foregroundActivity.update) to clear the foregroundActivity unconditionally when any activity is paused, and keep onActivityPreResumed to set the activity in _foregroundActivity.update as before so the flow represents truly visible/resumed activities (reference onActivityPostPaused, onActivityPreResumed, _foregroundActivity.update, and foregroundActivity).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt`:
- Around line 135-147: The onActivityPostPaused currently only clears the
foregroundActivity when activity.isFinishing, which can retain a paused Activity
reference and prevent GC; change onActivityPostPaused (the method that calls
_foregroundActivity.update) to clear the foregroundActivity unconditionally when
any activity is paused, and keep onActivityPreResumed to set the activity in
_foregroundActivity.update as before so the flow represents truly
visible/resumed activities (reference onActivityPostPaused,
onActivityPreResumed, _foregroundActivity.update, and foregroundActivity).
In `@app/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt`:
- Around line 79-82: The function onForegroundAppChanged has an unused default
parameter ourForegroundActivity set to
IDEApplication.instance.foregroundActivity; remove the default value so the
signature becomes onForegroundAppChanged(foregroundAppState: ForegroundAppState,
ourForegroundActivity: Activity?) and update any call sites (e.g., the combine
flow that currently passes the activity explicitly) to continue passing the
second argument unchanged—ensure references to the function name
onForegroundAppChanged and the parameter ourForegroundActivity are updated
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba918dad-2acb-40b2-8443-cd154c469662
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/app/ActivityLifecycleCallbacksDelegate.ktapp/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/app/IDEApplication.ktapp/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt
See ADFA-2841 for more details.