Skip to content

Commit

Permalink
GetPhysicalKeyStatus use correct COM struct definition
Browse files Browse the repository at this point in the history
This fixes some memory corruptions during Accelerator
callback handling
  • Loading branch information
stffabi committed Oct 23, 2023
1 parent c9ef764 commit d75e86c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 13 deletions.
11 changes: 11 additions & 0 deletions pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,14 @@ type COREWEBVIEW2_PHYSICAL_KEY_STATUS struct {
WasKeyDown bool
IsKeyReleased bool
}

// Bools need to be int32 in the native struct otherwise we end up in memory corruption. Using the internal
// struct is a hacky way so we don't break the public interface.
type internal_COREWEBVIEW2_PHYSICAL_KEY_STATUS struct {
RepeatCount uint32
ScanCode uint32
IsExtendedKey int32
IsMenuKeyDown int32
WasKeyDown int32
IsKeyReleased int32
}
11 changes: 9 additions & 2 deletions pkg/edge/ICoreWebView2AcceleratorKeyPressedEventArgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,22 @@ func (i *ICoreWebView2AcceleratorKeyPressedEventArgs) GetVirtualKey() (uint, err

func (i *ICoreWebView2AcceleratorKeyPressedEventArgs) GetPhysicalKeyStatus() (COREWEBVIEW2_PHYSICAL_KEY_STATUS, error) {
var err error
var physicalKeyStatus COREWEBVIEW2_PHYSICAL_KEY_STATUS
var physicalKeyStatus internal_COREWEBVIEW2_PHYSICAL_KEY_STATUS
_, _, err = i.vtbl.GetPhysicalKeyStatus.Call(
uintptr(unsafe.Pointer(i)),
uintptr(unsafe.Pointer(&physicalKeyStatus)),
)
if err != windows.ERROR_SUCCESS {
return COREWEBVIEW2_PHYSICAL_KEY_STATUS{}, err
}
return physicalKeyStatus, nil
return COREWEBVIEW2_PHYSICAL_KEY_STATUS{
RepeatCount: physicalKeyStatus.RepeatCount,
ScanCode: physicalKeyStatus.ScanCode,
IsExtendedKey: physicalKeyStatus.IsExtendedKey != 0,
IsMenuKeyDown: physicalKeyStatus.IsMenuKeyDown != 0,
WasKeyDown: physicalKeyStatus.WasKeyDown != 0,
IsKeyReleased: physicalKeyStatus.IsKeyReleased != 0,
}, nil
}

func (i *ICoreWebView2AcceleratorKeyPressedEventArgs) PutHandled(handled bool) error {
Expand Down
27 changes: 16 additions & 11 deletions pkg/edge/chromium.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"log"
"os"
"path/filepath"
"runtime"
"strings"
"sync/atomic"
"syscall"
Expand Down Expand Up @@ -73,7 +72,6 @@ func NewChromium() *Chromium {
There's a proposal to add a runtime.Pin function, to prevent moving pinned objects, which would allow to easily fix
this issue by just pinning the handlers. The https://go-review.googlesource.com/c/go/+/367296/ should land in Go 1.19.
*/
var pinner runtime.Pinner
e.envCompleted = newICoreWebView2CreateCoreWebView2EnvironmentCompletedHandler(e)
e.controllerCompleted = newICoreWebView2CreateCoreWebView2ControllerCompletedHandler(e)
e.webMessageReceived = newICoreWebView2WebMessageReceivedEventHandler(e)
Expand All @@ -83,15 +81,22 @@ func NewChromium() *Chromium {
e.navigationCompleted = newICoreWebView2NavigationCompletedEventHandler(e)
e.processFailed = newICoreWebView2ProcessFailedEventHandler(e)
e.containsFullScreenElementChanged = newICoreWebView2ContainsFullScreenElementChangedEventHandler(e)
pinner.Pin(e.envCompleted)
pinner.Pin(e.controllerCompleted)
pinner.Pin(e.webMessageReceived)
pinner.Pin(e.permissionRequested)
pinner.Pin(e.webResourceRequested)
pinner.Pin(e.acceleratorKeyPressed)
pinner.Pin(e.navigationCompleted)
pinner.Pin(e.processFailed)
pinner.Pin(e.containsFullScreenElementChanged)
/*
// Pinner seems to panic in some cases as reported on Discord, maybe during shutdown when GC detects pinned objects
// to be released that have not been unpinned.
// It would also be better to use our ComBridge for this event handlers implementation instead of pinning them.
// So all COM Implementations on the go-side use the same code.
var pinner runtime.Pinner
pinner.Pin(e.envCompleted)
pinner.Pin(e.controllerCompleted)
pinner.Pin(e.webMessageReceived)
pinner.Pin(e.permissionRequested)
pinner.Pin(e.webResourceRequested)
pinner.Pin(e.acceleratorKeyPressed)
pinner.Pin(e.navigationCompleted)
pinner.Pin(e.processFailed)
pinner.Pin(e.containsFullScreenElementChanged)
*/
e.permissions = make(map[CoreWebView2PermissionKind]CoreWebView2PermissionState)

return e
Expand Down

0 comments on commit d75e86c

Please sign in to comment.