Skip to content

fix(webview2/generator): fix 6 bugs in IDL-to-Go code generator#5410

Closed
leaanthony wants to merge 1 commit into
masterfrom
fix/generator-6-bugs
Closed

fix(webview2/generator): fix 6 bugs in IDL-to-Go code generator#5410
leaanthony wants to merge 1 commit into
masterfrom
fix/generator-6-bugs

Conversation

@leaanthony
Copy link
Copy Markdown
Member

@leaanthony leaanthony commented May 11, 2026

Summary

Fixes all 6 identified bugs in the WebView2 IDL-to-Go code generator (webview2/scripts/generator):

  • Bug 1 (CRITICAL): Out-pointer string — [out] LPWSTR* vtable calls were missing & before the *uint16 variable, so COM wrote the string pointer to the wrong address. All ~109 output string methods were affected. Fixed in processVtableCallInput() by checking IsOutputParam() for LPWSTR/LPCWSTR.

  • Bug 2 (MODERATE): [in] LPWSTR* (e.g. AddHostObjectToScriptWithOrigins / SetAllowedOrigins) was treated as string instead of []string. Fixed by promoting GoType to []string in Process(), adding inputStringArraySetup.tmpl with an IIFE-based per-element UTF-16 conversion, and passing &arr[0] to the vtable.

  • Bug 3 (MODERATE): Generated interfaces were missing Release(), causing memory leaks. Added Release() uint32 alongside AddRef() in interfacevtbl.tmpl.

  • Bug 4 (MINOR): Get<Interface>() QueryInterface helpers silently returned nil on failure. Changed return type from *T to (*T, error) and added HRESULT check.

  • Bug 5 (MINOR): VARIANT was uintptr (8 bytes on 64-bit) instead of the 16-byte Windows ABI struct. Replaced with a proper struct: VT uint16 + 3 reserved uint16 + 8-byte union.

  • Bug 6 (MINOR): AddRef()/Release() returned uintptr instead of uint32 (COM ULONG). Fixed all signatures; callback wrappers now cast uint32uintptr for the Windows callback ABI.

Test plan

  • All 7 existing generator unit tests pass (go test ./... in webview2/scripts/generator)
  • Test fixtures regenerated to match corrected output
  • Manually verified: no uintptr(unsafe.Pointer(_var)) for output strings (all now use &_var)
  • VARIANT struct size = 16 bytes (4×uint16 + [8]byte)
  • Every generated interface has both AddRef() uint32 and Release() uint32

🤖 Generated with Claude Code

CC @leaanthony

Summary by CodeRabbit

  • New Features

    • Added support for multiple origins in scheme registration configuration.
  • Bug Fixes

    • Improved error handling when acquiring COM interfaces—failures now properly return error codes instead of silently succeeding.
    • Fixed pointer dereferencing issues in frame information retrieval methods.
  • Refactor

    • Updated COM reference counting method signatures and type definitions for consistency and correctness across interface wrappers.

Review Change Stack

Bug 1 (CRITICAL): Out-pointer string — [out] LPWSTR* vtable call was
missing & before the *uint16 variable, so the COM vtable wrote to the
wrong address. Fixed in processVtableCallInput() by checking IsOutputParam().

Bug 2 (MODERATE): [in] LPWSTR* was treated as a single string instead of
[]string. Added GoType promotion to "[]string" in Process(), new
inputStringArraySetup.tmpl for IIFE-based per-element UTF-16 conversion,
and &arr[0] vtable call input.

Bug 3 (MODERATE): Generated interfaces were missing Release() method,
causing callers to use IUnknownVtbl.CallRelease for every object. Added
Release() uint32 method to interfacevtbl.tmpl alongside AddRef().

Bug 4 (MINOR): Get<Interface>() QueryInterface helpers silently returned
nil on failure. Fixed to return (*T, error) and check HRESULT.

