Skip to content

ADFA-2841: show debugger only in editor activity and child app#1043

Merged
itsaky-adfa merged 4 commits intostagefrom
fix/ADFA-2841
Mar 6, 2026
Merged

ADFA-2841: show debugger only in editor activity and child app#1043
itsaky-adfa merged 4 commits intostagefrom
fix/ADFA-2841

Conversation

@itsaky-adfa
Copy link
Contributor

@itsaky-adfa itsaky-adfa commented Mar 5, 2026

See ADFA-2841 for more details.

Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
@itsaky-adfa itsaky-adfa requested a review from a team March 5, 2026 14:35
@itsaky-adfa itsaky-adfa self-assigned this Mar 5, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Release Notes

Overview: Implemented activity-aware debugger toolbar visibility control to show the debugger only when the editor activity is in the foreground.

Changes

  • Activity Lifecycle Tracking: Migrated from manual activity tracking in BaseEditorActivity to application-level lifecycle callback monitoring via IDEApplication
  • Foreground Activity State: Introduced StateFlow-based foreground activity tracking in IDEApplication with public properties:
    • foregroundActivityState: StateFlow<Activity?>
    • foregroundActivity: Activity? convenience getter
  • Debugger Visibility Logic: Updated DebuggerService to require both:
    • The IDE app is in foreground
    • The editor activity specifically is in foreground (not just any IDE activity)
  • Activity Lifecycle Delegate: Added new ActivityLifecycleCallbacksDelegate class to manage lifecycle callbacks

Risks & Best Practices Violations

  • Unused Base Class: ActivityLifecycleCallbacksDelegate is created with only no-op implementations and delegated immediately via class signature. Consider if this indirection is necessary or if lifecycle callbacks should be implemented directly in IDEApplication.
  • Lifecycle Edge Cases: Reliance on onActivityPreResumed and onActivityPostPaused callbacks may not capture all activity state transitions (e.g., activities created but never resumed, or destroyed without pausing). Consider whether additional lifecycle states need to be tracked.
  • Removal of Manual Tracking: Removal of explicit activity tracking from BaseEditorActivity eliminates a direct control point. Ensure no other components depend on the removed IDEApplication.setCurrentActivity() pattern.
  • State Flow Initialization: Foreground activity starts as null and relies on lifecycle callbacks to populate it. Ensure graceful handling in consumers (like DebuggerService) when the activity state is null during early app startup.

Walkthrough

The 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

Cohort / File(s) Summary
Application Activity Tracking
app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt, app/src/main/java/com/itsaky/androidide/app/ActivityLifecycleCallbacksDelegate.kt
IDEApplication now implements Application.ActivityLifecycleCallbacks via delegate. Added _foregroundActivity MutableStateFlow backing with public foregroundActivityState and foregroundActivity getter. Lifecycle callbacks (onActivityPreResumed, onActivityPostPaused) updated to manage foreground activity state and logging. New ActivityLifecycleCallbacksDelegate class provides no-op implementations for all callback methods.
BaseEditorActivity Lifecycle Cleanup
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
Removed IDEApplication imports and all activity tracking calls (getCurrentActiveActivity registration/clearing in lifecycle methods). Minor layout parameter formatting adjustments.
Service Integration
app/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt, app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt
DebuggerService now combines ForegroundAppReceiver.foregroundAppState with IDEApplication.foregroundActivityState. onForegroundAppChanged extended to accept both ForegroundAppState and current Activity parameter, evaluating overlay visibility based on both package and activity state. CredentialProtectedApplicationLoader updated to use application.foregroundActivity instead of getCurrentActiveActivity().

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

  • Daniel-ADFA
  • dara-abijo-adfa
  • jatezzz

Poem

🐰 A rabbit hops through lifecycle flows,
StateFlow tracks where activity goes,
Delegates handle callbacks with care,
Foreground states float through the air,
Services listen, the debugger knows! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: showing debugger only in editor activity and child app, which aligns with the refactoring of activity lifecycle tracking and debugger visibility logic across multiple files.
Description check ✅ Passed The description references ADFA-2841 Jira ticket which contains the full context for the changes; while minimal, it directly relates to the changeset's objective of controlling debugger visibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ADFA-2841

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 ourForegroundActivity is never used since the function is only called from the combine flow 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 of isFinishing.

Currently, foregroundActivity is only cleared when the activity is finishing. If the user backgrounds the app (presses home) without finishing the activity, the Activity reference is retained in the MutableStateFlow. While the debugger overlay logic in DebuggerService handles this correctly (by also checking isCotg), holding a reference to a paused Activity could 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 since ForegroundAppReceiver will 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

📥 Commits

Reviewing files that changed from the base of the PR and between b6d0e37 and 7a4145c.

📒 Files selected for processing (5)
  • app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
  • app/src/main/java/com/itsaky/androidide/app/ActivityLifecycleCallbacksDelegate.kt
  • app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt
  • app/src/main/java/com/itsaky/androidide/app/IDEApplication.kt
  • app/src/main/java/com/itsaky/androidide/services/debug/DebuggerService.kt

Copy link
Contributor

@dara-abijo-adfa dara-abijo-adfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment

@itsaky-adfa itsaky-adfa merged commit efed047 into stage Mar 6, 2026
2 checks passed
@itsaky-adfa itsaky-adfa deleted the fix/ADFA-2841 branch March 6, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants