Invert condition for lifecycle method calls#6401
Conversation
Shouldn't mLifecycleHelper be called manually on versions OLDER than Q?
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe condition logic for lifecycle helper method invocations in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/com/android/launcher3/BaseActivity.java`:
- Around line 310-312: The lifecycle conditional using Utilities.ATLEAST_Q is
correct but verify and ensure each lifecycle method (onCreate, onStart,
onResume, onPause, onStop, onDestroy, onSaveInstanceState) in BaseActivity calls
mLifecycleHelper.<correspondingMethod>(this) only when Utilities.ATLEAST_Q is
false; confirm the helper is registered via registerActivityLifecycleCallbacks()
in BaseActivity and that LifecycleHelper (ActivityLifecycleCallbacksAdapter)
implements all lifecycle handlers so no duplicate calls occur on Q+ devices.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 937be04f-5d1c-4772-8ff5-860668d69667
📒 Files selected for processing (1)
src/com/android/launcher3/BaseActivity.java
| if (!Utilities.ATLEAST_Q) { | ||
| mLifecycleHelper.onActivityStopped(this); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify LifecycleHelper implementation and ensure it handles all lifecycle callbacks
# Find and examine the LifecycleHelper class
fd -t f 'LifecycleHelper.java' --exec cat {}
# Check if there are any other usages of mLifecycleHelper that might need similar treatment
rg -n 'mLifecycleHelper\.' --type javaRepository: LawnchairLauncher/lawnchair
Length of output: 825
🏁 Script executed:
# Search for LifecycleHelper class definition more broadly
rg -n 'class LifecycleHelper' --type java -A 5Repository: LawnchairLauncher/lawnchair
Length of output: 53
🏁 Script executed:
# Also search for LifecycleHelper in import statements and other files
rg -n 'LifecycleHelper' --type java | head -20Repository: LawnchairLauncher/lawnchair
Length of output: 1290
🏁 Script executed:
# Read the BaseActivity.java file around the lifecycle methods to see the full context
wc -l src/com/android/launcher3/BaseActivity.javaRepository: LawnchairLauncher/lawnchair
Length of output: 118
🏁 Script executed:
# Locate the actual LifecycleHelper.java file
fd -t f 'LifecycleHelper.java'Repository: LawnchairLauncher/lawnchair
Length of output: 53
🏁 Script executed:
# Read the relevant sections of BaseActivity.java to verify the conditional pattern
sed -n '260,290p' src/com/android/launcher3/BaseActivity.java
sed -n '305,315p' src/com/android/launcher3/BaseActivity.java
sed -n '320,330p' src/com/android/launcher3/BaseActivity.java
sed -n '330,340p' src/com/android/launcher3/BaseActivity.java
sed -n '345,355p' src/com/android/launcher3/BaseActivity.javaRepository: LawnchairLauncher/lawnchair
Length of output: 2336
🏁 Script executed:
# Read the LifecycleHelper class definition
fd -t f -p '*util*LifecycleHelper*' --exec cat {}Repository: LawnchairLauncher/lawnchair
Length of output: 372
🏁 Script executed:
# Find LifecycleHelper class file
fd -i lifecyclehelper -t fRepository: LawnchairLauncher/lawnchair
Length of output: 120
🏁 Script executed:
# Search for the LifecycleHelper class definition with its interface implementation
rg -n 'class LifecycleHelper' --type java -A 10Repository: LawnchairLauncher/lawnchair
Length of output: 53
🏁 Script executed:
# Read the LifecycleHelper Kotlin file to verify it implements Application.ActivityLifecycleCallbacks
cat src/com/android/launcher3/util/LifecycleHelper.ktRepository: LawnchairLauncher/lawnchair
Length of output: 2813
LGTM: Consistent pattern across all lifecycle methods.
The conditional pattern if (!Utilities.ATLEAST_Q) is correctly applied to all seven lifecycle methods: onCreate, onStart, onResume, onStop, onDestroy, onPause, and onSaveInstanceState. This ensures manual helper invocation on pre-Q devices and automatic dispatch via registered callback on Q+.
The LifecycleHelper class (in src/com/android/launcher3/util/LifecycleHelper.kt) implements ActivityLifecycleCallbacksAdapter and properly handles all lifecycle events, with the helper being registered via registerActivityLifecycleCallbacks() in BaseActivity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/com/android/launcher3/BaseActivity.java` around lines 310 - 312, The
lifecycle conditional using Utilities.ATLEAST_Q is correct but verify and ensure
each lifecycle method (onCreate, onStart, onResume, onPause, onStop, onDestroy,
onSaveInstanceState) in BaseActivity calls
mLifecycleHelper.<correspondingMethod>(this) only when Utilities.ATLEAST_Q is
false; confirm the helper is registered via registerActivityLifecycleCallbacks()
in BaseActivity and that LifecycleHelper (ActivityLifecycleCallbacksAdapter)
implements all lifecycle handlers so no duplicate calls occur on Q+ devices.
Shouldn't mLifecycleHelper be called manually on versions OLDER than Q?
Description
Fixes #(issue)
Reasoning
Testing
Type of change
✅ Bug fix (A non-breaking change that fixes an issue)
❌ New feature (A non-breaking change that adds functionality)
❌ Breaking change (A fix or feature that would cause existing functionality to not work as expected)
❌ Refactor (A code change that neither fixes a bug nor adds a feature)
❌ Performance (A code change that improves performance)
❌ Style (Code style changes)
❌ Docs (Changes to documentation)
❌ Chore (Changes to the build process or other tooling)
Summary by CodeRabbit