ADFA-3829 Clear StrictMode violations on create-init flow#1277
Conversation
Address 33 still-reproducible violations from the create-new-project log
(the other 16 from legacy noActivity/emptyActivity/basicActivity templates
were already removed by ADFA-3381).
Off-thread the heavy work:
- TemplateListFragment.reloadTemplates: wrap ITemplateProvider.getInstance
+ getTemplates in withContext(Dispatchers.IO); pre-warm
ITemplateWidgetViewProvider.getInstance so per-bind getInstance is a
cache hit.
- TemplateDetailsFragment.bindWithTemplate: invoke each parameter
beforeCreateView on Dispatchers.IO before constructing the adapter so
that getNewProjectName's File.exists loop runs off the UI thread.
- Parameter.beforeCreateView: now one-shot per instance (reset on
release) so the bind-time call is a no-op once pre-warmed.
- TemplateWidgetViewProviderImpl.createTextField: per-keystroke
ConstraintVerifier.verify (which stats the filesystem for
EXISTS / FILE / DIRECTORY constraints) is now debounced and dispatched
to IO via the view tree's lifecycleScope.
- TemplateWidgetsListAdapter: lift ITemplateWidgetViewProvider.getInstance
out of onBindViewHolder into the adapter's init.
Defer initialization:
- JavaDebugAdapter.vmm: by lazy { Bootstrap.virtualMachineManager() }
eliminates a 507ms NetworkViolation and 4 disk reads at field init.
Tag the JDWP listener socket:
- ListenerState.startListening: wrap with TrafficStats.setThreadStatsTag
/ clearThreadStatsTag to clear UntaggedSocketViolation.
Whitelist vendor-framework call (not app-owned):
- MIUI's NotificationManager.notify lazily inits EpFrameworkFactory via
File.exists; rule + test added.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughRelease Notes: StrictMode Violations Clearance (Create-Init Flow)Performance Improvements
Optimization Changes
Framework Compliance
Risks & Considerations
WalkthroughAdds async template initialization and debounced text validation, a MIUI-specific StrictMode DiskReadViolation whitelist and test, plus small debug and JDWP listener instrumentation changes. ChangesTemplate Async Initialization & Debounced Validation
Strict Mode MIUI Enterprise Whitelist
Performance & Instrumentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.kt`:
- Around line 256-260: The current matchAdjacentFrames invocation that looks for
classAndMethod("java.io.File","exists") followed by the three MIUI frames should
be narrowed to require the downstream NotificationManager call path as well;
update the matcher that builds this rule (the matchAdjacentFrames call) to also
require, in order, a classAndMethod("android.app.NotificationManager","notify")
or classAndMethod("android.app.NotificationManager","notifyAsUser") frame after
(or before, as appropriate to call order) the MIUI EpFrameworkFactory frames so
the rule only triggers when the MIUI chain is part of a notification delivery
path.
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 173-183: The coroutine launched in
viewLifecycleOwner.lifecycleScope.launch can overlap older runs and assign the
wrong adapter; add a Job field (e.g., widgetsBindJob) on the fragment, cancel
widgetsBindJob?.cancel() before starting a new
viewLifecycleOwner.lifecycleScope.launch, store the returned Job in
widgetsBindJob, do the Dispatchers.IO work (template.widgets.forEach /
ParameterWidget.beforeCreateView) inside that coroutine, and only after the job
is the active one assign binding.widgets.adapter =
TemplateWidgetsListAdapter(template.widgets) (also keep the existing _binding
null-check) so stale/cancelled jobs cannot overwrite the adapter.
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 125-166: The reloadTemplates coroutine can run multiple concurrent
jobs and older ones may overwrite newer UI state; add a cancellable Job property
(e.g., private var reloadJob: Job? = null) on TemplateListFragment, cancel any
existing reloadJob before starting a new
viewLifecycleOwner.lifecycleScope.launch in reloadTemplates, then assign the
launched Job to reloadJob so only the latest job updates
adapter/binding/warnings; keep the existing _binding check and UI update logic
unchanged.
In
`@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kt`:
- Around line 48-53: Preserve and restore any existing thread stats tag around
the JDWP listener setup: before calling
TrafficStats.setThreadStatsTag(JDWP_LISTENER_SOCKET_TAG) read and save the
current tag with TrafficStats.getThreadStatsTag(), then call
TrafficStats.setThreadStatsTag(...), run connector.startListening(args) and in
the finally block restore the saved tag by calling
TrafficStats.setThreadStatsTag(savedTag) instead of
TrafficStats.clearThreadStatsTag(); reference the
TrafficStats.getThreadStatsTag, TrafficStats.setThreadStatsTag,
TrafficStats.clearThreadStatsTag calls and the connector.startListening(args)
invocation in ListenerState.kt to implement this change.
In `@templates-api/src/main/java/com/itsaky/androidide/templates/parameters.kt`:
- Around line 189-203: The one-shot guard in beforeCreateView currently does a
non-atomic check/set on beforeCreateViewInvoked allowing races; make the
check-and-set atomic (e.g., replace the boolean with an AtomicBoolean and use
compareAndSet, or protect the check/set and invocation with a synchronized
block) so that beforeCreateViewInvoked is set and actionBeforeCreateView is
invoked exactly once across threads; update the beforeCreateView method to
atomically test-and-set before invoking actionBeforeCreateView(this).
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt`:
- Around line 179-190: The validation branch toggles root.isErrorEnabled and
only sets root.error when err != null, leaving stale error text when validation
passes; in both the lifecycle-owner branch and the fallback
(ConstraintVerifier.verify) branch inside TemplateWidgetViewProviderImpl
(references: root, root.isErrorEnabled, root.error,
ConstraintVerifier.verify(value, constraints = param.constraints), value,
param.constraints) ensure that when err == null you explicitly clear the error
by setting root.error = null and set root.isErrorEnabled = false so no stale
error text remains after successful validation.
🪄 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: c0c09bb1-f570-4b8d-9de5-78aa7da5a6c0
📒 Files selected for processing (9)
app/src/androidTest/kotlin/com/itsaky/androidide/app/strictmode/WhitelistRulesTest.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateWidgetsListAdapter.ktapp/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kttemplates-api/src/main/java/com/itsaky/androidide/templates/parameters.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt
- WhitelistEngine: narrow MIUI EpFrameworkFactory rule to require the NotificationManager.notifyAsUser frame so it only matches the notification delivery path (and the intermediate ApplicationHelperStub frames), not arbitrary EpFrameworkFactory uses. - TemplateListFragment / TemplateDetailsFragment: track the launched coroutine in a Job field and cancel before launching a new one so a stale reload/bind cannot overwrite newer UI state. - ListenerState.startListening: save the previous thread stats tag with TrafficStats.getThreadStatsTag and restore it in finally instead of calling clearThreadStatsTag, so callers that already had a tag set keep theirs. - Parameter.beforeCreateView: replace the non-atomic boolean guard with AtomicBoolean.compareAndSet so the action runs exactly once across threads. - TemplateWidgetViewProviderImpl: factor validation result handling into applyValidationResult that explicitly clears root.error and isErrorEnabled when validation passes, so stale error text doesn't linger after the user fixes the input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kt`:
- Around line 18-21: The inline comment for the JDWP_LISTENER_SOCKET_TAG
constant in ListenerState.kt is incorrect: the hex 0x444A574C decodes to "DJWL",
not "JDWL". Fix by either updating the comment to "DJWL" to match the existing
value or, if the intent was the tag "JDWL", change the constant value to
0x4A44574C and update the comment accordingly; the symbol to edit is
JDWP_LISTENER_SOCKET_TAG in the ListenerState companion object.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt`:
- Around line 161-191: The initial setText triggers the text watcher registered
in configureTextFieldCommon which calls ConstraintVerifier.verify synchronously
when root.findViewTreeLifecycleOwner() is null; change the fallback so it does
NOT call ConstraintVerifier.verify on the UI thread: in param.configureTextField
(the block that creates/assigns validationJob and calls applyValidationResult)
replace the elvis fallback branch with a non-blocking behavior — either skip
validation (call applyValidationResult with no error or skip calling it) or
defer validation until a lifecycle owner is available by adding a listener
(e.g., observe root.viewTreeLifecycleOwnerLiveData or
addOnAttachStateChangeListener) and launching the same debounce+IO validation
once attached; keep the existing validationJob/cancel semantics so the initial
setText never performs disk IO on the main thread.
🪄 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: feadb7c7-bcc5-4c86-ae08-23eb4157d4c6
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/ListenerState.kttemplates-api/src/main/java/com/itsaky/androidide/templates/parameters.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateWidgetViewProviderImpl.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/itsaky/androidide/app/strictmode/WhitelistEngine.kt
- templates-api/src/main/java/com/itsaky/androidide/templates/parameters.kt
- ListenerState: fix JDWP_LISTENER_SOCKET_TAG. The previous value 0x444A574C decoded to "DJWL", not the "JDWL" the comment claimed. Use 0x4A44574C so the bytes match the mnemonic. - TemplateWidgetViewProviderImpl: when the view has no lifecycle owner (the initial setText fires before the RecyclerView attaches the inflated row), skip ConstraintVerifier.verify instead of running it synchronously on the UI thread. Validation re-runs on the user's first post-attach keystroke and on submit, so the form is still validated before acceptance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address 33 still-reproducible violations from the create-new-project log (the other 16 from legacy noActivity/emptyActivity/basicActivity templates were already removed by ADFA-3381).
Off-thread the heavy work:
Defer initialization:
Tag the JDWP listener socket:
Whitelist vendor-framework call (not app-owned):