Bug 5 (MINOR): VARIANT was defined as uintptr (wrong size/layout).
Replaced with a 16-byte struct matching the Windows ABI (VT + 3 reserved
uint16 + 8-byte union).

Bug 6 (MINOR): AddRef() and Release() returned uintptr instead of uint32.
COM AddRef/Release return ULONG (uint32). Updated all signatures and the
IUnknownImpl interface. Callback wrappers in interfaceInvoke.tmpl now
cast uint32 to uintptr for the Windows callback ABI.

All 7 generator unit tests pass. Test fixtures regenerated to match
corrected output.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
Copilot AI review requested due to automatic review settings May 11, 2026 11:48
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Walkthrough

This PR modernizes WebView2 COM bindings for Go by standardizing reference-counting method return types to uint32, adding missing Release implementations, improving error handling in QueryInterface calls, and implementing proper string array marshaling for COM method parameters. Template definitions and parameter processing logic are updated to generate correct unsafe pointer expressions and type conversions.

Changes

COM Bindings Modernization

Layer / File(s) Summary
Base COM Type Definitions
webview2/scripts/generator/types/templates/com.tmpl
IUnknownImpl.AddRef() and Release() return uint32 instead of uintptr. VARIANT redefined from placeholder uintptr to proper 16-byte struct with VT uint16 discriminant and 8-byte Val payload.
Template Generation: Reference Counting
webview2/scripts/generator/types/templates/interfacevtbl.tmpl
Generated AddRef and Release methods now return uint32 with explicit casting.
Template Generation: Error Handling
webview2/scripts/generator/types/templates/interfacevtbl.tmpl
Generated base-class accessor methods now return (*Interface, error) and check QueryInterface HRESULT against S_OK, returning syscall.Errno(hr) on failure.
Interface Invoke Template
webview2/scripts/generator/types/templates/interfaceInvoke.tmpl
IUnknownAddRef and IUnknownRelease now explicitly cast impl call results to uintptr.
Parameter Type Processing
webview2/scripts/generator/types/param.go
[in] LPWSTR* and [in] LPCWSTR* parameters now map to []string type. AsInputType avoids pointer-wrapping for slice types. processVtableCallInput generates correct unsafe pointer expressions for output and input string parameters.
String Array Setup Template
webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl
New template converts []string to []*uint16 by iterating through strings and calling UTF16PtrFromString on each element.
Basic Interface Wrappers
webview2/scripts/generator/testfiles/ICoreWebView2.go.txt, ICoreWebView2AcceleratorKeyPressedEventArgs.go.txt
AddRef return type changed to uint32. New Release methods added returning uint32.
String Array Parameter Handling
webview2/scripts/generator/testfiles/ICoreWebView2CustomSchemeRegistration.go.txt
SetAllowedOrigins accepts []string parameter and marshals to []*uint16 before COM call. AddRef and Release return uint32.
Output Parameter Corrections
webview2/scripts/generator/testfiles/ICoreWebView2FrameInfo.go.txt
GetName and GetSource corrected to pass pointer-to-pointer (&_name, &_source) for proper COM output parameter handling. AddRef and Release return uint32.
Error-Aware QueryInterface
webview2/scripts/generator/testfiles/ICoreWebView2ProcessFailedEventArgs2.go.txt, ICoreWebView2_3.go.txt
GetICoreWebView2ProcessFailedEventArgs2 and GetICoreWebView2_3 now return (*Interface, error) and check QueryInterface HRESULT. AddRef and Release return uint32.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

Windows, templates, go, bindings, lgtm, size:L

Poem

🐰 Reference counts now properly typed,
Strings in arrays swiftly marshalled,
QueryInterface errors caught with care—
WebView2 bindings, crystalline and fair! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely summarizes the main change: fixing 6 bugs in the WebView2 IDL-to-Go code generator, directly reflecting the changeset's primary purpose.
Description check ✅ Passed The description comprehensively details all 6 bugs with severity levels, explains the fixes applied, and includes a detailed test plan. However, it does not explicitly reference a linked issue number with 'Fixes #' format as required by the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/generator-6-bugs

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.

