make v3 android example working in windows#4984
make v3 android example working in windows#4984a1012112796 wants to merge 9 commits intowailsapp:v3-alphafrom
Conversation
…controls - Added methods to enable/disable scrolling, bounce effects, and scroll indicators in the Android WebView. - Implemented back/forward swipe gesture navigation and link preview toggling. - Introduced custom user agent setting for the WebView. - Updated the Android runtime to handle new method calls from the frontend. - Created corresponding TypeScript interfaces for the new Android functionalities. - Ensured compatibility with existing methods and maintained a clean interface for future enhancements.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds comprehensive Android support across example app, runtime, Java bridge, JNI/Go integration, event wiring, and build tasks — introducing Android runtime APIs, native tabs, WebView controls, new message/transport paths, platform-specific stubs, and Windows-aware Gradle invocation in Taskfiles. Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend (JS)
participant RT as Runtime TS
participant GO as Go App
participant JNI as JNI layer
participant JAVA as Android (WailsBridge)
FE->>RT: Android.Device.Info()
RT->>GO: runtime call (Android Device Info)
GO->>JNI: getDeviceInfoJsonOnBridge()
JNI->>JAVA: WailsBridge.getDeviceInfoJson()
JAVA-->>JNI: JSON device info
JNI-->>GO: parsed JSON
GO-->>RT: return device info
RT-->>FE: resolve Promise
sequenceDiagram
participant FE as Frontend (JS)
participant EVT as Example event handler
participant GO as Go App
participant JNI as JNI layer
participant JAVA as Android (WailsBridge/WebView)
FE->>EVT: emit android:setScrollEnabled payload
EVT->>GO: registerAndroidRuntimeEventHandlers parses payload
GO->>JNI: setScrollEnabledOnBridge(true/false)
JNI->>JAVA: WailsBridge.setScrollEnabled(...)
JAVA->>JAVA: applyWebViewSettings()
JAVA-->>GO: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.1)v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsBridge.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m v3/pkg/application/application_android.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsJSBridge.java (1)
36-38:⚠️ Potential issue | 🟡 MinorDebug logging of full messages may leak sensitive data in production.
Log.d(TAG, "Invoke called: " + message)logs every IPC message (which may contain user data or tokens) to logcat. On Android, other apps withREAD_LOGSpermission or ADB access can read these. Consider guarding withBuildConfig.DEBUGor removing in production builds.v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java (2)
186-189:⚠️ Potential issue | 🟡 MinorUnescaped exception message in JSON response — potential JSON breakage.
e.getMessage()may contain double quotes, backslashes, or control characters that break the hand-crafted JSON string. UseJSONObjectto build the error response safely, or at least escape the message.🐛 Proposed fix
} catch (Exception e) { Log.e(TAG, "Error handling message", e); - return "{\"error\":\"" + e.getMessage() + "\"}"; + JSONObject err = new JSONObject(); + try { err.put("error", e.getMessage()); } catch (JSONException ignored) {} + return err.toString(); }
480-485:⚠️ Potential issue | 🟡 Minor
escapeJsStringdoes not escape</script>, null bytes, or Unicode line terminators.If
eventNameor any string passed through this function contains sequences like</script>,\u2028,\u2029, or\0, the resulting JS injection could break or be exploitable. Consider using a JSON serializer (e.g.,JSONObject.quote()ornew JSONObject().put("v", str).toString()to extract the escaped value) for safer escaping.v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsBridge.java (1)
33-497:⚠️ Potential issue | 🟠 MajorThis file is an exact duplicate of
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java; consolidate these copies to prevent divergence.Maintaining two identical copies of
WailsBridge.javais error-prone. Future bug fixes, security updates, or feature additions to one will inevitably diverge from the other.The code also contains several issues that exist in both copies:
- Thread safety: Mutable fields (
nativeTabTitles,nativeTabsEnabled,scrollEnabled,bounceEnabled,scrollIndicatorsEnabled,backForwardGesturesEnabled,linkPreviewEnabled,customUserAgent,touchStartX,touchStartY,swipeThresholdPx) are non-volatile and accessed/modified from multiple threads (UI callbacks and setter methods) without synchronization.- JSON escaping bug (line 188): Error responses construct JSON via string concatenation without escaping
getMessage(), which can break JSON if the exception message contains quotes or special characters.- Incomplete
escapeJsString()(line 483–487): Missing escape for double quotes ("), which are required for valid JSON string literals.Have the example reference or copy from the build assets template at build time rather than maintaining two separate copies.
v3/pkg/application/application_android.go (1)
853-886:⚠️ Potential issue | 🔴 CriticalMemory leak:
C.CString()allocations are never freed.In
nativeHandleMessage(lines 869, 874) andnativeGetAssetMimeType(line 885),C.CString(...)allocates heap memory viamalloc, but the returned pointer is passed directly intoC.createJString()and never freed.NewStringUTFcopies the data into a Java string, so the C-side allocation leaks on every call.This is particularly concerning for
nativeHandleMessagewhich is called on every runtime message from the frontend.🐛 Proposed fix: free the C strings after creating the Java string
if app == nil { errorResponse := `{"error":"App not initialized"}` - return C.createJString(env, C.CString(errorResponse)) + cErr := C.CString(errorResponse) + defer C.free(unsafe.Pointer(cErr)) + return C.createJString(env, cErr) } // Parse and handle the message response := handleMessageForAndroid(app, goMessage) - return C.createJString(env, C.CString(response)) + cResp := C.CString(response) + defer C.free(unsafe.Pointer(cResp)) + return C.createJString(env, cResp) } //export Java_com_wails_app_WailsBridge_nativeGetAssetMimeType func Java_com_wails_app_WailsBridge_nativeGetAssetMimeType(env *C.JNIEnv, obj C.jobject, jpath C.jstring) C.jstring { cPath := C.jstringToC(env, jpath) defer C.releaseJString(env, jpath, cPath) goPath := C.GoString(cPath) mimeType := getMimeTypeForPath(goPath) - return C.createJString(env, C.CString(mimeType)) + cMime := C.CString(mimeType) + defer C.free(unsafe.Pointer(cMime)) + return C.createJString(env, cMime) }
🤖 Fix all issues with AI agents
In
`@v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsJSBridge.java`:
- Around line 119-132: openURL in WailsJSBridge currently passes any URI to
Intent which allows risky schemes like intent:// and file://; update openURL to
parse the Uri, extract and lowercase the scheme (Uri.getScheme()), and
explicitly deny dangerous schemes (at minimum "intent" and "file") by logging a
warning (using TAG) and returning early; keep existing null/empty checks and
exception handling, and reference webView.getContext() and Intent.ACTION_VIEW
unchanged for allowed schemes.
In
`@v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java`:
- Around line 428-437: The tab bar is shown when nativeTabsEnabled is true even
if there are no titles, producing an empty visible BottomNavigationView; update
the logic in WailsBridge (around where nativeTabsEnabled, nativeTabTitles,
DEFAULT_NATIVE_TAB_TITLES and tabs are used) to determine titles first (apply
the fallback to DEFAULT_NATIVE_TAB_TITLES), then set shouldShow to true only if
nativeTabsEnabled && titles is non-empty, and if not shouldShow call
tabs.setVisibility(View.GONE) and return so the bar is hidden when there are no
items to display.
- Around line 47-60: The listed mutable fields in WailsBridge (e.g.,
scrollEnabled, bounceEnabled, scrollIndicatorsEnabled,
backForwardGesturesEnabled, linkPreviewEnabled, nativeTabsEnabled,
customUserAgent, touchStartX, touchStartY, swipeThresholdPx, touchListener) must
be made thread-safe: mark these primitive/reference fields as volatile so writes
from JNI/Go threads are visible to Runnables posted to webView; for
nativeTabTitles (currently an ArrayList) replace it with a thread-safe
collection (e.g., CopyOnWriteArrayList) or guard all accesses with synchronized
blocks to avoid concurrent-modification/staleness; keep blockLongClickListener
final. Locate and update the fields declared in class WailsBridge and adjust any
setter/getter usage accordingly.
In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/android.ts:
- Around line 32-42: The Device.Info TypeScript interface declared in the Device
namespace is missing fields returned by the Java bridge; update the Device.Info
interface (the exported interface named Info) to include manufacturer, brand,
device, product (all strings) and sdkInt (number) so it matches the JSON
returned by getDeviceInfoJson(), leaving the existing platform, model and
version fields as strings; keep the existing Device.Info() function that calls
DeviceInfo unchanged.
In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/runtime.ts:
- Around line 63-83: The function parseAndroidInvokeResponse currently re-throws
any Error caught (including SyntaxError from JSON.parse) which prevents
returning raw non-JSON text; update the catch to only re-throw genuine
non-SyntaxError Errors (so the Error thrown when parsed.ok === false still
surfaces) and return responseText when the exception is a SyntaxError (or when a
non-Error is caught). Locate parseAndroidInvokeResponse and the try/catch around
JSON.parse and change the catch behavior to: if err is an Error and err.name !==
'SyntaxError' then throw err, otherwise return responseText.
In `@v3/pkg/application/application_android.go`:
- Around line 1003-1004: The call to processor.HandleRuntimeCallWithIDs
currently uses context.Background() which can block the calling Java thread;
change this to create a cancellable context with a timeout (use
context.WithTimeout) before calling HandleRuntimeCallWithIDs, pass that timed
context into HandleRuntimeCallWithIDs, and defer the cancel to ensure resources
are freed; choose a sensible timeout value for
nativeHandleMessage/marshalAndroidInvokeResponse flows and handle the timeout
error returned from HandleRuntimeCallWithIDs so marshalAndroidInvokeResponse
receives the appropriate error result.
- Around line 161-483: There is a lot of duplicated JNI boilerplate around
g_jvm/g_bridge/GetEnv/AttachCurrentThread/CallVoidMethod/ExceptionCheck/DetachCurrentThread
across functions like setNativeTabsEnabledOnBridge, setScrollEnabledOnBridge,
setBounceEnabledOnBridge, setScrollIndicatorsEnabledOnBridge,
setBackForwardGesturesEnabledOnBridge, setLinkPreviewEnabledOnBridge,
selectNativeTabIndexOnBridge, setNativeTabsItemsJsonOnBridge,
setCustomUserAgentOnBridge and getDeviceInfoJsonOnBridge; factor this into a
small set of shared helpers (e.g., callBridgeVoidBool(jmethodID, jboolean),
callBridgeVoidInt(jmethodID, jint), callBridgeVoidString(jmethodID, const char*)
and callBridgeGetString(jmethodID) or similar) that encapsulate the
GetEnv/Attach/Call/ExceptionCheck/Detach logic and handle jstring
creation/deletion and NULL jstring cases, then replace each wrapper body with a
one-line call to the appropriate helper passing the existing g_* method IDs
(g_setNativeTabsEnabledMethod, g_selectNativeTabIndexMethod,
g_setNativeTabsItemsMethod, g_setCustomUserAgentMethod,
g_getDeviceInfoJsonMethod, etc.) and preserve current behavior for jstring
conversion and returned strdup.
- Around line 1030-1034: The fallback constructs JSON using fmt.Sprintf with
marshalErr.Error(), which risks producing invalid JSON if the error contains
quotes or backslashes; in the function where json.Marshal(response) is attempted
(variables: payload, marshalErr and call json.Marshal), replace the raw
fmt.Sprintf fallback with a safe JSON construction—e.g., build a small struct or
map like {"ok":false,"error":<escaped string>} and marshal that (or use
strconv.Quote to escape marshalErr.Error())—so the error string is properly
escaped and the function returns valid JSON even when json.Marshal fails.
- Around line 77-87: The GetMethodID calls (e.g., g_setNativeTabsEnabledMethod,
g_setNativeTabsItemsMethod, g_getDeviceInfoJsonMethod, etc.) can leave a pending
JNI exception which makes subsequent JNI calls undefined; after each
(*env)->GetMethodID (or at minimum after the batch) call check
(*env)->ExceptionCheck(env) and if true call (*env)->ExceptionDescribe(env) or
log the error, then call (*env)->ExceptionClear(env) and abort/return
initialization failure so you don't cache invalid method IDs; ensure you still
call (*env)->DeleteLocalRef(env, bridgeClass) before returning and propagate an
error state so the caller knows initialization failed.
- Around line 462-476: CallObjectMethod's return (jInfo) is being used before
checking for Java exceptions which makes jInfo undefined if an exception
occurred; update the logic in the function around the CallObjectMethod call
(g_bridge/g_getDeviceInfoJsonMethod) so you call (*env)->ExceptionCheck(env)
immediately after CallObjectMethod and, if an exception exists, call
(*env)->ExceptionDescribe(env) and (*env)->ExceptionClear(env) and skip any use
of jInfo/jstringToC; only proceed to use jInfo, call jstringToC, strdup,
releaseJString and DeleteLocalRef when ExceptionCheck reports no exception and
jInfo is non-NULL.
In `@v3/pkg/application/messageprocessor_android.go`:
- Around line 116-118: The file defines androidDeviceInfo() which calls
androidDeviceInfoViaJNI(), but its build constraint is just "//go:build android"
while androidDeviceInfoViaJNI() implementations use "//go:build android && !cgo
&& !server", causing link errors when building with the server tag; update the
build constraint on messageprocessor_android.go to include "!server" (i.e.,
match the other definitions) so androidDeviceInfo and androidDeviceInfoViaJNI
are compiled under the same build conditions.
In `@v3/pkg/events/events_android.go`:
- Around line 13-21: The ActivityCreated constant in newAndroidEvents is set to
1259 which is outside the fixed-size listener array size MAX_EVENTS=100, causing
registerListener and hasListeners (which check event < MAX_EVENTS) to no-op; fix
by either assigning ActivityCreated a value < MAX_EVENTS (update
newAndroidEvents to use an ID within range) or increase MAX_EVENTS to a value
>1259, or replace the fixed array hasListener with a dynamic map (map[int]bool)
and update registerListener and hasListeners to use the map; adjust all
references to ActivityCreated, MAX_EVENTS, hasListener, registerListener, and
hasListeners accordingly.
🧹 Nitpick comments (11)
v3/pkg/application/application_android_nocgo.go (1)
61-67: Function name is slightly misleading for a no-cgo stub.
androidDeviceInfoViaJNIsuggests JNI usage, but this build (!cgo) can never call JNI. Consider naming itandroidDeviceInfoin both the stub and the cgo variant (the cgo file can document JNI internally), or add a brief comment clarifying this is a stub matching the cgo counterpart's signature.Minor nit — not blocking.
v3/internal/commands/build_assets/android/Taskfile.yml (2)
126-135: Consider usinggradlew.batdirectly on Windows instead of invoking through PowerShell.On Windows, Gradle projects include a
gradlew.batfile alongside the shell script. Callingcmd /c gradlew.bator justgradlew.batis more conventional and avoids a PowerShell dependency:case "$(uname -s)" in - MINGW*|MSYS*|CYGWIN*) gradlew="powershell ./gradlew" ;; + MINGW*|MSYS*|CYGWIN*) gradlew="cmd //c gradlew.bat" ;; *) gradlew="./gradlew" ;; esacThat said, the PowerShell approach works too — just noting the more typical Windows convention.
126-153: Duplicated gradlew detection block acrossassemble:apkandassemble:apk:release.The Windows-aware gradlew selection logic is copy-pasted in both tasks. Consider extracting it into a shared task or a shell function to reduce duplication and simplify future maintenance.
v3/pkg/application/application_options.go (1)
361-367:NativeTabItem.SystemImageis iOS-specific but the type is shared with Android.The
NativeTabItemstruct and itsNativeTabIconconstants (SF Symbols like"house","gear") are iOS-specific concepts. Android'sAndroidOptions.NativeTabsItemsreuses this type, but theSystemImagefield has no meaning on Android. Consider documenting this in the struct orAndroidOptionscomment to avoid confusion.v3/examples/android/frontend/main.js (1)
64-72: Payload normalization is brittle and couples the generic caller to specific argument shapes.The
androidJsSetfunction is supposed to be a generic caller forAndroid.<Group>.<Method>(args), but lines 64–71 contain hard-coded knowledge of specific argument field names (enabled,ua). Any new method with different argument shapes will silently pass the full object, creating inconsistent behavior. This also means adding a new Android method requires updating this generic caller.Consider either:
- Letting the Android runtime TypeScript API handle argument normalization internally (it already wraps args in
{ enabled }/{ ua }perandroid.ts), and always passargsas-is here.- Or documenting that
androidJsSetexpects a single primitive or a pre-normalized value.♻️ Proposed simplification — remove the special-casing
- let payload = args; - if (args && typeof args === 'object') { - if ('enabled' in args) { - payload = !!args.enabled; - } else if ('ua' in args) { - payload = args.ua; - } - } - await fn(payload); + await fn(args);The Android TS runtime (
android.ts) already sends structured payloads like{ enabled }and{ ua }to the Go side. If the intermediate JS caller also unwraps them, the Go handler would receive a bare boolean/string instead of the expected map—check which layer is responsible for unwrapping.v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts (2)
85-113: Auto-configured Android transport won't be restored aftersetTransport(null).
configureAndroidTransport()runs once at module load (line 113) and bails out ifcustomTransportis already set. If a consumer later callssetTransport(null)to reset, the Android bridge transport is not re-established. This may be intentional, but it could be surprising.Consider either re-running
configureAndroidTransport()insidesetTransportwhen the argument isnull, or documenting this behavior.
153-157:newRuntimeCallerreturn type is inferred — consider an explicit type annotation.The function returns a closure whose signature is
(method: number, args?: any) => Promise<any>, but this isn't declared. An explicit return type would improve IDE discoverability and catch accidental signature drift.v3/internal/runtime/desktop/@wailsio/runtime/src/android.ts (1)
76-79:Setshadows the globalSetconstructor (flagged by Biome).While scoped inside the
UserAgentnamespace, the function nameSetshadows the globalSet(the built-in collection). Consider renaming to avoid confusion and satisfy the linter.♻️ Proposed rename
export namespace UserAgent { - export function Set(ua: string): Promise<void> { + export function SetCustom(ua: string): Promise<void> { return call(UserAgentSet, { ua }); } }Note: This would require updating the caller in
main.js(androidJsSetwith'UserAgent.Set'→'UserAgent.SetCustom').v3/pkg/application/application_android.go (3)
1069-1071: Usestrings.HasSuffixinstead of a customendsWith.Go's standard library provides
strings.HasSuffixwhich is already imported (viastrings). The customendsWithis functionally equivalent but adds unnecessary code.
1111-1113: Redundant double castC.jboolean(boolToJNI(...)).
boolToJNIalready returnsC.jboolean(line 1186), so wrapping it inC.jboolean(...)at every call site is redundant. This pattern repeats across all boolean wrapper functions (lines 1112, 1129, 1133, 1137, 1141, 1145).♻️ Example fix
func androidSetNativeTabsEnabled(enabled bool) { - C.setNativeTabsEnabledOnBridge(C.jboolean(boolToJNI(enabled))) + C.setNativeTabsEnabledOnBridge(boolToJNI(enabled)) }
776-801: Fragile RLock/RUnlock pattern — lock released mid-iteration.The read-lock acquired at line 776 is released at line 790 inside the loop body (before
executeJavaScript), then the function returns. The fallthrough path at line 801 also unlocks. This works today but is error-prone: any future modification (e.g., adding code after the loop, or processing multiple windows) risks double-unlock or operating under a stale lock assumption.Consider extracting the window lookup into a short critical section that copies the reference, then operating on it outside the lock.
♻️ Safer pattern: find window under lock, then operate outside it
// Inject the runtime into the first window (with proper locking) app.windowsLock.RLock() - windowCount := len(app.windows) - androidLogf("info", "🤖 [JNI] nativeOnPageFinished: window count = %d", windowCount) - for id, win := range app.windows { - androidLogf("info", "🤖 [JNI] Found window ID: %d", id) - if win != nil { - androidLogf("info", "🤖 [JNI] Injecting runtime.Core() into window %d", id) - // Get the runtime core JavaScript - flags := map[string]any{} - if app.impl != nil { - flags = app.impl.GetFlags(app.options) - } - runtimeJS := runtime.Core(flags) - androidLogf("info", "🤖 [JNI] Runtime JS length: %d bytes", len(runtimeJS)) - app.windowsLock.RUnlock() - // IMPORTANT: We must bypass win.ExecJS because it queues if runtimeLoaded is false. - // On Android, we need to inject the runtime directly since the runtime hasn't been loaded yet. - // This is the bootstrap injection that enables the runtime to load. - androidLogf("info", "🤖 [JNI] Calling executeJavaScript directly (bypassing queue)") - executeJavaScript(runtimeJS) - // Emit event - app.Event.Emit("PageFinished", url) - return - } + var targetWin Window + for _, win := range app.windows { + if win != nil { + targetWin = win + break + } } app.windowsLock.RUnlock() + + if targetWin != nil { + androidLogf("info", "🤖 [JNI] Injecting runtime.Core() into window %d", targetWin.ID()) + flags := map[string]any{} + if app.impl != nil { + flags = app.impl.GetFlags(app.options) + } + runtimeJS := runtime.Core(flags) + androidLogf("info", "🤖 [JNI] Runtime JS length: %d bytes", len(runtimeJS)) + executeJavaScript(runtimeJS) + app.Event.Emit("PageFinished", url) + return + }
v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsJSBridge.java
Show resolved
Hide resolved
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java
Show resolved
Hide resolved
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
v3/pkg/application/application_android.go (2)
743-751:⚠️ Potential issue | 🔴 CriticalMemory leak:
C.CStringallocations are never freed.
C.CStringallocates C heap memory. On lines 745 and 750,C.CString(...)is passed directly toC.createJStringand the returned C pointer is never freed. Every call tonativeHandleMessageandnativeGetAssetMimeTypeleaks memory.🐛 Proposed fix
func Java_com_wails_app_WailsBridge_nativeHandleMessage(env *C.JNIEnv, obj C.jobject, jmessage C.jstring) C.jstring { cMessage := C.jstringToC(env, jmessage) defer C.releaseJString(env, jmessage, cMessage) goMessage := C.GoString(cMessage) androidLogf("debug", "🤖 [JNI] nativeHandleMessage: %s", goMessage) globalAppLock.RLock() app := globalApp globalAppLock.RUnlock() if app == nil { - errorResponse := `{"error":"App not initialized"}` - return C.createJString(env, C.CString(errorResponse)) + cErr := C.CString(`{"error":"App not initialized"}`) + defer C.free(unsafe.Pointer(cErr)) + return C.createJString(env, cErr) } // Parse and handle the message response := handleMessageForAndroid(app, goMessage) - return C.createJString(env, C.CString(response)) + cResp := C.CString(response) + defer C.free(unsafe.Pointer(cResp)) + return C.createJString(env, cResp) }Apply the same pattern to
nativeGetAssetMimeType:func Java_com_wails_app_WailsBridge_nativeGetAssetMimeType(env *C.JNIEnv, obj C.jobject, jpath C.jstring) C.jstring { cPath := C.jstringToC(env, jpath) defer C.releaseJString(env, jpath, cPath) goPath := C.GoString(cPath) mimeType := getMimeTypeForPath(goPath) - return C.createJString(env, C.CString(mimeType)) + cMime := C.CString(mimeType) + defer C.free(unsafe.Pointer(cMime)) + return C.createJString(env, cMime) }
502-504:⚠️ Potential issue | 🟠 Major
GetFlagsreturnsnilon Android, inconsistent with other platforms and causingruntime.Core(nil)to be called.The Android implementation of
GetFlagsreturnsnilat lines 502-504, while macOS, Linux, and Windows all return a map (empty or populated with values). At line 662-665 innativeOnPageFinished, the code initializesflagsas an empty map but then reassigns it to the result ofGetFlags, which means on Android it becomesnilandruntime.Core(nil)is executed rather thanruntime.Core({}). This divergence from other platforms may silently break runtime initialization features that depend on flags being a non-nil map (e.g., debug mode, version info). Android should returnoptions.Flags(with nil-check creating an empty map) to match the behavior of desktop platforms.v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java (1)
186-191:⚠️ Potential issue | 🟠 MajorJSON injection in error response —
e.getMessage()may contain unescaped quotes.Line 189 constructs JSON by string concatenation:
"{\"error\":\"" + e.getMessage() + "\"}". If the exception message contains"or\, the resulting JSON is malformed. Consider usingJSONObjectfor safe construction, consistent withgetDeviceInfoJson().🐛 Proposed fix
} catch (Exception e) { Log.e(TAG, "Error handling message", e); - return "{\"error\":\"" + e.getMessage() + "\"}"; + JSONObject err = new JSONObject(); + try { + err.put("error", e.getMessage()); + } catch (JSONException ignored) { + return "{\"error\":\"unknown error\"}"; + } + return err.toString(); }
🤖 Fix all issues with AI agents
In
`@v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsJSBridge.java`:
- Around line 121-142: In WailsJSBridge.openURL, update the denylist check that
currently blocks "intent" and "file" schemes to also block the "content" scheme
(i.e., treat "content" like "intent" and "file"), so when
normalizedScheme.equals("content") you Log.w(TAG, "openURL blocked for scheme: "
+ normalizedScheme) and return; keep the existing Uri parsing, Locale.ROOT
normalization, exception handling, and the Intent launch logic otherwise; this
prevents content:// URLs from being opened via
webView.getContext().startActivity.
In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/android.ts:
- Around line 81-85: The function UserAgent.Set shadows the global Set
constructor; rename the method to UserAgent.SetUserAgent and update all callers
to use SetUserAgent instead of Set (keep the existing call(UserAgentSet, { ua })
identifier unless the RPC symbol must also change), or alternatively add a small
deprecated alias export like export const Set = SetUserAgent after the rename to
preserve backwards compatibility; update references to the function name in the
codebase (search for UserAgent.Set) and run tests/lint to ensure the Biome
warning is resolved.
In `@v3/pkg/application/application_android.go`:
- Around line 1043-1069: The parsed JSON currently replaces the default map in
androidDeviceInfoViaJNI, so if the JNI JSON omits "platform" callers can lose
that key; change the function to keep the defaults in info and merge values from
the parsed map into info (i.e., unmarshal into a temporary map and for each key
in parsed set info[key]=parsed[key]) rather than returning parsed directly,
ensuring "platform" stays present and other parsed fields override defaults.
🧹 Nitpick comments (5)
v3/examples/android/build/android/app/src/main/java/com/wails/app/WailsJSBridge.java (1)
116-142: Good implementation addressing the previous scheme-denylist feedback.The null/empty guard,
intent://andfile://scheme blocking,FLAG_ACTIVITY_NEW_TASK, and broad exception handling all look correct.One minor consideration:
content://andjavascript:schemes are also potentially risky on Android. For an example app this is likely fine, but if this bridge code is intended to be reused as scaffolding for real apps, extending the denylist would be prudent.- if ("intent".equals(normalizedScheme) || "file".equals(normalizedScheme)) { + if ("intent".equals(normalizedScheme) || "file".equals(normalizedScheme) + || "content".equals(normalizedScheme) || "javascript".equals(normalizedScheme)) {v3/internal/runtime/desktop/@wailsio/runtime/src/android.ts (1)
15-24: Method IDs must stay in sync with Go constants — consider documenting this.These IDs (0–8) must exactly match the constants in
messageprocessor_android.go. There's no compile-time check, so any drift would cause silent runtime failures. A comment linking to the Go file would help future maintainers.v3/pkg/application/application_android.go (2)
652-677: Lock release inside loop iteration is fragile — early return bypasses outerRUnlock.The
windowsLock.RLock()on line 652 is released inside the loop body (line 666) beforereturnon line 674. If the loop finds a window, it unlocks and returns correctly. But this pattern couples lock management with loop control flow. If someone later adds acontinueor modifies the loop, the lock could be double-released or leaked.Consider restructuring to capture the target window under the lock, release the lock, then operate on the captured window outside the loop.
♻️ Proposed refactor
- app.windowsLock.RLock() - windowCount := len(app.windows) - androidLogf("info", "🤖 [JNI] nativeOnPageFinished: window count = %d", windowCount) - for id, win := range app.windows { - androidLogf("info", "🤖 [JNI] Found window ID: %d", id) - if win != nil { - androidLogf("info", "🤖 [JNI] Injecting runtime.Core() into window %d", id) - flags := map[string]any{} - if app.impl != nil { - flags = app.impl.GetFlags(app.options) - } - runtimeJS := runtime.Core(flags) - androidLogf("info", "🤖 [JNI] Runtime JS length: %d bytes", len(runtimeJS)) - app.windowsLock.RUnlock() - androidLogf("info", "🤖 [JNI] Calling executeJavaScript directly (bypassing queue)") - executeJavaScript(runtimeJS) - app.Event.Emit("PageFinished", url) - return - } - } - app.windowsLock.RUnlock() + app.windowsLock.RLock() + windowCount := len(app.windows) + androidLogf("info", "🤖 [JNI] nativeOnPageFinished: window count = %d", windowCount) + var targetWinID uint + var found bool + for id, win := range app.windows { + androidLogf("info", "🤖 [JNI] Found window ID: %d", id) + if win != nil { + targetWinID = id + found = true + break + } + } + app.windowsLock.RUnlock() + + if found { + androidLogf("info", "🤖 [JNI] Injecting runtime.Core() into window %d", targetWinID) + flags := map[string]any{} + if app.impl != nil { + flags = app.impl.GetFlags(app.options) + } + runtimeJS := runtime.Core(flags) + androidLogf("info", "🤖 [JNI] Runtime JS length: %d bytes", len(runtimeJS)) + androidLogf("info", "🤖 [JNI] Calling executeJavaScript directly (bypassing queue)") + executeJavaScript(runtimeJS) + } else { + androidLogf("warn", "🤖 [JNI] nativeOnPageFinished: no windows found to inject runtime") + } + + app.Event.Emit("PageFinished", url)
922-956: Custom MIME type detection — consider usingmime.TypeByExtensionfrom stdlib.The hand-rolled
getMimeTypeForPathmisses common types (e.g.,.webp,.mp4,.xml,.txt) and the customendsWithfunction reimplementsstrings.HasSuffix. Go'smimepackage orfilepath.Ext+mime.TypeByExtensionwould be more comprehensive and maintainable.♻️ Proposed refactor
+import ( + "mime" + "path/filepath" +) + func getMimeTypeForPath(path string) string { - switch { - case endsWith(path, ".html"), endsWith(path, ".htm"): - return "text/html" - // ... all cases ... - default: - return "application/octet-stream" - } + ext := filepath.Ext(path) + if mimeType := mime.TypeByExtension(ext); mimeType != "" { + return mimeType + } + return "application/octet-stream" }v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java (1)
375-376: Non-atomicclear()+addAll()onCopyOnWriteArrayList.Between
nativeTabTitles.clear()(line 375) andnativeTabTitles.addAll(titles)(line 376), a concurrent reader could observe an empty list. In practice this is safe becauseapplyNativeTabs()reads the list inside arunOnUiThreadrunnable that will execute after this method completes, and CopyOnWriteArrayList's snapshot semantics ensure iteration consistency. Still, a single atomic replacement would be cleaner:♻️ Optional: atomic replacement
- nativeTabTitles.clear(); - nativeTabTitles.addAll(titles); + // Atomic replacement: readers see either old or new state, never empty + synchronized (nativeTabTitles) { + nativeTabTitles.clear(); + nativeTabTitles.addAll(titles); + }Or since CopyOnWriteArrayList doesn't have a
replaceAllfor content, use an intermediate approach. The current code is acceptable given UI-thread consumption.
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsJSBridge.java
Show resolved
Hide resolved
- Updated WailsJSBridge to block "content" scheme in openURL method. - Renamed UserAgent.Set function to SetUserAgent for clarity and added an alias for backward compatibility. - Modified error handling in parseAndroidInvokeResponse to ignore SyntaxError exceptions. - Enhanced androidDeviceInfoViaJNI to merge parsed device info into the existing info map.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v3/pkg/application/application_android.go (2)
729-751:⚠️ Potential issue | 🔴 CriticalMemory leak:
C.CStringallocations are never freed.
C.CStringallocates viamallocand the caller must free it. On lines 745 and 750, the C strings are passed toC.createJString(which copies viaNewStringUTF) but the originals are never freed. This leaks memory on every message from JavaScript.🐛 Proposed fix
if app == nil { errorResponse := `{"error":"App not initialized"}` - return C.createJString(env, C.CString(errorResponse)) + cError := C.CString(errorResponse) + defer C.free(unsafe.Pointer(cError)) + return C.createJString(env, cError) } // Parse and handle the message response := handleMessageForAndroid(app, goMessage) - return C.createJString(env, C.CString(response)) + cResponse := C.CString(response) + defer C.free(unsafe.Pointer(cResponse)) + return C.createJString(env, cResponse)
753-762:⚠️ Potential issue | 🔴 CriticalSame
C.CStringmemory leak innativeGetAssetMimeType.Line 761:
C.CString(mimeType)is never freed.🐛 Proposed fix
goPath := C.GoString(cPath) mimeType := getMimeTypeForPath(goPath) - return C.createJString(env, C.CString(mimeType)) + cMime := C.CString(mimeType) + defer C.free(unsafe.Pointer(cMime)) + return C.createJString(env, cMime)
🤖 Fix all issues with AI agents
In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/android.ts:
- Around line 81-87: The module-level export "Set" reintroduces shadowing of the
global Set constructor; remove the alias export or rename it to a
non-conflicting identifier (e.g., SetUserAgentAlias or SetUserAgentExport) so
callers use UserAgent.SetUserAgent or the new alias instead; update any local
references to the module-level "Set" export accordingly and keep the exported
UserAgent.SetUserAgent function unchanged.
🧹 Nitpick comments (4)
v3/internal/assetserver/bundledassets/runtime.js (1)
1-1: This is a minified build artifact — review the TypeScript source files instead.This file is generated from
v3/internal/runtime/desktop/@wailsio/runtime/src/(particularlyruntime.ts). Meaningful review should target the source TypeScript files. The static analysis warnings (sparse arrays, redeclared variables, class methodthen, use-before-declaration) are expected artifacts of minification and can be ignored.Consider adding this path to
.gitattributeswithlinguist-generated=trueso reviewers and automated tools skip it automatically:v3/internal/assetserver/bundledassets/runtime.js linguist-generated=truev3/pkg/application/application_android.go (3)
266-269: Empty string treated as NULL incallBridgeVoidString.
strlen(value) > 0on line 267 means an empty string""results in a NULLjstringbeing passed to the Java method, which is semantically different from passing an empty Java string. Current callers (e.g.,setCustomUserAgentOnBridge) seem to handle this, but it's a subtle contract worth documenting or handling explicitly.
922-956: Consider usingmime.TypeByExtensionfrom the standard library.The manual MIME mapping and custom
endsWithhelper duplicate functionality available in Go'smimepackage.mime.TypeByExtensionhandles a broader set of types and is maintained by the Go team. The customendsWithcould also be replaced withstrings.HasSuffix.♻️ Sketch
+import "mime" + func getMimeTypeForPath(path string) string { - switch { - case endsWith(path, ".html"), endsWith(path, ".htm"): - return "text/html" - // ... all cases ... - default: - return "application/octet-stream" + ext := filepath.Ext(path) + if mimeType := mime.TypeByExtension(ext); mimeType != "" { + return mimeType } + return "application/octet-stream" }
651-677: Fragile lock release pattern inside the loop.
windowsLock.RUnlock()is called inside the loop (line 666) beforeexecuteJavaScriptandreturn. If future edits remove thereturnor add acontinue, the lock will be released prematurely while iteration continues. Consider restructuring to unlock in a single place after the loop, storing the needed data while holding the lock.♻️ Sketch: extract JS string under lock, execute after
app.windowsLock.RLock() - windowCount := len(app.windows) - androidLogf("info", "🤖 [JNI] nativeOnPageFinished: window count = %d", windowCount) - for id, win := range app.windows { - androidLogf("info", "🤖 [JNI] Found window ID: %d", id) - if win != nil { - androidLogf("info", "🤖 [JNI] Injecting runtime.Core() into window %d", id) - flags := map[string]any{} - if app.impl != nil { - flags = app.impl.GetFlags(app.options) - } - runtimeJS := runtime.Core(flags) - androidLogf("info", "🤖 [JNI] Runtime JS length: %d bytes", len(runtimeJS)) - app.windowsLock.RUnlock() - androidLogf("info", "🤖 [JNI] Calling executeJavaScript directly (bypassing queue)") - executeJavaScript(runtimeJS) - app.Event.Emit("PageFinished", url) - return - } - } + var runtimeJS string + for id, win := range app.windows { + if win != nil { + flags := map[string]any{} + if app.impl != nil { + flags = app.impl.GetFlags(app.options) + } + runtimeJS = runtime.Core(flags) + break + } + } app.windowsLock.RUnlock() + + if runtimeJS != "" { + executeJavaScript(runtimeJS) + } else { + androidLogf("warn", "🤖 [JNI] nativeOnPageFinished: no windows found to inject runtime") + } + app.Event.Emit("PageFinished", url)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to make the v3 Android example workable from a Windows development environment, while also extending the Android runtime bridge (Go/JNI/JS) with additional Android-specific runtime APIs (scrolling, navigation gestures, link previews, custom UA, device info, toast, haptics) and a native bottom tab bar.
Changes:
- Added Android runtime object support to the JS runtime (object ID 12) and introduced
Runtime.AndroidAPIs. - Extended the Android Go/JNI bridge and Java bridge to support device info, WebView settings toggles, URL opening, and native bottom navigation tabs.
- Updated Android Taskfiles to better support Windows/MSYS-style environments.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/pkg/events/known_events.go | Registers a new Android event name (android:ActivityCreated) as known. |
| v3/pkg/events/events_android.go | Adds an Android event type container and listener tracking map. |
| v3/pkg/events/events.go | Maps Android event ID 1259 to JS event name. |
| v3/pkg/application/messageprocessor_android.go | Adds Android runtime method IDs and handlers (scroll, gestures, links, UA, device info via JNI). |
| v3/pkg/application/application_options.go | Adds Android native tabs options; formatting cleanup in iOS options section. |
| v3/pkg/application/application_android_nocgo.go | Adds JNI device-info stub + native tabs warning when cgo is unavailable. |
| v3/pkg/application/application_android.go | Adds JNI method caching + Android runtime invoke routing + native tabs and WebView control calls. |
| v3/pkg/application/android_runtime_stub.go | Adds exported no-op Android runtime setters for non-Android/non-cgo/server builds. |
| v3/pkg/application/android_runtime_android.go | Adds exported Android runtime setters for android+cgo builds. |
| v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts | Adds Android object ID and auto-configures an Android window.wails.invoke transport. |
| v3/internal/runtime/desktop/@wailsio/runtime/src/index.ts | Exports the new Android runtime module. |
| v3/internal/runtime/desktop/@wailsio/runtime/src/browser.ts | Uses Android bridge wails.openURL() when present. |
| v3/internal/runtime/desktop/@wailsio/runtime/src/android.ts | New Runtime.Android API surface. |
| v3/internal/commands/build_assets/android/app/... | Adds BottomNavigationView to layout + Java bridge additions (tabs/webview settings/device info/openURL). |
| v3/internal/commands/build_assets/android/Taskfile.yml | Attempts to make Gradle invocation work on Windows shells. |
| v3/internal/assetserver/bundledassets/runtime.js | Updates bundled runtime JS to include Android runtime support. |
| v3/examples/android/... | Updates example to enable Android native tabs + new frontend calls + Windows-aware Gradle invocation. |
| v3/examples/android/android_runtime_events_*.go | Adds Android-only Go event handlers to control Android WebView settings from the example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java
Outdated
Show resolved
Hide resolved
v3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.java
Outdated
Show resolved
Hide resolved
…prove message processing
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 (1)
v3/pkg/application/application_android.go (1)
744-751:⚠️ Potential issue | 🟠 MajorFree
C.CStringbuffers before returningjstring.
C.CStringallocates memory that must be freed;createJStringcalls JNINewStringUTFwhich does not take ownership of the C string. ThenativeHandleMessageandnativeGetAssetMimeTypefunctions leak memory on each call.Suggested fix
- errorResponse := `{"error":"App not initialized"}` - return C.createJString(env, C.CString(errorResponse)) + errorResponse := `{"error":"App not initialized"}` + cErr := C.CString(errorResponse) + defer C.free(unsafe.Pointer(cErr)) + return C.createJString(env, cErr)- response := handleMessageForAndroid(app, goMessage) - return C.createJString(env, C.CString(response)) + response := handleMessageForAndroid(app, goMessage) + cResp := C.CString(response) + defer C.free(unsafe.Pointer(cResp)) + return C.createJString(env, cResp)- mimeType := getMimeTypeForPath(goPath) - return C.createJString(env, C.CString(mimeType)) + mimeType := getMimeTypeForPath(goPath) + cMime := C.CString(mimeType) + defer C.free(unsafe.Pointer(cMime)) + return C.createJString(env, cMime)Lines: 745, 750, 761
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/application_android.go` around lines 744 - 751, The nativeHandleMessage and nativeGetAssetMimeType paths call C.CString to build strings passed into createJString but never free those C allocations; free the C strings after calling createJString (or use defer to call C.free(unsafe.Pointer(cstr))) so the C memory is released; locate calls to C.CString in nativeHandleMessage/nativeGetAssetMimeType and ensure each corresponding C.free is invoked after createJString returns, referencing createJString, C.CString and C.free to eliminate the leak.
♻️ Duplicate comments (2)
v3/pkg/application/application_android.go (1)
81-111:⚠️ Potential issue | 🟠 MajorCheck JNI exceptions after each
GetMethodIDto avoid pending-exception fallout.With a pending JNI exception, subsequent
GetMethodIDcalls may return invalid IDs. Consider checking/aborting per call instead of only once at the end. This is the same concern raised earlier—please confirm it’s fully resolved in the current code.🧩 Possible pattern to hard‑fail per call
+#define GET_METHOD_OR_FAIL(out, name, sig) \ + do { \ + out = (*env)->GetMethodID(env, bridgeClass, name, sig); \ + if ((*env)->ExceptionCheck(env) || out == NULL) { \ + goto cache_fail; \ + } \ + } while (0) + - g_executeJsMethod = (*env)->GetMethodID(env, bridgeClass, "executeJavaScript", "(Ljava/lang/String;)V"); - g_setNativeTabsEnabledMethod = (*env)->GetMethodID(env, bridgeClass, "setNativeTabsEnabled", "(Z)V"); + GET_METHOD_OR_FAIL(g_executeJsMethod, "executeJavaScript", "(Ljava/lang/String;)V"); + GET_METHOD_OR_FAIL(g_setNativeTabsEnabledMethod, "setNativeTabsEnabled", "(Z)V"); // ... repeat for the rest + +cache_fail: + (*env)->ExceptionDescribe(env); + (*env)->ExceptionClear(env); + // cleanup + return JNI_FALSE#!/bin/bash # Inspect JNI method ID caching and exception checks rg -n "GetMethodID|ExceptionCheck" v3/pkg/application/application_android.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/application_android.go` around lines 81 - 111, The current code only calls (*env)->ExceptionCheck(env) once after all GetMethodID calls, which can leave earlier JNI exceptions pending and cause later GetMethodID to return invalid IDs; update the initialization sequence (the GetMethodID calls that set g_executeJsMethod, g_setNativeTabsEnabledMethod, g_setNativeTabsItemsMethod, g_selectNativeTabIndexMethod, g_setScrollEnabledMethod, g_setBounceEnabledMethod, g_setScrollIndicatorsEnabledMethod, g_setBackForwardGesturesEnabledMethod, g_setLinkPreviewEnabledMethod, g_setCustomUserAgentMethod, g_getDeviceInfoJsonMethod) to check (*env)->ExceptionCheck(env) immediately after each GetMethodID call and, on failure, call (*env)->ExceptionDescribe(env), (*env)->ExceptionClear(env), perform the existing cleanup (DeleteLocalRef on bridgeClass, DeleteGlobalRef on g_bridge, set g_bridge and the affected g_* method IDs to NULL) and return JNI_FALSE so we abort fast on the first JNI exception instead of continuing with potentially invalid IDs.v3/examples/android/build/android/Taskfile.yml (1)
145-151: Duplicated Windows detection block — consolidate by inlining thecasedispatch.Lines 145–151 are a verbatim copy of lines 129–135, including the same reliability concern with
powershell ./gradlew.bat. The proposed fix above (inlining thecasedispatch without an intermediate variable) applies equally here and removes the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/examples/android/build/android/Taskfile.yml` around lines 145 - 151, Duplicate Windows-detection and gradlew invocation blocks should be consolidated by removing the repeated "case ... gradlew=..." assignment and instead inlining the case dispatch at the call site so you run either "powershell ./gradlew.bat" or "./gradlew" directly; update the second occurrence (the block that defines gradlew and then calls "$gradlew assembleRelease") to the single-line case-dispatch execution pattern (match MINGW*|MSYS*|CYGWIN* to run powershell ./gradlew.bat, otherwise ./gradlew) and remove the intermediate gradlew variable declaration to eliminate duplication and address the same powershell reliability concern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@v3/pkg/application/application_android.go`:
- Around line 744-751: The nativeHandleMessage and nativeGetAssetMimeType paths
call C.CString to build strings passed into createJString but never free those C
allocations; free the C strings after calling createJString (or use defer to
call C.free(unsafe.Pointer(cstr))) so the C memory is released; locate calls to
C.CString in nativeHandleMessage/nativeGetAssetMimeType and ensure each
corresponding C.free is invoked after createJString returns, referencing
createJString, C.CString and C.free to eliminate the leak.
---
Duplicate comments:
In `@v3/examples/android/build/android/Taskfile.yml`:
- Around line 145-151: Duplicate Windows-detection and gradlew invocation blocks
should be consolidated by removing the repeated "case ... gradlew=..."
assignment and instead inlining the case dispatch at the call site so you run
either "powershell ./gradlew.bat" or "./gradlew" directly; update the second
occurrence (the block that defines gradlew and then calls "$gradlew
assembleRelease") to the single-line case-dispatch execution pattern (match
MINGW*|MSYS*|CYGWIN* to run powershell ./gradlew.bat, otherwise ./gradlew) and
remove the intermediate gradlew variable declaration to eliminate duplication
and address the same powershell reliability concern.
In `@v3/pkg/application/application_android.go`:
- Around line 81-111: The current code only calls (*env)->ExceptionCheck(env)
once after all GetMethodID calls, which can leave earlier JNI exceptions pending
and cause later GetMethodID to return invalid IDs; update the initialization
sequence (the GetMethodID calls that set g_executeJsMethod,
g_setNativeTabsEnabledMethod, g_setNativeTabsItemsMethod,
g_selectNativeTabIndexMethod, g_setScrollEnabledMethod,
g_setBounceEnabledMethod, g_setScrollIndicatorsEnabledMethod,
g_setBackForwardGesturesEnabledMethod, g_setLinkPreviewEnabledMethod,
g_setCustomUserAgentMethod, g_getDeviceInfoJsonMethod) to check
(*env)->ExceptionCheck(env) immediately after each GetMethodID call and, on
failure, call (*env)->ExceptionDescribe(env), (*env)->ExceptionClear(env),
perform the existing cleanup (DeleteLocalRef on bridgeClass, DeleteGlobalRef on
g_bridge, set g_bridge and the affected g_* method IDs to NULL) and return
JNI_FALSE so we abort fast on the first JNI exception instead of continuing with
potentially invalid IDs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
v3/examples/android/build/android/Taskfile.ymlv3/examples/android/build/android/app/src/main/java/com/wails/app/WailsBridge.javav3/internal/commands/build_assets/android/Taskfile.ymlv3/internal/commands/build_assets/android/app/src/main/java/com/wails/app/WailsBridge.javav3/pkg/application/application.gov3/pkg/application/application_android.gov3/pkg/application/messageprocessor_android.go
🚧 Files skipped from review as they are similar to previous changes (1)
- v3/internal/commands/build_assets/android/Taskfile.yml
Description
try make v3 android example working in windows develop env, all code generated by ai.
Fixes # (issue)
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
Improvements
Chores