From d75e86c51980aa6477a63497599fa2d9fd0e7235 Mon Sep 17 00:00:00 2001
From: stffabi <stffabi@users.noreply.github.com>
Date: Mon, 23 Oct 2023 13:59:17 +0200
Subject: [PATCH] GetPhysicalKeyStatus use correct COM struct definition

This fixes some memory corruptions during Accelerator
callback handling
---
 pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go  | 11 ++++++++
 ...eWebView2AcceleratorKeyPressedEventArgs.go | 11 ++++++--
 pkg/edge/chromium.go                          | 27 +++++++++++--------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go b/pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go
index dd88342..a8698e8 100644
--- a/pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go
+++ b/pkg/edge/COREWEBVIEW2_PHYSICAL_KEY_STATUS.go
@@ -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
+}
diff --git a/pkg/edge/ICoreWebView2AcceleratorKeyPressedEventArgs.go b/pkg/edge/ICoreWebView2AcceleratorKeyPressedEventArgs.go
index 2a3a9c8..20a18ce 100644
--- a/pkg/edge/ICoreWebView2AcceleratorKeyPressedEventArgs.go
+++ b/pkg/edge/ICoreWebView2AcceleratorKeyPressedEventArgs.go
@@ -54,7 +54,7 @@ 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)),
@@ -62,7 +62,14 @@ func (i *ICoreWebView2AcceleratorKeyPressedEventArgs) GetPhysicalKeyStatus() (CO
 	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 {
diff --git a/pkg/edge/chromium.go b/pkg/edge/chromium.go
index 1a2ad06..e15d435 100644
--- a/pkg/edge/chromium.go
+++ b/pkg/edge/chromium.go
@@ -8,7 +8,6 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"sync/atomic"
 	"syscall"
@@ -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)
@@ -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