[v3] Fix not enough memory error when initialising drag and drop on Windows#4765
Conversation
WalkthroughAdds OLE initialization and refines Windows drag-and-drop handling: converts drop coordinates (screen → window → webview), emits drag events (OnEnter/OnOver/OnLeave), registers drop targets for child Chromium render windows, and adds debug traces and duplicate-registration handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
v3/pkg/application/webview_window_windows.go (2)
563-614: Coordinate conversion looks sound; verify behaviour on high‑DPI setups.The logic here for mapping window‑relative coordinates into client/webview coordinates via
GetWindowRect+ClientToScreenis correct and nicely instrumented.One thing to double‑check: on per‑monitor DPI >100%, WebView2/JS uses DIPs (CSS px) while these Win32 APIs work in device pixels. That means the
webviewX/webviewYyou ultimately pass intoInitiateFrontendDropProcessing(via the nativeOnDroppath) might be scaled differently from thex/ycoming from the"FilesDropped:x:y"JS path.Might be worth explicitly testing on a high‑DPI monitor to confirm both paths line up; if not, you may want to normalise here using the window DPI (e.g. via
scaleWithWindowDPI) so native and JS drop coordinates are consistent.
1963-2022: OLE initialisation placement is good; consider lifetime and teardown semantics.Calling
w32.OleInitialise()here, on the same thread and immediately before creating / registering theDropTarget, is the right place and should address the"Not enough memory resources..."failures fromRegisterDragDrop.Two follow‑ups worth considering:
OleInitialize lifetime.
OleInitializeis ref‑counted per thread and normally expects a matchingOleUninitialize. IfsetupChromiumcan run multiple times on the same OS thread (e.g. repeatedly creating/destroying windows), this will keep incrementing the OLE ref count without ever releasing it. For the typical “run for the life of the process” model this is probably fine, but if you want to be strict you could:
- expose a
w32.OleUninitialise()wrapper and call it during app/window teardown, or- guard
w32.OleInitialise()behind async.Onceat package level so it runs only once per thread/process.DropTarget registration teardown. You register the same
DropTargetinstance on every childHWNDviaw32.RegisterDragDrop, and only callw.dropTarget.Release()indestroy(). Ifw32.RegisterDragDropis a thin wrapper over the Win32 API, OLE usually expects a correspondingRevokeDragDrop(hwnd)before destroying the window. It may be worth ensuring you either:
- explicitly revoke for each registered
HWNDon teardown, or- have the w32
DropTarget/registration helper own revocation so registration and revocation stay paired.None of this blocks the fix for #4701, but clarifying the intended lifetime now will help avoid subtle leaks or surprises in more dynamic window lifecycles.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/pkg/application/webview_window_windows.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T23:16:11.570Z
Learnt from: Sammy-T
Repo: wailsapp/wails PR: 4570
File: v2/internal/frontend/desktop/linux/window_webkit6.go:97-108
Timestamp: 2025-10-17T23:16:11.570Z
Learning: For webkit_6/GTK4 builds in v2/internal/frontend/desktop/linux/window_webkit6.go, GTK widget creation should not be wrapped in invokeOnMainThread. The activation mechanism (activateWg + onActivate export) already handles thread safety, and additional wrapping would cause issues.
Applied to files:
v3/pkg/application/webview_window_windows.go
🧬 Code graph analysis (1)
v3/pkg/application/webview_window_windows.go (1)
v3/pkg/w32/ole32.go (1)
OleInitialise(92-94)
|
|
Thanks! 🙏 |
Description
This change fixes
Not enough memory resources are available to complete this operation.error during drag'n'drop example application initialisation.Fixes #4701
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
Checklist:
Not sure if I have to update v2 changelog for v3 bugfix, so leaving it unchecked.
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.