Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughA new ChangesAndroid WebView Upgrade Plugin
Android Compilation Gates and Frontend UI Fixes
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note right of Android: App startup
participant Android as Android Runtime
participant BootstrapProvider as BootstrapProvider
participant Bootstrap as WebViewUpgradeBootstrap
participant Swap as WebViewSwap
end
rect rgba(255, 218, 185, 0.5)
note right of PMHook: Binder hook layer
participant PMHook as PackageManagerHook
participant WVUSHook as WebViewUpdateServiceHook
participant AMHook as ActivityManagerHook
end
rect rgba(144, 238, 144, 0.5)
note right of SandboxDelegate: Sandbox layer
participant SandboxDelegate as SandboxedProcessServiceDelegate
participant StubService as StubSandboxedProcessServiceN
end
Android->>BootstrapProvider: onCreate() — high initOrder
BootstrapProvider->>Bootstrap: run(context)
Bootstrap->>Bootstrap: currentWebView() — detect major version
Bootstrap->>Swap: swapToInstalledPackage(context, packageName)
Swap->>PMHook: hook() — intercept getPackageInfo/getApplicationInfo
Swap->>WVUSHook: hook() — rewrite waitForAndGetProvider response
Swap->>AMHook: hook() — intercept bindIsolatedService → redirect to StubService
Swap->>Android: new WebView() — lock provider binding
Android-->>Swap: getCurrentWebViewPackage() → replaceWebViewPackageInfo
Swap-->>Bootstrap: swapped PackageInfo or exception
Bootstrap->>Bootstrap: schedule OutdatedWebViewNotice on failure
AMHook-->>StubService: redirected sandbox bind intent
StubService->>SandboxDelegate: onCreate / onBind
SandboxDelegate->>SandboxDelegate: injectDex + installChromiumClassLoader
SandboxDelegate-->>StubService: real service IBinder
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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
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 |
6 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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 `@src-tauri/risuko-webview-upgrade/android/build.gradle.kts`:
- Around line 48-53: The parsePluginBlock function and similar configuration
parsing code (referenced at lines 92-99) silently default to fallback values
when configuration is malformed or invalid, which can hide configuration errors.
Modify parsePluginBlock to validate that both parsed values are greater than 0
and that minSupportedMajor is less than or equal to minUpgradeMajor, throwing a
clear build error with a descriptive message when validation fails instead of
using the elvis operator to default. Apply the same validation logic to the
other configuration parsing code mentioned in the comment at lines 92-99.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/Common.kt`:
- Around line 35-42: The CANDIDATE_PACKAGES list in Common.kt is missing the
com.huawei.webview package that is declared in the AndroidManifest.xml queries
section. This causes the Huawei webview provider to be queried but never
evaluated as a viable swap or detection target. Add the string
"com.huawei.webview" to the CANDIDATE_PACKAGES list to ensure it is considered
alongside the other webview provider packages.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/OutdatedWebViewNotice.kt`:
- Around line 21-27: The schedule() method has a race condition in its
check-then-set pattern on the scheduled variable. Two concurrent threads could
both read scheduled as false before either writes true, allowing duplicate
execution. Add the `@Synchronized` annotation to the schedule() method to make the
entire method atomic and thread-safe, ensuring only one thread can execute the
check and set operations at a time.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxedProcessServiceDelegate.kt`:
- Around line 232-317: The injectDex method has high cyclomatic and cognitive
complexity due to handling multiple concerns: extracting dex elements,
extracting native elements, merging arrays, and determining component types.
Break this into smaller focused helper methods by extracting the following
responsibilities into separate private functions: create a method to extract dex
and native elements from a given PathClassLoader (used for both webview and
shared loaders), create a method to combine dex element arrays given their
individual sizes and the current path list, and create a method to combine
native element arrays with similar parameters. This refactoring will reduce the
main injectDex method's complexity while maintaining clarity through descriptive
method names and keeping each helper method to a single responsibility.
- Around line 27-30: The onUnbind method is not being forwarded to the real
service in the stub implementations, which prevents onRebind from being called
and breaks the unbind lifecycle. Add an onUnbind override method to each of the
stub service classes (StubSandboxedProcessService1,
StubSandboxedProcessService2, StubSandboxedProcessService3, and
StubSandboxedProcessService4) that uses the reflection pattern to call the
onUnbindMethod on the delegate service, ensuring the boolean return value is
preserved and returned. This matches the existing onRebind delegation pattern
already implemented and restores the proper unbind lifecycle chain.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService0.kt`:
- Around line 8-15: The StubSandboxedProcessService0 class is currently running
as a normal child process with app UID privileges instead of in an isolated
sandbox process. Configure StubSandboxedProcessService0 to run in a properly
isolated process by ensuring it is registered with appropriate process isolation
attributes in the Android manifest (such as android:isolatedProcess or a
dedicated sandbox process declaration) so that compromised renderer code runs
with a separate isolated UID rather than with the app's UID privileges.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BinderHook.kt`:
- Around line 9-12: The `proxyBinder` property is written under synchronization
in the `hook()` method but is read from arbitrary binder threads (e.g., in
`WebViewUpdateServiceHook` and `PackageManagerHook` `asBinder` method) without
acquiring the lock, creating a race condition where reading threads may observe
stale or null values. Mark the `proxyBinder` backing field with the `@Volatile`
annotation to ensure proper memory visibility across all threads accessing this
property without requiring synchronization.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewSwap.kt`:
- Around line 85-94: The `@SuppressLint` annotation with "SetJavaScriptEnabled" on
the lockProviderBinding method is unnecessary because the method does not call
setJavaScriptEnabled anywhere in its implementation. Remove this annotation from
the method signature to clean up the code and avoid misleading future developers
about potential lint warnings.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewUpdateServiceHook.kt`:
- Around line 43-58: The code in the try block assumes that the status field in
WebViewProviderResponse is always located at dataSize() - 4 bytes, which relies
on an undocumented internal Parcel format that Android does not guarantee will
remain stable across versions. Add a version check or fallback mechanism around
the parcel manipulation logic to handle potential format changes gracefully.
Consider wrapping the offset assumption
(parcel.setDataPosition(parcel.dataSize() - 4) and subsequent
parcel.writeInt(0)) with an explicit SDK-level guard or exception handling that
can detect and handle cases where the expected field offset may have changed in
future Android versions.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.kt`:
- Around line 81-89: The trySwap method accepts two parameters, fallbackMajor
and minSupported, but neither is referenced anywhere in the method body.
Determine whether these parameters should be used to implement version checking
or fallback logic within the swap attempt, or if they are dead code that should
be removed entirely from the method signature. If they encode intended behavior,
implement the logic that uses them; if they are unused, remove them from the
method signature and update all call sites (around lines 50 and 67) to stop
passing them.
- Around line 26-57: The early return statement in the systemMajor threshold
check (at lines 26-33) prevents debug override properties from being evaluated,
making it impossible to test force-swap and nudge features on devices with
recent WebView versions. Move the retrieval and evaluation of the debug override
properties (PROP_FORCE via SystemProps.get and PROP_NUDGE check) before the
early return threshold comparison, so that debug overrides are checked
regardless of the system WebView version. This allows the debug paths in trySwap
and OutdatedWebViewNotice.schedule to execute even when systemMajor is already
recent enough, and then remove the now-redundant re-reads of forcePkg and
PROP_NUDGE that appear after the threshold gate.
In `@src/renderer/styles/android.css`:
- Around line 830-851: The CSS rules at lines 830-851 that target
`[data-slot="input"]`, `[data-slot="textarea"]`, `.mo-input-prepend`, and
`.mo-input-append` are applying globally to all mobile inputs. Scope these rules
to only apply within the add-task container by adding the add-task container
selector as a parent to each rule set. This will prevent the 48px sizing
constraints from unintentionally affecting input elements outside the add-task
flow on mobile and Android platforms.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72b6a1ce-c5be-4fe4-aa6f-dd2ed11851dd
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
.gitignoreREADME.mdsrc-tauri/Cargo.tomlsrc-tauri/risuko-cookies/src/browser/mod.rssrc-tauri/risuko-cookies/src/lib.rssrc-tauri/risuko-cookies/src/utils/mod.rssrc-tauri/risuko-webview-upgrade/.gitignoresrc-tauri/risuko-webview-upgrade/Cargo.tomlsrc-tauri/risuko-webview-upgrade/android/build.gradle.ktssrc-tauri/risuko-webview-upgrade/android/consumer-rules.prosrc-tauri/risuko-webview-upgrade/android/proguard-rules.prosrc-tauri/risuko-webview-upgrade/android/settings.gradlesrc-tauri/risuko-webview-upgrade/android/src/main/AndroidManifest.xmlsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/BootstrapProvider.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/Common.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/HiddenApi.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/OutdatedWebViewNotice.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/SystemProps.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/reflect/Reflect.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxExtras.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxedProcessServiceDelegate.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService0.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService1.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService2.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService3.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService4.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ActivityManagerHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BindServiceArgsAdapter.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BinderHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/Framework.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/PackageManagerHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ProxyBinder.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewSwap.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewUpdateServiceHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/res/values-ja/strings.xmlsrc-tauri/risuko-webview-upgrade/android/src/main/res/values-zh-rCN/strings.xmlsrc-tauri/risuko-webview-upgrade/android/src/main/res/values-zh-rTW/strings.xmlsrc-tauri/risuko-webview-upgrade/android/src/main/res/values/strings.xmlsrc-tauri/risuko-webview-upgrade/build.rssrc-tauri/risuko-webview-upgrade/src/lib.rssrc-tauri/src/lib.rssrc-tauri/src/managers/flyout.rssrc-tauri/src/state.rssrc-tauri/src/utils/run_mode.rssrc-tauri/tauri.conf.jsonsrc/renderer/components/DragSelect/Index.vuesrc/renderer/styles/android.css
💤 Files with no reviewable changes (2)
- README.md
- src-tauri/src/managers/flyout.rs
Comment on lines
+48
to
+53
| fun parsePluginBlock(block: Map<String, Any?>?): Pair<Int, Int> { | ||
| if (block == null) return defaultMinUpgradeMajor to defaultMinSupportedMajor | ||
| val upgrade = (block["minUpgradeMajor"] as? Number)?.toInt() ?: defaultMinUpgradeMajor | ||
| val supported = (block["minSupportedMajor"] as? Number)?.toInt() ?: defaultMinSupportedMajor | ||
| return upgrade to supported | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail fast on invalid threshold config instead of silently defaulting.
Line 50-Line 52 and Line 95 currently coerce malformed/invalid explicit config into defaults, which can silently ship wrong swap thresholds. Treat malformed env/plugin values as build errors, and validate bounds (> 0, minSupportedMajor <= minUpgradeMajor).
Suggested fix
+import org.gradle.api.GradleException
import groovy.json.JsonSlurper
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
@@
`@Suppress`("UNCHECKED_CAST")
fun parsePluginBlock(block: Map<String, Any?>?): Pair<Int, Int> {
if (block == null) return defaultMinUpgradeMajor to defaultMinSupportedMajor
- val upgrade = (block["minUpgradeMajor"] as? Number)?.toInt() ?: defaultMinUpgradeMajor
- val supported = (block["minSupportedMajor"] as? Number)?.toInt() ?: defaultMinSupportedMajor
+ val upgrade = (block["minUpgradeMajor"] as? Number)?.toInt() ?: defaultMinUpgradeMajor
+ val supported = (block["minSupportedMajor"] as? Number)?.toInt() ?: defaultMinSupportedMajor
+ if (upgrade <= 0 || supported <= 0) {
+ throw GradleException("webview-upgrade config values must be positive: minUpgradeMajor=$upgrade, minSupportedMajor=$supported")
+ }
+ if (supported > upgrade) {
+ throw GradleException("webview-upgrade config is invalid: minSupportedMajor ($supported) must be <= minUpgradeMajor ($upgrade)")
+ }
return upgrade to supported
}
@@
val (minUpgrade, minSupported, source) = when {
- !envJson.isNullOrBlank() ->
- readFromEnv(envJson)?.let { Triple(it.first, it.second, "TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG env var") }
- ?: Triple(defaultMinUpgradeMajor, defaultMinSupportedMajor, "defaults (env var unparseable)")
+ !envJson.isNullOrBlank() -> {
+ val parsed = readFromEnv(envJson)
+ ?: throw GradleException("TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG is not valid JSON object for webview-upgrade config")
+ Triple(parsed.first, parsed.second, "TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG env var")
+ }
tauriConfFile != null ->
readFromTauriConf(tauriConfFile)?.let { Triple(it.first, it.second, "tauri.conf.json at $tauriConfFile") }
?: Triple(defaultMinUpgradeMajor, defaultMinSupportedMajor, "defaults (tauri.conf.json had no block)")Also applies to: 92-99
🤖 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 `@src-tauri/risuko-webview-upgrade/android/build.gradle.kts` around lines 48 -
53, The parsePluginBlock function and similar configuration parsing code
(referenced at lines 92-99) silently default to fallback values when
configuration is malformed or invalid, which can hide configuration errors.
Modify parsePluginBlock to validate that both parsed values are greater than 0
and that minSupportedMajor is less than or equal to minUpgradeMajor, throwing a
clear build error with a descriptive message when validation fails instead of
using the elvis operator to default. Apply the same validation logic to the
other configuration parsing code mentioned in the comment at lines 92-99.
Comment on lines
+27
to
+30
| private var onBindMethod: java.lang.reflect.Method? = null | ||
| private var onDestroyMethod: java.lang.reflect.Method? = null | ||
| private var onRebindMethod: java.lang.reflect.Method? = null | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Forward onUnbind to the real service; onRebind is currently unreachable.
Line 61 adds rebind delegation, but none of the delegate/stub flow preserves onUnbind’s boolean return. Android only calls onRebind after an onUnbind that returned true, and default Service.onUnbind() returns false. This drops the real service’s unbind lifecycle and can break sandbox process state management.
Suggested fix
internal class SandboxedProcessServiceDelegate {
@@
private var onRebindMethod: java.lang.reflect.Method? = null
+ private var onUnbindMethod: java.lang.reflect.Method? = null
@@
+ fun onUnbind(intent: Intent): Boolean {
+ if (!ready) return false
+ val real = realService ?: return false
+ val unbind = onUnbindMethod ?: return false
+ return try {
+ (unbind.invoke(real, intent) as? Boolean) ?: false
+ } catch (t: Throwable) {
+ Log.e(LOG_TAG, "sandbox onUnbind delegate failed", t)
+ false
+ }
+ }
@@
onRebindMethod = runCatching {
realClass.getMethod("onRebind", Intent::class.java).apply { isAccessible = true }
}.getOrNull()
+ onUnbindMethod = runCatching {
+ realClass.getMethod("onUnbind", Intent::class.java).apply { isAccessible = true }
+ }.getOrNull() class StubSandboxedProcessService0 : Service() {
@@
override fun onBind(intent: Intent): IBinder? = delegate.onBind(intent)
+ override fun onUnbind(intent: Intent): Boolean = delegate.onUnbind(intent)
override fun onRebind(intent: Intent) {Apply the same onUnbind override pattern to StubSandboxedProcessService1..4.
Also applies to: 61-65, 319-342
🤖 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
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxedProcessServiceDelegate.kt`
around lines 27 - 30, The onUnbind method is not being forwarded to the real
service in the stub implementations, which prevents onRebind from being called
and breaks the unbind lifecycle. Add an onUnbind override method to each of the
stub service classes (StubSandboxedProcessService1,
StubSandboxedProcessService2, StubSandboxedProcessService3, and
StubSandboxedProcessService4) that uses the reflection pattern to call the
onUnbindMethod on the delegate service, ensuring the boolean return value is
preserved and returned. This matches the existing onRebind delegation pattern
already implemented and restores the proper unbind lifecycle chain.
Comment on lines
+232
to
+317
| private fun injectDex( | ||
| apkPath: String, | ||
| nativeLibDir: String?, | ||
| sharedLibs: Array<String>?, | ||
| mergeWebViewApkIntoHost: Boolean, | ||
| ): Boolean { | ||
| return try { | ||
| if (!mergeWebViewApkIntoHost && (sharedLibs == null || sharedLibs.isEmpty())) { | ||
| return true | ||
| } | ||
| val currentCl = appContext?.classLoader ?: return false | ||
| val dexPathListClass = Class.forName("dalvik.system.DexPathList") | ||
| val dexElementsField = dexPathListClass.getDeclaredField("dexElements").apply { isAccessible = true } | ||
| val nativeLibPathElementsField = runCatching { | ||
| dexPathListClass.getDeclaredField("nativeLibraryPathElements").apply { isAccessible = true } | ||
| }.getOrNull() | ||
| val pathListField = Class.forName("dalvik.system.BaseDexClassLoader") | ||
| .getDeclaredField("pathList").apply { isAccessible = true } | ||
|
|
||
| var webviewDexElements = arrayOfNulls<Any>(0) | ||
| var webviewNativeElements = arrayOfNulls<Any>(0) | ||
| if (mergeWebViewApkIntoHost) { | ||
| val webviewLoader = PathClassLoader(apkPath, nativeLibDir, currentCl.parent) | ||
| val webviewPathList = pathListField.get(webviewLoader) | ||
| webviewDexElements = dexElementsField.get(webviewPathList) as Array<Any?> | ||
| if (nativeLibPathElementsField != null) { | ||
| (nativeLibPathElementsField.get(webviewPathList) as? Array<Any?>)?.let { webviewNativeElements = it } | ||
| } | ||
| } | ||
|
|
||
| var sharedDexElements = arrayOfNulls<Any>(0) | ||
| var sharedNativeElements = arrayOfNulls<Any>(0) | ||
| if (sharedLibs != null && sharedLibs.isNotEmpty()) { | ||
| val sharedPath = sharedLibs.joinToString(File.pathSeparator) | ||
| val sharedLoader = PathClassLoader(sharedPath, null, currentCl.parent) | ||
| val sharedPathList = pathListField.get(sharedLoader) | ||
| (dexElementsField.get(sharedPathList) as? Array<Any?>)?.let { sharedDexElements = it } | ||
| if (nativeLibPathElementsField != null) { | ||
| (nativeLibPathElementsField.get(sharedPathList) as? Array<Any?>)?.let { sharedNativeElements = it } | ||
| } | ||
| } | ||
|
|
||
| val currentPathList = pathListField.get(currentCl) | ||
| val oldDexElements = dexElementsField.get(currentPathList) as? Array<Any?> | ||
| val oldLen = oldDexElements?.size ?: 0 | ||
| val webLen = webviewDexElements.size | ||
| val sharedLen = sharedDexElements.size | ||
| if (webLen == 0 && sharedLen == 0) { | ||
| Log.e(LOG_TAG, "no dex elements extracted") | ||
| return false | ||
| } | ||
| val componentType = (oldDexElements ?: webviewDexElements).javaClass.componentType!! | ||
| @Suppress("UNCHECKED_CAST") | ||
| val combined = ReflectArray.newInstance(componentType, webLen + sharedLen + oldLen) as Array<Any?> | ||
| if (webLen > 0) System.arraycopy(webviewDexElements, 0, combined, 0, webLen) | ||
| if (sharedLen > 0) System.arraycopy(sharedDexElements, 0, combined, webLen, sharedLen) | ||
| if (oldLen > 0) System.arraycopy(oldDexElements!!, 0, combined, webLen + sharedLen, oldLen) | ||
| dexElementsField.set(currentPathList, combined) | ||
|
|
||
| if (nativeLibPathElementsField != null) { | ||
| val oldNative = nativeLibPathElementsField.get(currentPathList) as? Array<Any?> | ||
| val oldNativeLen = oldNative?.size ?: 0 | ||
| val webNativeLen = webviewNativeElements.size | ||
| val sharedNativeLen = sharedNativeElements.size | ||
| if (webNativeLen > 0 || sharedNativeLen > 0) { | ||
| val nativeComponentType = when { | ||
| webNativeLen > 0 -> webviewNativeElements.javaClass.componentType!! | ||
| oldNativeLen > 0 -> oldNative!!.javaClass.componentType!! | ||
| else -> sharedNativeElements.javaClass.componentType!! | ||
| } | ||
| @Suppress("UNCHECKED_CAST") | ||
| val combinedNative = ReflectArray.newInstance( | ||
| nativeComponentType, webNativeLen + sharedNativeLen + oldNativeLen, | ||
| ) as Array<Any?> | ||
| if (webNativeLen > 0) System.arraycopy(webviewNativeElements, 0, combinedNative, 0, webNativeLen) | ||
| if (sharedNativeLen > 0) System.arraycopy(sharedNativeElements, 0, combinedNative, webNativeLen, sharedNativeLen) | ||
| if (oldNativeLen > 0) System.arraycopy(oldNative!!, 0, combinedNative, webNativeLen + sharedNativeLen, oldNativeLen) | ||
| nativeLibPathElementsField.set(currentPathList, combinedNative) | ||
| } | ||
| } | ||
| true | ||
| } catch (t: Throwable) { | ||
| Log.e(LOG_TAG, "dex injection failed", t) | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
Split injectDex into smaller units before this path becomes unmaintainable.
Line 232 currently combines extraction, merge ordering, component-type resolution, and native element patching in one method; detekt already flags both cyclomatic and cognitive complexity beyond threshold. In this reflection-heavy bootstrap path, this materially increases regression risk.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 232-232: The function injectDex appears to be too complex based on Cyclomatic Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CyclomaticComplexMethod)
[warning] 232-232: The function injectDex appears to be too complex based on Cognitive Complexity (complexity: 32). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
[warning] 313-313: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
🤖 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
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxedProcessServiceDelegate.kt`
around lines 232 - 317, The injectDex method has high cyclomatic and cognitive
complexity due to handling multiple concerns: extracting dex elements,
extracting native elements, merging arrays, and determining component types.
Break this into smaller focused helper methods by extracting the following
responsibilities into separate private functions: create a method to extract dex
and native elements from a given PathClassLoader (used for both webview and
shared loaders), create a method to combine dex element arrays given their
individual sizes and the current path list, and create a method to combine
native element arrays with similar parameters. This refactoring will reduce the
main injectDex method's complexity while maintaining clarity through descriptive
method names and keeping each helper method to a single responsibility.
Source: Linters/SAST tools
Comment on lines
+43
to
+58
| val parcel = Parcel.obtain() | ||
| return try { | ||
| parcel.writeParcelable(response as Parcelable, 0) | ||
| parcel.setDataPosition(parcel.dataSize() - 4) | ||
| parcel.writeInt(0) | ||
| parcel.setDataPosition(0) | ||
| val loader = response.javaClass.classLoader | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| parcel.readParcelable(loader, response.javaClass) | ||
| } else { | ||
| @Suppress("DEPRECATION") | ||
| parcel.readParcelable(loader) | ||
| } | ||
| } finally { | ||
| parcel.recycle() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Android WebViewProviderResponse writeToParcel field order packageInfo status source code AOSP
💡 Result:
In the AOSP source code for android.webkit.WebViewProviderResponse, the writeToParcel method writes its fields to the Parcel in the following order: 1. packageInfo (as a TypedObject) [1][2] 2. status (as an int) [1][2] The implementation in the writeToParcel(Parcel out, int flags) method is as follows: out.writeTypedObject(packageInfo, flags); [1][2] out.writeInt(status); [1][2] Correspondingly, the constructor used by the Parcelable.Creator to recreate the object from a Parcel reads these fields in the exact same order: packageInfo = in.readTypedObject(PackageInfo.CREATOR); [1][2] status = in.readInt; [1][2] This ensures that the marshaling and unmarshaling processes remain consistent. [3]
Citations:
- 1: https://www.javatips.net/api/platform_frameworks_base-master/core/java/android/webkit/WebViewProviderResponse.java
- 2: https://www.javatips.net/api/android_frameworks_base-master/android_frameworks_base-cm-14.1/core/java/android/webkit/WebViewProviderResponse.java
- 3: https://android.googlesource.com/platform/frameworks/base/+/HEAD/core/java/android/os/Parcelable.java
🌐 Web query:
WebViewProviderResponse field order history Android version changes SDK evolution
💡 Result:
WebViewProviderResponse is an internal Android class, marked with the @hide annotation, used primarily for communication between the WebViewFactory and the system's WebViewUpdateService [1][2][3]. It is not part of the public Android SDK API [4][3]. Because this class is internal, its structure is not subject to public API compatibility guarantees and is intended for use only by Android system implementers [4]. Throughout its existence in the Android framework, the core purpose and structure of WebViewProviderResponse have remained stable, serving as a container to return the package information of the WebView provider and the status of the WebView loading process [3]. The class contains exactly two fields [3]: 1. packageInfo (of type android.content.pm.PackageInfo) [3] 2. status (of type int) [3] These fields are defined and accessed within the context of AIDL interfaces and internal service logic [5][1]. In its implementation, specifically within the writeToParcel method used for inter-process communication (IPC) via Binder, the field order is strictly defined to ensure compatibility between the service and the client [3]: 1. packageInfo is written and read first [3]. 2. status is written and read second [3]. There is no record of "field order history changes" because the internal structure has remained consistent to maintain the stability of the WebView initialization process, which is critical for system-level security and performance [6][3]. Changes to this internal class, if they were to occur, would be managed strictly within the Android platform source code repository and would be coupled with changes to the corresponding AIDL definitions and Parcelable implementation to prevent breaking the system server's communication with apps [5][1][3].
Citations:
- 1: https://android.googlesource.com/platform/frameworks/base/+/master/services/core/java/com/android/server/webkit/WebViewUpdateService.java
- 2: https://android.googlesource.com/platform/frameworks/base/+/android-8.1.0_r27/core/java/android/webkit/WebViewFactory.java
- 3: https://www.javatips.net/api/platform_frameworks_base-master/core/java/android/webkit/WebViewProviderResponse.java
- 4: https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/WebViewProvider.java
- 5: https://android.googlesource.com/platform/frameworks/base/+/main/core/java/android/webkit/IWebViewUpdateService.aidl
- 6: https://chromium.googlesource.com/chromium/src/+/HEAD/android_webview/docs/webview-providers.md
🌐 Web query:
Android Parcel internal structure stability version compatibility warnings
💡 Result:
The Android Parcel class is not a general-purpose serialization mechanism and does not maintain stable structure or compatibility across different versions of the Android platform [1][2][3]. Official documentation explicitly warns that the marshalled representation is highly optimized for local Inter-Process Communication (IPC) and that changes in the underlying implementation of any data within a Parcel can render older data unreadable [1][2][4]. Key stability and compatibility constraints include: Persistence Warning: You must never store Parcel data in persistent storage (e.g., local disk, across a network). Because the format is unstable across platform versions, data saved by one version of the system may become unreadable after a system update or when accessed by a different platform version [1][3][4]. IPC Design: Parcel is designed specifically as a high-performance IPC transport [2][3]. To maintain speed, it consciously avoids the overhead associated with versioning and forward/backward compatibility [5]. Stable AIDL Solution: For scenarios requiring versioned, stable interfaces (such as communication between components that may be updated independently), developers should use Stable AIDL [6][5]. Stable AIDL requires that all parcelables be defined in AIDL (structured) and supports explicit versioning, allowing the build system to enforce backward compatibility and track API changes [6][7]. General Best Practice: Unless you define a Parcelable yourself or it provides explicit guarantees regarding its internal composition, you should not assume that its marshalled return value will remain consistent across different environments or platform versions [8]. If you require a mechanism that supports versioning and long-term storage, use standard serialization mechanisms (e.g., JSON, Protobuf, or Parcelable with explicit versioning logic if strictly necessary) rather than raw Parcel data [1][3][5].
Citations:
- 1: https://developer.android.com/reference/android/os/Parcel
- 2: https://developer.android.com/reference/kotlin/android/os/Parcel
- 3: https://android.googlesource.com/platform/frameworks/base/+/HEAD/core/java/android/os/Parcel.java
- 4: https://android.googlesource.com/platform/frameworks/base/+/4e6a73c16ab35c6a8d7f524fa2dad6b8b822d7a9/core/java/android/os/Parcel.java
- 5: https://stackoverflow.com/questions/18197783/android-aidl-interface-parcelables-and-backwards-compatibility
- 6: https://source.android.google.cn/docs/core/architecture/aidl/stable-aidl
- 7: https://source.android.com/docs/core/architecture/aidl/stable-aidl-apis
- 8: https://android.googlesource.com/platform/frameworks/base/%2B/HEAD/core/java/android/os/Parcel.java
Guard the parcel field offset assumption against future framework changes.
The WebViewProviderResponse field order (packageInfo, then status as final int) is currently stable in AOSP, confirming that dataSize() - 4 correctly targets the status field. However, Android's official documentation warns that Parcel's marshalled format is not guaranteed to remain stable across platform versions—the underlying implementation can change independently of the class structure. While the current implementation is correct, relying on the internal byte offset of a framework class without an explicit SDK-level guard is fragile for a system-critical component. Consider either (1) confirming via integration tests across all supported API levels that the offset remains valid, or (2) wrapping this marshalling logic with a version check or fallback mechanism to detect and handle format changes gracefully.
🤖 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
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewUpdateServiceHook.kt`
around lines 43 - 58, The code in the try block assumes that the status field in
WebViewProviderResponse is always located at dataSize() - 4 bytes, which relies
on an undocumented internal Parcel format that Android does not guarantee will
remain stable across versions. Add a version check or fallback mechanism around
the parcel manipulation logic to handle potential format changes gracefully.
Consider wrapping the offset assumption
(parcel.setDataPosition(parcel.dataSize() - 4) and subsequent
parcel.writeInt(0)) with an explicit SDK-level guard or exception handling that
can detect and handle cases where the expected field offset may have changed in
future Android versions.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
16 issues found across 50 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="src-tauri/risuko-webview-upgrade/Cargo.toml">
<violation number="1" location="src-tauri/risuko-webview-upgrade/Cargo.toml:1">
P3: Missing `[lib]` section - all other workspace library crates explicitly declare one.</violation>
</file>
<file name="src-tauri/src/lib.rs">
<violation number="1" location="src-tauri/src/lib.rs:48">
P3: `with_desktop_plugins` on Android registers an Android-specific WebView upgrade plugin — the function name is misleading for this cfg path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| #[cfg(target_os = "android")] | ||
| fn with_desktop_plugins<R: tauri::Runtime>(builder: tauri::Builder<R>) -> tauri::Builder<R> { | ||
| builder | ||
| builder.plugin(risuko_webview_upgrade::init()) |
There was a problem hiding this comment.
P3: with_desktop_plugins on Android registers an Android-specific WebView upgrade plugin — the function name is misleading for this cfg path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/lib.rs, line 48:
<comment>`with_desktop_plugins` on Android registers an Android-specific WebView upgrade plugin — the function name is misleading for this cfg path.</comment>
<file context>
@@ -45,7 +45,7 @@ fn with_desktop_plugins<R: tauri::Runtime>(builder: tauri::Builder<R>) -> tauri:
#[cfg(target_os = "android")]
fn with_desktop_plugins<R: tauri::Runtime>(builder: tauri::Builder<R>) -> tauri::Builder<R> {
- builder
+ builder.plugin(risuko_webview_upgrade::init())
}
</file context>
| @@ -0,0 +1,17 @@ | |||
| [package] | |||
There was a problem hiding this comment.
P3: Missing [lib] section - all other workspace library crates explicitly declare one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/risuko-webview-upgrade/Cargo.toml, line 1:
<comment>Missing `[lib]` section - all other workspace library crates explicitly declare one.</comment>
<file context>
@@ -0,0 +1,17 @@
+[package]
+name = "risuko-webview-upgrade"
+description = "Swap the in-process Android WebView kernel to a newer installed Chrome / Google WebView when the system WebView is too old to render Risuko."
</file context>
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
2 issues found across 17 files (changes from recent commits).
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="src-tauri/risuko-webview-upgrade/Cargo.toml">
<violation number="1" location="src-tauri/risuko-webview-upgrade/Cargo.toml:1">
P3: Missing `[lib]` section - all other workspace library crates explicitly declare one.</violation>
</file>
<file name="src-tauri/src/lib.rs">
<violation number="1" location="src-tauri/src/lib.rs:48">
P3: `with_desktop_plugins` on Android registers an Android-specific WebView upgrade plugin — the function name is misleading for this cfg path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.kt (1)
52-61: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon’t suppress the outdated-WebView notice after swapping to an unsupported candidate.
bestCandidate()only guaranteescandidate.second > systemMajor; a successful swap from major 60 to 70 returns before theminSupportedcheck, so no update notice is shown even though the active WebView is still unsupported. The same notice path also excludes unknown current versions because-1is not in0 until minSupported.Proposed fix
val candidate = bestCandidate(context, systemMajor) if (candidate != null) { Log.i(LOG_TAG, "candidate kernel: ${candidate.first} major=${candidate.second}") - if (trySwap(context, candidate.first)) return + if (trySwap(context, candidate.first)) { + if (candidate.second < minSupported) { + OutdatedWebViewNotice.schedule(context, candidate.first) + } + return + } } else { Log.i(LOG_TAG, "no newer installed WebView candidate found") } - if (systemMajor in 0 until minSupported) { + if (systemMajor < minSupported) { OutdatedWebViewNotice.schedule(context, candidate?.first ?: bestCandidatePackage(context)) }🤖 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 `@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.kt` around lines 52 - 61, The issue is that when trySwap() successfully swaps to a newer WebView candidate, the function returns immediately without checking if the new version is still below minSupported. This means users get upgraded to an unsupported version without being notified. Fix this by checking if the swapped candidate version (candidate.second) is still below minSupported after a successful swap, and if so, schedule the OutdatedWebViewNotice using the swapped candidate package before returning from the trySwap success path.src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BindServiceArgsAdapter.kt (1)
29-44: 🩺 Stability & Availability | 🟠 MajorReturn
nullwhen no feasible bindService overload matched.The filtering loop (lines 31-42) only scores candidates that pass both filter conditions: parameter count ≤ source length and gap ≤ 3. When no candidate satisfies these constraints,
bestremains null. However, line 44 returnscandidates[0]unconditionally, which bypasses the filtering logic and sends a rejected method to the caller. This causesActivityManagerHook.callBindServiceOnRealAM()to attempt invoking an incompatible signature instead of handling the null case at line 130-132.Proposed fix
- return best ?: candidates[0] + return best🤖 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 `@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BindServiceArgsAdapter.kt` around lines 29 - 44, The filtering loop in the method only assigns to the `best` variable when candidates satisfy the parameter count and gap constraints, but the return statement at line 44 returns `candidates[0]` unconditionally regardless of whether `best` was set. Replace the return statement to return `best` directly instead of `candidates[0]`, so that null is properly returned when no candidate satisfies the filtering criteria and the caller can handle the null case appropriately.src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ActivityManagerHook.kt (1)
126-139: 🩺 Stability & Availability | 🟠 MajorPass the fallback closure to
callBindServiceOnRealAM()to prevent returningnullfor primitive return types.The intercepted
bindIsolatedServiceandbindServiceInstancemethods returnint(status code on IActivityManager). WhencallBindServiceOnRealAM()fails—either due to missingoriginalAm, no matching overload, or reflection errors—it currently returnsnull. This null return can cause unboxing exceptions during proxy return type coercion, or silently drop the bind request if the return type is non-primitive. Fallback to the original intercepted method ensures the call is handled by the real ActivityManager as a safe default.Proposed fix
private fun dispatchBind(args: Array<Any?>, original: () -> Any?): Any? { val redirected = args.filterIsInstance<Intent>().firstOrNull { redirectSandboxServiceIfNeeded(it) } - return if (redirected != null) callBindServiceOnRealAM(args) else original() + return if (redirected != null) callBindServiceOnRealAM(args, original) else original() } - private fun callBindServiceOnRealAM(originalArgs: Array<Any?>): Any? { - val am = originalAm ?: return null + private fun callBindServiceOnRealAM(originalArgs: Array<Any?>, fallback: () -> Any?): Any? { + val am = originalAm ?: return fallback() return try { val bindServiceMethod = BindServiceArgsAdapter.findBestBindServiceMethod(am, originalArgs) if (bindServiceMethod == null) { Log.e(LOG_TAG, "no bindService overload found on AMS") - return null + return fallback() } val adapted = BindServiceArgsAdapter.adapt(originalArgs, bindServiceMethod, hostPackageName) bindServiceMethod.invoke(am, *adapted) } catch (t: Throwable) { Log.e(LOG_TAG, "forwarding bindService failed", t) - null + fallback() } }🤖 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 `@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ActivityManagerHook.kt` around lines 126 - 139, The callBindServiceOnRealAM() function returns null when failures occur (missing originalAm, no matching overload, or reflection errors), which causes unboxing exceptions for primitive return types like int. Modify the callBindServiceOnRealAM() function signature to accept a fallback closure parameter that represents the original intercepted method. Then update all the return null statements within the function (when originalAm is null, when no bindService overload is found, and in the catch block) to invoke the fallback closure instead, ensuring the call falls back to the real ActivityManager as a safe default. Update all call sites of callBindServiceOnRealAM() to pass the appropriate fallback closure.
♻️ Duplicate comments (1)
src-tauri/risuko-webview-upgrade/android/build.gradle.kts (1)
59-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail fast for invalid explicit env config instead of silently defaulting.
Line 64 catches
Exception, which also capturesIllegalArgumentExceptionraised byparsePluginBlock(Lines 52-54). That turns an explicitly invalidTAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIGinto silent defaults, so misconfigured thresholds can ship unnoticed.Suggested fix
+import groovy.json.JsonException +import org.gradle.api.GradleException @@ fun readFromEnv(jsonText: String?): Pair<Int, Int>? { if (jsonText.isNullOrBlank()) return null - return try { - val parsed = JsonSlurper().parseText(jsonText) as? Map<String, Any?> ?: return null - parsePluginBlock(parsed) - } catch (e: Exception) { - logger.warn("TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG parse error: ${e.message}; using defaults") - null - } + val parsed = try { + JsonSlurper().parseText(jsonText) as? Map<String, Any?> + ?: throw GradleException("TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG must be a JSON object") + } catch (e: JsonException) { + throw GradleException("TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG is not valid JSON", e) + } + return try { + parsePluginBlock(parsed) + } catch (e: IllegalArgumentException) { + throw GradleException("Invalid webview-upgrade env config: ${e.message}", e) + } }🤖 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 `@src-tauri/risuko-webview-upgrade/android/build.gradle.kts` around lines 59 - 66, The readFromEnv function's catch block is too broad—it catches all Exception types including the IllegalArgumentException raised by parsePluginBlock, which masks invalid configuration and causes silent defaults. Narrow the exception handling to only catch JSON parsing exceptions from JsonSlurper().parseText() (such as groovy.json.JsonException or similar), while allowing IllegalArgumentException from parsePluginBlock to propagate uncaught so invalid TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG values fail fast instead of silently reverting to defaults.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ActivityManagerHook.kt`:
- Around line 126-139: The callBindServiceOnRealAM() function returns null when
failures occur (missing originalAm, no matching overload, or reflection errors),
which causes unboxing exceptions for primitive return types like int. Modify the
callBindServiceOnRealAM() function signature to accept a fallback closure
parameter that represents the original intercepted method. Then update all the
return null statements within the function (when originalAm is null, when no
bindService overload is found, and in the catch block) to invoke the fallback
closure instead, ensuring the call falls back to the real ActivityManager as a
safe default. Update all call sites of callBindServiceOnRealAM() to pass the
appropriate fallback closure.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BindServiceArgsAdapter.kt`:
- Around line 29-44: The filtering loop in the method only assigns to the `best`
variable when candidates satisfy the parameter count and gap constraints, but
the return statement at line 44 returns `candidates[0]` unconditionally
regardless of whether `best` was set. Replace the return statement to return
`best` directly instead of `candidates[0]`, so that null is properly returned
when no candidate satisfies the filtering criteria and the caller can handle the
null case appropriately.
In
`@src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.kt`:
- Around line 52-61: The issue is that when trySwap() successfully swaps to a
newer WebView candidate, the function returns immediately without checking if
the new version is still below minSupported. This means users get upgraded to an
unsupported version without being notified. Fix this by checking if the swapped
candidate version (candidate.second) is still below minSupported after a
successful swap, and if so, schedule the OutdatedWebViewNotice using the swapped
candidate package before returning from the trySwap success path.
---
Duplicate comments:
In `@src-tauri/risuko-webview-upgrade/android/build.gradle.kts`:
- Around line 59-66: The readFromEnv function's catch block is too broad—it
catches all Exception types including the IllegalArgumentException raised by
parsePluginBlock, which masks invalid configuration and causes silent defaults.
Narrow the exception handling to only catch JSON parsing exceptions from
JsonSlurper().parseText() (such as groovy.json.JsonException or similar), while
allowing IllegalArgumentException from parsePluginBlock to propagate uncaught so
invalid TAURI_WEBVIEW_UPGRADE_PLUGIN_CONFIG values fail fast instead of silently
reverting to defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9558ed67-c781-4cf8-9a94-362df66d81e3
📒 Files selected for processing (17)
src-tauri/risuko-webview-upgrade/android/build.gradle.ktssrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/Common.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/OutdatedWebViewNotice.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/WebViewUpgradeBootstrap.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxExtras.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/SandboxedProcessServiceDelegate.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService0.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService1.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService2.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService3.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/sandbox/StubSandboxedProcessService4.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/ActivityManagerHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BindServiceArgsAdapter.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/BinderHook.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/Framework.ktsrc-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewSwap.ktsrc/renderer/styles/android.css
💤 Files with no reviewable changes (2)
- src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/Framework.kt
- src-tauri/risuko-webview-upgrade/android/src/main/java/app/risuko/webview_upgrade/swap/WebViewSwap.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds an Android WebView upgrade plugin that swaps the app’s WebView to a newer installed Chrome/WebView when the system WebView is too old, preventing blank or broken UI. It shows a localized native notice with a Play Store link if no upgrade path exists.
New Features
risuko-webview-upgradeplugin: swaps to a newer WebView when below a version threshold; includes sandboxed renderer stubs and system hooks.tauri.conf.json:plugins.webview-upgrade.minUpgradeMajor(106) andminSupportedMajor(75); testing via system props (debug.risuko.wvup.force,debug.risuko.wvup.nudge,debug.risuko.wvup.minupgrade,debug.risuko.wvup.minsupported).Migration
Written for commit 7cd4529. Summary will update on new commits.
Summary by CodeRabbit