refactor: convert PackageLister from Java to Kotlin#27
Conversation
Makes the codebase fully Kotlin (was 8 Kotlin files + 1 Java). The @file:JvmName("PackageLister") annotation preserves the generated class name, so the app_process invocation is unchanged: app_process / com.mobilenext.devicekit.PackageLister Behavior is identical: reflection into ServiceManager/IPackageManager, same JSON output. Verified the dex still exposes Lcom/mobilenext/devicekit/PackageLister; with main([Ljava/lang/String;)V.
WalkthroughThe PR introduces a new 🚥 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1 (1)
40-41: ⚡ Quick winUse a primitive parameter token for reflective
getDisplayInfo(int)lookup.On Line 40, use
Int::class.javaPrimitiveTypefor deterministic method resolution ofgetDisplayInfo(int). UsingInt::class.javahere is ambiguous and can fail lookup depending on JVM mapping assumptions, forcing fallback defaults.Suggested patch
- val getDisplayInfoMethod = displayManager.javaClass.getMethod("getDisplayInfo", Int::class.java) + val getDisplayInfoMethod = displayManager.javaClass.getMethod( + "getDisplayInfo", + Int::class.javaPrimitiveType + )🤖 Prompt for 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. In `@app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1` around lines 40 - 41, The reflective method lookup for getDisplayInfo on line 40 uses Int::class.java as the parameter type, which is ambiguous and can fail depending on JVM mapping assumptions. Replace Int::class.java with Int::class.javaPrimitiveType in the getMethod call to ensure deterministic resolution of the primitive int parameter type and avoid fallback defaults.
🤖 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/mobilenext/devicekit/DisplayUtils.kt1`:
- Around line 92-110: The issue is that the reflection code is attempting to
invoke createVirtualDisplay on the hidden IDisplayManager interface, which has
an incompatible signature (returns int instead of VirtualDisplay) and will cause
a casting exception at line 109 that propagates to process exit. Replace the
reflection-based approach (lines 92-109) with a direct call to the public
DisplayManager.createVirtualDisplay method, which is the proper public API that
returns a VirtualDisplay object and handles API compatibility correctly. This
should be done by using the displayManager object directly (obtained from
context) rather than reflecting on its hidden interface layer.
In `@app/src/main/java/com/mobilenext/devicekit/PackageLister.kt`:
- Around line 61-70: The AssetManager instance created on line 61 via
getDeclaredConstructor().newInstance() is never closed, causing native resource
leaks within the loop. Add a finally block after the try-catch statement that
closes the assets object, or refactor the code to use Kotlin's use extension
function to ensure the AssetManager is properly closed after the label lookup is
complete. The AssetManager should be closed immediately after the Resources
object finishes retrieving the getString call, whether the operation succeeds or
fails.
---
Nitpick comments:
In `@app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1`:
- Around line 40-41: The reflective method lookup for getDisplayInfo on line 40
uses Int::class.java as the parameter type, which is ambiguous and can fail
depending on JVM mapping assumptions. Replace Int::class.java with
Int::class.javaPrimitiveType in the getMethod call to ensure deterministic
resolution of the primitive int parameter type and avoid fallback defaults.
🪄 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: 607d6e6d-3adc-4170-8a4c-5ec2d52e9185
📒 Files selected for processing (3)
app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1app/src/main/java/com/mobilenext/devicekit/PackageLister.javaapp/src/main/java/com/mobilenext/devicekit/PackageLister.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/mobilenext/devicekit/PackageLister.java
| val createVirtualDisplayMethod = displayManager.javaClass | ||
| .getMethod( | ||
| "createVirtualDisplay", | ||
| String::class.java, | ||
| Int::class.javaPrimitiveType, | ||
| Int::class.javaPrimitiveType, | ||
| Int::class.javaPrimitiveType, | ||
| Surface::class.java | ||
| ) | ||
|
|
||
| createVirtualDisplayMethod.invoke( | ||
| displayManager, | ||
| name, | ||
| width, | ||
| height, | ||
| dpi, | ||
| surface | ||
| ) as VirtualDisplay | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Android AOSP (API 29 through latest), what are the exact method signatures and return types for android.hardware.display.IDisplayManager.createVirtualDisplay, and does any overload return android.hardware.display.VirtualDisplay directly?
💡 Result:
The android.hardware.display.IDisplayManager interface, which is defined in AOSP via AIDL, does not return a VirtualDisplay object. Instead, its createVirtualDisplay method returns an int [1][2]. The method signature for createVirtualDisplay within IDisplayManager.aidl has evolved across AOSP versions but consistently returns an integer representing a display ID or status, or is used to facilitate the creation process [1][3][4][2]. In the current AOSP master branch, the signature is defined as: int createVirtualDisplay(in VirtualDisplayConfig virtualDisplayConfig, in IVirtualDisplayCallback callback, in IMediaProjection projectionToken, String packageName); [1][2] The android.hardware.display.VirtualDisplay object itself is managed at a higher abstraction layer by the android.hardware.display.DisplayManager class in the Android framework, not by the low-level IDisplayManager interface [5][6][7]. Developers interact with DisplayManager, which provides public methods that return a VirtualDisplay instance, such as: public VirtualDisplay createVirtualDisplay(String name, int width, int height, int densityDpi, Surface surface, int flags) [6][8] No overload of the IDisplayManager.createVirtualDisplay method returns an android.hardware.display.VirtualDisplay directly [1][3][2]. The IDisplayManager interface serves as the binder interface to the system service, while the creation of the high-level VirtualDisplay wrapper is handled within the framework's DisplayManager implementation [5][7].
Citations:
- 1: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/display/IDisplayManager.aidl
- 2: https://android.googlesource.com/platform/frameworks/base/+/37169d9adb45f3e88828c06ec86c6f322b90992a/core/java/android/hardware/display/IDisplayManager.aidl
- 3: https://android.googlesource.com/platform/frameworks/base/+/feef9887e8f8eb6f64fc1b4552c02efb5755cdc1/core/java/android/hardware/display/IDisplayManager.aidl
- 4: https://android.googlesource.com/platform/frameworks/base/+/19e8d4659c4e717d4c0a256e1884c86b67a2a211/core/java/android/hardware/display/IDisplayManager.aidl
- 5: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/display/DisplayManager.java
- 6: https://developer.android.com/reference/android/hardware/display/DisplayManager
- 7: https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/hardware/display/VirtualDisplay.java
- 8: https://developer.android.com/reference/kotlin/android/hardware/display/DisplayManager
🏁 Script executed:
cat -n app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1 | sed -n '70,120p'Repository: mobile-next/devicekit-android
Length of output: 2184
createVirtualDisplay is reflecting on the wrong API layer with incompatible signature and return type.
The code at lines 83–89 obtains an IDisplayManager interface proxy, then attempts to invoke createVirtualDisplay(String, int, int, int, Surface) on it (lines 92–109). According to AOSP, IDisplayManager.createVirtualDisplay returns an int (display ID), not VirtualDisplay, and the method signature differs significantly across API levels. The cast to VirtualDisplay (line 109) will fail, the exception is caught (lines 111–114), and null is returned—triggering immediate process exit in both MjpegServer and AvcServer.
To fix this, call DisplayManager.createVirtualDisplay directly via the public framework API instead of reflecting on the hidden IDisplayManager interface.
🤖 Prompt for 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.
In `@app/src/main/java/com/mobilenext/devicekit/DisplayUtils.kt1` around lines 92
- 110, The issue is that the reflection code is attempting to invoke
createVirtualDisplay on the hidden IDisplayManager interface, which has an
incompatible signature (returns int instead of VirtualDisplay) and will cause a
casting exception at line 109 that propagates to process exit. Replace the
reflection-based approach (lines 92-109) with a direct call to the public
DisplayManager.createVirtualDisplay method, which is the proper public API that
returns a VirtualDisplay object and handles API compatibility correctly. This
should be done by using the displayManager object directly (obtained from
context) rather than reflecting on its hidden interface layer.
| val assets = AssetManager::class.java.getDeclaredConstructor().newInstance() | ||
| val addAssetPath = AssetManager::class.java.getMethod("addAssetPath", String::class.java) | ||
| addAssetPath.invoke(assets, appInfo.sourceDir) | ||
| val res = Resources(assets, DisplayMetrics(), Configuration()) | ||
| val label = res.getString(appInfo.labelRes) | ||
| if (label.isNotEmpty()) { | ||
| displayName = label | ||
| } | ||
| } catch (ignored: Exception) { | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify allocation site and absence/presence of explicit close in this file.
rg -n -C2 'AssetManager::class\.java|getDeclaredConstructor\(\)\.newInstance\(\)|assets\.close\(' app/src/main/java/com/mobilenext/devicekit/PackageLister.ktRepository: mobile-next/devicekit-android
Length of output: 571
Close AssetManager after each package label lookup.
On line 61, a new AssetManager is created per package but never closed. In this loop, that leaks native resources and can destabilize the process.
Suggested fix
- val assets = AssetManager::class.java.getDeclaredConstructor().newInstance()
- val addAssetPath = AssetManager::class.java.getMethod("addAssetPath", String::class.java)
- addAssetPath.invoke(assets, appInfo.sourceDir)
- val res = Resources(assets, DisplayMetrics(), Configuration())
- val label = res.getString(appInfo.labelRes)
- if (label.isNotEmpty()) {
- displayName = label
- }
+ val assets = AssetManager::class.java.getDeclaredConstructor().newInstance()
+ try {
+ val addAssetPath = AssetManager::class.java.getMethod("addAssetPath", String::class.java)
+ addAssetPath.invoke(assets, appInfo.sourceDir)
+ val res = Resources(assets, DisplayMetrics(), Configuration())
+ val label = res.getString(appInfo.labelRes)
+ if (label.isNotEmpty()) {
+ displayName = label
+ }
+ } finally {
+ assets.close()
+ }🤖 Prompt for 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.
In `@app/src/main/java/com/mobilenext/devicekit/PackageLister.kt` around lines 61
- 70, The AssetManager instance created on line 61 via
getDeclaredConstructor().newInstance() is never closed, causing native resource
leaks within the loop. Add a finally block after the try-catch statement that
closes the assets object, or refactor the code to use Kotlin's use extension
function to ensure the AssetManager is properly closed after the label lookup is
complete. The AssetManager should be closed immediately after the Resources
object finishes retrieving the getString call, whether the operation succeeds or
fails.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/mobilenext/devicekit/PackageLister.kt">
<violation number="1" location="app/src/main/java/com/mobilenext/devicekit/PackageLister.kt:61">
P2: The `AssetManager` instance created here is never closed after use. Since this runs in a loop over all installed packages, each iteration leaks native resources. Add a `try/finally` block (or `.use {}`) to close the `AssetManager` after the label lookup.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| displayName = appInfo.nonLocalizedLabel.toString() | ||
| } else if (appInfo.labelRes != 0 && appInfo.sourceDir != null) { | ||
| try { | ||
| val assets = AssetManager::class.java.getDeclaredConstructor().newInstance() |
There was a problem hiding this comment.
P2: The AssetManager instance created here is never closed after use. Since this runs in a loop over all installed packages, each iteration leaks native resources. Add a try/finally block (or .use {}) to close the AssetManager after the label lookup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/mobilenext/devicekit/PackageLister.kt, line 61:
<comment>The `AssetManager` instance created here is never closed after use. Since this runs in a loop over all installed packages, each iteration leaks native resources. Add a `try/finally` block (or `.use {}`) to close the `AssetManager` after the label lookup.</comment>
<file context>
@@ -0,0 +1,87 @@
+ displayName = appInfo.nonLocalizedLabel.toString()
+ } else if (appInfo.labelRes != 0 && appInfo.sourceDir != null) {
+ try {
+ val assets = AssetManager::class.java.getDeclaredConstructor().newInstance()
+ val addAssetPath = AssetManager::class.java.getMethod("addAssetPath", String::class.java)
+ addAssetPath.invoke(assets, appInfo.sourceDir)
</file context>
1 participant
Summary
Converts the project's lone Java file,
PackageLister.java, to Kotlin so the codebase is fully Kotlin (was 8 Kotlin files + 1 Java).Key detail: the
app_processentry point is preservedPackageListeris a standaloneapp_processtool, not an app component:A top-level Kotlin
fun mainwould normally compile to a class namedPackageListerKt, which would change that invocation. To avoid that, the file uses:@file:JvmName("PackageLister")so the generated class stays
com.mobilenext.devicekit.PackageListerand the command line is unchanged.Behavior
Identical to the Java version — reflection into
ServiceManager/IPackageManager, same JSON output ({packageName, appName, version}).Verification
./gradlew assembleDebugsucceeds.Lcom/mobilenext/devicekit/PackageLister;withmain([Ljava/lang/String;)V.Test plan
app_process / com.mobilenext.devicekit.PackageListeron a device and confirm JSON output matches the previous build.Summary by cubic
Converted
PackageListerto Kotlin and preserved theapp_processentry point. Removed a strayDisplayUtils.kt1file. No behavior or JSON output changes.PackageLister.javaasPackageLister.ktand added@file:JvmName("PackageLister")to keepcom.mobilenext.devicekit.PackageLister.DisplayUtils.kt1.Written for commit bf19fcf. Summary will update on new commits.