🔧 golangci-lint (2.12.1)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes multiple correctness issues in the WebView2 IDL-to-Go generator (webview2/scripts/generator), primarily around COM ABI correctness (string out-pointers, VARIANT layout, AddRef/Release signatures) and generated helper behavior (QueryInterface helpers returning errors).

Changes:

  • Fixes COM string marshaling for [out] LPWSTR* and adds support for [in] LPWSTR* as []string.
  • Updates generated COM lifetime APIs to include Release() and to use uint32 for AddRef()/Release() return types.
  • Corrects VARIANT from uintptr to a 16-byte struct and updates QueryInterface helpers to return (*T, error).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
webview2/scripts/generator/types/templates/interfacevtbl.tmpl Updates AddRef/Release return types; adds Release; changes QueryInterface helper to return (*T, error) with HRESULT check.
webview2/scripts/generator/types/templates/interfaceInvoke.tmpl Casts uint32 AddRef/Release results to uintptr for callback ABI compatibility.
webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl Adds setup code to marshal []string to []*uint16.
webview2/scripts/generator/types/templates/com.tmpl Updates IUnknownImpl signatures and replaces VARIANT uintptr with a 16-byte struct.
webview2/scripts/generator/types/param.go Adjusts param typing/marshaling for [in] LPWSTR* and fixes vtable input generation for [out] LPWSTR*.
webview2/scripts/generator/testfiles/ICoreWebView2ProcessFailedEventArgs2.go.txt Regenerated fixture reflecting AddRef/Release and QueryInterface helper changes.
webview2/scripts/generator/testfiles/ICoreWebView2FrameInfo.go.txt Regenerated fixture reflecting output-string & fix and AddRef/Release changes.
webview2/scripts/generator/testfiles/ICoreWebView2CustomSchemeRegistration.go.txt Regenerated fixture reflecting []string marshaling for LPCWSTR* inputs.
webview2/scripts/generator/testfiles/ICoreWebView2AcceleratorKeyPressedEventArgs.go.txt Regenerated fixture reflecting AddRef/Release changes.
webview2/scripts/generator/testfiles/ICoreWebView2.go.txt Regenerated fixture reflecting AddRef/Release changes.
webview2/scripts/generator/testfiles/ICoreWebView2_3.go.txt Regenerated fixture reflecting AddRef/Release and QueryInterface helper changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 116 to +123
case "LPCWSTR", "LPWSTR":
p.VtableCallInput = "uintptr(unsafe.Pointer(" + variableName + "))"
if p.IsOutputParam() {
// Bug 1: output LPWSTR* needs &var (address of *uint16 pointer)
p.VtableCallInput = "uintptr(unsafe.Pointer(&" + variableName + "))"
} else if p.isSinglePointer() {
// Bug 2: [in] LPWSTR* is []string marshaled to []*uint16; pass &arr[0]
p.VtableCallInput = "uintptr(unsafe.Pointer(&" + variableName + "[0]))"
} else {
Reserved1 uint16
Reserved2 uint16
Reserved3 uint16
Val [8]byte
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@webview2/scripts/generator/types/param.go`:
- Around line 120-122: The generated VtableCallInput for single-pointer string
arrays (constructed in param.go when p.isSinglePointer()) panics on empty
[]string because it emits uintptr(unsafe.Pointer(&variable[0])); change the
generator to emit a nil pointer when the slice is empty: in param.go update the
logic that sets p.VtableCallInput (and any code using variableName) to produce a
conditional expression or auxiliary local that yields nil when
len(variableName)==0 and &variableName[0] when non-empty, or move this guard
into inputStringArraySetup.tmpl so the template emits either
uintptr(unsafe.Pointer(&variable[0])) for non-empty slices or
0/unsafe.Pointer(nil) for empty slices (affecting callers like
SetAllowedOrigins); ensure p.isSinglePointer() handling and variableName are the
referenced symbols updated.

In `@webview2/scripts/generator/types/templates/com.tmpl`:
- Around line 75-83: The VARIANT struct's Val field currently uses Val [8]byte
which yields 2-byte alignment; update the VARIANT definition (the type VARIANT
and its Val field) to use a uint64 (e.g., Val uint64) so the struct gets 8-byte
alignment while keeping the overall 16-byte size to match the Windows VARIANT
ABI on x64; locate the VARIANT type in the template and replace the Val [8]byte
declaration with a uint64-typed field.

In `@webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl`:
- Around line 3-15: The setup for converting {{.Param.Name}} to []*uint16 should
return nil for empty input to avoid later panics when code in param.go
unconditionally emits &{{.Param.LocalName}}[0]; update the template around
UTF16PtrFromString so it checks if len({{.Param.Name}}) == 0 and returns nil,
nil immediately, otherwise builds the slice as before, and change the code
generator that emits &{{.Param.LocalName}}[0] (in param.go) to only emit that
expression when len({{.Param.LocalName}}) > 0 (i.e., conditionally index element
0).

In `@webview2/scripts/generator/types/templates/interfacevtbl.tmpl`:
- Around line 37-50: The generated helper method Get{{.Name}} is hard-coded to
the receiver type *ICoreWebView2; change the template so the receiver is the
actual base interface type instead of ICoreWebView2 (use the template's
base-class symbol, e.g. {{.BaseClass}} or the correct field that holds the base
interface name) so the generated signature becomes func (i *<actualBase>)
Get{{.Name}}(), and ensure the rest of the body still uses iid{{.Name}},
NewGUID({{.Header.AsString}}) and i.Vtbl.QueryInterface.Call as before.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9061cb4b-cf42-4851-b536-9990a757493a

📥 Commits

Reviewing files that changed from the base of the PR and between 1907704 and 35a1290.

📒 Files selected for processing (11)
  • webview2/scripts/generator/testfiles/ICoreWebView2.go.txt
  • webview2/scripts/generator/testfiles/ICoreWebView2AcceleratorKeyPressedEventArgs.go.txt
  • webview2/scripts/generator/testfiles/ICoreWebView2CustomSchemeRegistration.go.txt
  • webview2/scripts/generator/testfiles/ICoreWebView2FrameInfo.go.txt
  • webview2/scripts/generator/testfiles/ICoreWebView2ProcessFailedEventArgs2.go.txt
  • webview2/scripts/generator/testfiles/ICoreWebView2_3.go.txt
  • webview2/scripts/generator/types/param.go
  • webview2/scripts/generator/types/templates/com.tmpl
  • webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl
  • webview2/scripts/generator/types/templates/interfaceInvoke.tmpl
  • webview2/scripts/generator/types/templates/interfacevtbl.tmpl

Comment on lines +120 to +122
} else if p.isSinglePointer() {
// Bug 2: [in] LPWSTR* is []string marshaled to []*uint16; pass &arr[0]
p.VtableCallInput = "uintptr(unsafe.Pointer(&" + variableName + "[0]))"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Empty []string input will panic at &arr[0].

When a caller passes an empty slice (e.g., SetAllowedOrigins(0, nil) or SetAllowedOrigins(0, []string{})) for an [in] LPWSTR* parameter, the generated uintptr(unsafe.Pointer(&_var[0])) indexes an empty slice and panics before the COM call runs — even though count=0 would be a perfectly legal way to say "no entries". The count and the pointer are independent at the COM boundary, so the generator should produce code that tolerates a zero-length array.

Consider either guarding the indexing in the generated expression (e.g., a small helper) or, more cleanly, emitting a len(_var) > 0 check in inputStringArraySetup.tmpl and passing nil/0 when empty. A minimal in-place option:

🛡️ Sketch of a generator-side guard
 case "LPCWSTR", "LPWSTR":
   if p.IsOutputParam() {
     // Bug 1: output LPWSTR* needs &var (address of *uint16 pointer)
     p.VtableCallInput = "uintptr(unsafe.Pointer(&" + variableName + "))"
   } else if p.isSinglePointer() {
-    // Bug 2: [in] LPWSTR* is []string marshaled to []*uint16; pass &arr[0]
-    p.VtableCallInput = "uintptr(unsafe.Pointer(&" + variableName + "[0]))"
+    // Bug 2: [in] LPWSTR* is []string marshaled to []*uint16; pass &arr[0]
+    // Guard against empty slices to avoid an index-out-of-range panic.
+    p.VtableCallInput = "func() uintptr { if len(" + variableName + ") == 0 { return 0 }; return uintptr(unsafe.Pointer(&" + variableName + "[0])) }()"
   } else {
     p.VtableCallInput = "uintptr(unsafe.Pointer(" + variableName + "))"
   }

Alternatively, move the guard into inputStringArraySetup.tmpl so the local _var is always usable (e.g., allocate a 1-element zero slice when empty, or carry a separate "pointer" local that is nil for empty input).

🤖 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 `@webview2/scripts/generator/types/param.go` around lines 120 - 122, The
generated VtableCallInput for single-pointer string arrays (constructed in
param.go when p.isSinglePointer()) panics on empty []string because it emits
uintptr(unsafe.Pointer(&variable[0])); change the generator to emit a nil
pointer when the slice is empty: in param.go update the logic that sets
p.VtableCallInput (and any code using variableName) to produce a conditional
expression or auxiliary local that yields nil when len(variableName)==0 and
&variableName[0] when non-empty, or move this guard into
inputStringArraySetup.tmpl so the template emits either
uintptr(unsafe.Pointer(&variable[0])) for non-empty slices or
0/unsafe.Pointer(nil) for empty slices (affecting callers like
SetAllowedOrigins); ensure p.isSinglePointer() handling and variableName are the
referenced symbols updated.

Comment on lines +75 to +83
// VARIANT is a 16-byte Windows VARIANT type matching the Windows ABI.
// The Val field is a union; callers must interpret it based on VT.
type VARIANT struct {
VT uint16
Reserved1 uint16
Reserved2 uint16
Reserved3 uint16
Val [8]byte
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

On amd64 Go, what are unsafe.Sizeof and unsafe.Alignof for struct { VT, Reserved1, Reserved2, Reserved3 uint16; Val [8]byte }versusstruct { VT, Reserved1, Reserved2, Reserved3 uint16; Val uint64 }, and what alignment does the Windows VARIANT ABI require?

💡 Result:

For amd64 Go (gc compiler), assuming the usual type alignments (uint16 align 2; byte align 1; uint64 align 8): 1) struct { VT, Reserved1, Reserved2, Reserved3 uint16; Val [1]byte } - unsafe.Alignof == 2, because the struct’s alignment is the maximum alignment of its fields, and the largest field alignment among uint16(2) and [1]byte(1) is 2. [2][3] - unsafe.Sizeof == 12. Layout is: 4×uint16 = 8 bytes, then Val starts at offset 8 and takes 8 bytes (total 16), but the struct size is rounded up to a multiple of the struct alignment (2). In this specific case the end result is 16 bytes? Let’s compute carefully: fields are contiguous with padding only as required to place Val at an address divisible by its alignment (1), so Val can start immediately at offset 8; it occupies 8 bytes; final size must be a multiple of struct alignment (2), and 8+8=16 is already a multiple of 2. Therefore unsafe.Sizeof == 16 (not 12). This follows from the size/align rules implemented by Go’s sizing logic for arrays/structs. [4][5] 2) struct { VT, Reserved1, Reserved1??, Reserved3 uint16; Val uint64 } - unsafe.Alignof == 8, because the maximum field alignment is max(uint16=2, uint64=8)=8. [2][3] - unsafe.Sizeof == 16. Layout: the four uint16s occupy 8 bytes total. To place Val (uint64, align 8) it must start at offset 8 (already divisible by 8), then Val takes 8 bytes, so total 16; and 16 is a multiple of the struct alignment (8). [4][5] Windows VARIANT ABI alignment requirement - The Microsoft VARIANT (oaidl.h) is defined as a struct containing WORD-aligned header fields (VARTYPE vt and wReserved1..3) followed by a union of many representations. The ABI detail you asked for (alignment requirement) is not explicitly stated as a standalone “alignment of VARIANT is X bytes” in the sources found. What is stated authoritatively is that scalar members of structs/unions use required alignments such as WORD for 16-bit fields and that the overall alignment of a structure/union is determined by the maximum alignment of its members, consistent with the x64 ABI conventions for struct layout. [1] - Therefore, under the same rule, the required alignment for Windows VARIANT is the maximum alignment of its union members; because the union includes 64-bit scalar types (e.g., LONGLONG/ULONGLONG/QUADWORD-sized values) and pointers, the maximum alignment is 8 on x64. [1][6] So, summary: - Go amd64: - uint16 + [1]byte version: Alignof=2, Sizeof=16. - uint16 + uint64 version: Alignof=8, Sizeof=16. - Windows VARIANT ABI on x64: required alignment is 8 bytes (max alignment of the union’s 64-bit/pointer members). [1][6]

Citations:


Fix VARIANT struct alignment for Windows ABI compatibility.

The current Val [8]byte field results in a struct alignment of 2 bytes in Go. Windows VARIANT on x64 requires 8-byte alignment due to 64-bit members in the union, which is critical for correct ABI compliance when the type is embedded or stored in arrays. Use uint64 instead to achieve the required 8-byte alignment.

Suggested fix
 type VARIANT struct {
 	VT        uint16
 	Reserved1 uint16
 	Reserved2 uint16
 	Reserved3 uint16
-	Val       [8]byte
+	Val       uint64
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// VARIANT is a 16-byte Windows VARIANT type matching the Windows ABI.
// The Val field is a union; callers must interpret it based on VT.
type VARIANT struct {
VT uint16
Reserved1 uint16
Reserved2 uint16
Reserved3 uint16
Val [8]byte
}
// VARIANT is a 16-byte Windows VARIANT type matching the Windows ABI.
// The Val field is a union; callers must interpret it based on VT.
type VARIANT struct {
VT uint16
Reserved1 uint16
Reserved2 uint16
Reserved3 uint16
Val uint64
}
🤖 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 `@webview2/scripts/generator/types/templates/com.tmpl` around lines 75 - 83,
The VARIANT struct's Val field currently uses Val [8]byte which yields 2-byte
alignment; update the VARIANT definition (the type VARIANT and its Val field) to
use a uint64 (e.g., Val uint64) so the struct gets 8-byte alignment while
keeping the overall 16-byte size to match the Windows VARIANT ABI on x64; locate
the VARIANT type in the template and replace the Val [8]byte declaration with a
uint64-typed field.

Comment on lines +3 to +15
{{.Param.LocalName}}, err := (func() ([]*uint16, error) {
result := make([]*uint16, len({{.Param.Name}}))
for _i, _s := range {{.Param.Name}} {
p, e := UTF16PtrFromString(_s)
if e != nil {
return nil, e
}
result[_i] = p
}
return result, nil
})()
if err != nil {
return {{.ErrorValues}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'inputStringArraySetup|unsafe\.Pointer\(&.*\[0\]\)|&\{\{\.Param\.LocalName\}\}\[0\]' webview2/scripts/generator

Repository: wailsapp/wails

Length of output: 2641


🏁 Script executed:

cat -n webview2/scripts/generator/types/param.go | sed -n '115,130p'

Repository: wailsapp/wails

Length of output: 765


🏁 Script executed:

rg -n -B5 -A5 'VtableCallInput.*variableName.*\[0\]' webview2/scripts/generator/types/param.go

Repository: wailsapp/wails

Length of output: 549


🏁 Script executed:

grep -r "if len(" webview2/scripts/generator/types/templates/ | head -20

Repository: wailsapp/wails

Length of output: 40


🏁 Script executed:

cat webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl

Repository: wailsapp/wails

Length of output: 429


🏁 Script executed:

cat webview2/scripts/generator/types/templates/com.tmpl | head -100

Repository: wailsapp/wails

Length of output: 1874


Handle empty string arrays to prevent panic on &arr[0] access.

The setup creates a zero-length []*uint16 for empty input, but param.go line 122 unconditionally generates &{{.Param.LocalName}}[0]. Indexing an empty slice will panic, even though passing a zero count to the COM method should be valid. Modify the setup to return nil for empty inputs, and only index element 0 when len({{.Param.LocalName}}) > 0.

🤖 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 `@webview2/scripts/generator/types/templates/inputStringArraySetup.tmpl` around
lines 3 - 15, The setup for converting {{.Param.Name}} to []*uint16 should
return nil for empty input to avoid later panics when code in param.go
unconditionally emits &{{.Param.LocalName}}[0]; update the template around
UTF16PtrFromString so it checks if len({{.Param.Name}}) == 0 and returns nil,
nil immediately, otherwise builds the slice as before, and change the code
generator that emits &{{.Param.LocalName}}[0] (in param.go) to only emit that
expression when len({{.Param.LocalName}}) > 0 (i.e., conditionally index element
0).

Comment on lines 37 to 50
{{if .BaseClass }}
func (i *ICoreWebView2) Get{{.Name}}() *{{.Name}} {
func (i *ICoreWebView2) Get{{.Name}}() (*{{.Name}}, error) {
var result *{{.Name}}

iid{{.Name}} := NewGUID({{.Header.AsString}})
_, _, _ = i.Vtbl.QueryInterface.Call(
hr, _, _ := i.Vtbl.QueryInterface.Call(
uintptr(unsafe.Pointer(i)),
uintptr(unsafe.Pointer(iid{{.Name}})),
uintptr(unsafe.Pointer(&result)))

return result
if windows.Handle(hr) != windows.S_OK {
return nil, syscall.Errno(hr)
}
return result, nil
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Generate Get{{.Name}} on the actual base interface.

This helper is still hard-coded to *ICoreWebView2, so non-WebView2 inheritance chains are generated on the wrong receiver. The regenerated ICoreWebView2ProcessFailedEventArgs2.go.txt now emits func (i *ICoreWebView2) GetICoreWebView2ProcessFailedEventArgs2(), which means callers on *ICoreWebView2ProcessFailedEventArgs still can't use the helper.

Suggested fix
-func (i *ICoreWebView2) Get{{.Name}}() (*{{.Name}}, error) {
+func (i *{{.BaseClass}}) Get{{.Name}}() (*{{.Name}}, error) {
🤖 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 `@webview2/scripts/generator/types/templates/interfacevtbl.tmpl` around lines
37 - 50, The generated helper method Get{{.Name}} is hard-coded to the receiver
type *ICoreWebView2; change the template so the receiver is the actual base
interface type instead of ICoreWebView2 (use the template's base-class symbol,
e.g. {{.BaseClass}} or the correct field that holds the base interface name) so
the generated signature becomes func (i *<actualBase>) Get{{.Name}}(), and
ensure the rest of the body still uses iid{{.Name}},
NewGUID({{.Header.AsString}}) and i.Vtbl.QueryInterface.Call as before.

@leaanthony
Copy link
Copy Markdown
Member Author

Superseded by #5419 — the v2 consolidation branch feat/webview2-v2 folds this PR's 6 bug fixes in alongside the parser/type-system foundation from #5407, the new webview2gen CLI, the capabilities emitter, tests, docs, and the CI gate. Closing in favor of the consolidated review.

@leaanthony leaanthony closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants