Try to resolve random crash on Windows

The debugger tells me the Invoke method is accessing an invalid, but
non-null pointer. This appears to be a use-after-free delegate context
pointer. This is not easy to reproduce - even with the browser, I'd have
to open and close tons of windows and trigger specific callbacks to get
the behavior. (I found the best one to get it was the status text
changed event out of WebView2, probably because it uses a member
variable without any explicit local.)

I suspect it has to do with the delegate trying to capture both `this`
and other local variables at the same time. I couldn't confirm this, but
the theory is that normally, when a delegate captures a local variable,
the compiler places it in an allocated heap block instead of on the
stack, but perhaps `this` gets different treatment and it is wiped out
or the block is GC'd or something. (The AddRef does GC.addRoot to the
containing class that had the delegate member, so that should have
prevented it from disappearing, but it is possible I did it wrong.)

Nevertheless, I recall having trouble with this in the past as well,
and decided perhaps the best thing to do is to just take control of
which context, exactly, is passed and retained. Hence the conversion to
`function` so I can hold on to it better. This makes things harder to
use - especially since IFTI wouldn't pick up the usage correctly - but
meh, I'm mostly just passing the class handles down so not that bad.

Not 100% sure this actually even fixed the crash; it hasn't happened
again since using it, but that's no guarantee given how difficult it is
to reproduce.

minigui_addons/webview.d

@@ -138,149 +138,158 @@ class WebViewWidget_WV2 : WebViewWidgetBase {
 
 	private bool initialized;
 
-	this(string url, void delegate(scope OpenNewWindowParams) openNewWindow, BrowserSettings settings, Widget parent) {
+	private HRESULT initializeWithController(ICoreWebView2Controller controller_raw) {
-		// FIXME: openNewWindow and openUrlInNewWindow
-		super(parent);
-		// that ctor sets containerWindow
 
-		Wv2App.useEnvironment((env) {
+		// need to keep this beyond the callback or we're doomed.
-			env.CreateCoreWebView2Controller(containerWindow.impl.hwnd,
+		this.controller = RC!ICoreWebView2Controller(controller_raw);
-				callback!(ICoreWebView2CreateCoreWebView2ControllerCompletedHandler)(delegate(error, controller_raw) {
-					if(error || controller_raw is null)
-						return error;
 
-					// need to keep this beyond the callback or we're doomed.
+		this.webview_window = controller.CoreWebView2;
-					controller = RC!ICoreWebView2Controller(controller_raw);
 
-					webview_window = controller.CoreWebView2;
+		this.webview_window_ext_1 = this.webview_window.queryInterface!(ICoreWebView2_16);
 
-					webview_window_ext_1 = webview_window.queryInterface!(ICoreWebView2_16);
+		bool enableStatusBar = true;
 
-					bool enableStatusBar = true;
+		if(this.webview_window_ext_1) {
+			enableStatusBar = false;
+			this.webview_window_ext_1.add_StatusBarTextChanged!(typeof(this))((sender, args, this_) {
+				this_.status = toGC(&this_.webview_window_ext_1.raw.get_StatusBarText);
+				return S_OK;
+			}, this);
 
-					if(webview_window_ext_1) {
+			webview_window_ext_1.add_FaviconChanged!(typeof(this))((sender, args, this_) {
-						enableStatusBar = false;
+				this_.webview_window_ext_1.GetFavicon(
-						webview_window_ext_1.add_StatusBarTextChanged((sender, args) {
+					COREWEBVIEW2_FAVICON_IMAGE_FORMAT_PNG,
-							this.status = toGC(&webview_window_ext_1.raw.get_StatusBarText);
+					callback!(ICoreWebView2GetFaviconCompletedHandler, typeof(this_))(function(error, streamPtrConst, ctx2) {
-							return S_OK;
-						});
 
-						webview_window_ext_1.add_FaviconChanged((sender, args) {
+						auto streamPtr = cast(IStream) streamPtrConst;
-							webview_window_ext_1.GetFavicon(
-								COREWEBVIEW2_FAVICON_IMAGE_FORMAT_PNG,
-								callback!(ICoreWebView2GetFaviconCompletedHandler)(delegate(error, streamPtrConst) {
 
-									auto streamPtr = cast(IStream) streamPtrConst;
+						ubyte[] buffer = new ubyte[](640); // most favicons are pretty small
+						enum growth_size = 1024; // and we'll grow linearly by the kilobyte
+						size_t at;
 
-									ubyte[] buffer = new ubyte[](640); // most favicons are pretty small
+						more:
-									enum growth_size = 1024; // and we'll grow linearly by the kilobyte
+						ULONG actuallyRead;
-									size_t at;
+						auto ret = streamPtr.Read(buffer.ptr + at, cast(UINT) (buffer.length - at), &actuallyRead);
+						if(ret == S_OK) {
+							// read completed, possibly more data pending
+							auto moreData = actuallyRead >= (buffer.length - at);
 
-									more:
+							at += actuallyRead;
-									ULONG actuallyRead;
+							if(moreData && (buffer.length - at < growth_size))
-									auto ret = streamPtr.Read(buffer.ptr + at, cast(UINT) (buffer.length - at), &actuallyRead);
+								buffer.length += growth_size;
-									if(ret == S_OK) {
+							goto more;
-										// read completed, possibly more data pending
+						} else if(ret == S_FALSE) {
-										auto moreData = actuallyRead >= (buffer.length - at);
+							// end of file reached
+							at += actuallyRead;
+							buffer = buffer[0 .. at];
 
-										at += actuallyRead;
+							import arsd.png;
-										if(moreData && (buffer.length - at < growth_size))
+							ctx2.favicon = readPngFromBytes(buffer);
-											buffer.length += growth_size;
+						} else {
-										goto more;
+							// other error
-									} else if(ret == S_FALSE) {
+							throw new ComException(ret);
-										// end of file reached
-										at += actuallyRead;
-										buffer = buffer[0 .. at];
-
-										import arsd.png;
-										this.favicon = readPngFromBytes(buffer);
-									} else {
-										// other error
-										throw new ComException(ret);
-									}
-
-									return S_OK;
-								})
-							);
-
-							return S_OK;
-						});
-					}
-
-					webview_window.add_DocumentTitleChanged((sender, args) {
-						this.title = toGC(&sender.get_DocumentTitle);
-						return S_OK;
-					});
-
-					webview_window.add_NewWindowRequested((sender, args) {
-						// args.get_Uri
-						// args.get_IsUserInitiated
-						// args.put_NewWindow();
-
-						string url = toGC(&args.get_Uri);
-						int ret;
-
-						WebViewWidget_WV2 widget;
-
-						runInGuiThread({
-							ret = 0;
-
-							scope WebViewWidget delegate(Widget, BrowserSettings) accept = (parent, passed_settings) {
-								ret = 1;
-								if(parent !is null) {
-									auto widget = new WebViewWidget_WV2(url, openNewWindow, passed_settings, parent);
-
-									return widget;
-								}
-								return null;
-							};
-							openNewWindow(OpenNewWindowParams(url, accept));
-							return;
-						});
-
-						if(ret) {
-							args.put_Handled(true);
-							// args.put_NewWindow(widget.webview_window.returnable);
 						}
 
 						return S_OK;
+					}, this_)
+				);
 
-					});
+				return S_OK;
+			}, this);
+		}
 
-					// add_HistoryChanged
+		webview_window.add_DocumentTitleChanged!(typeof(this))((sender, args, this_) {
-					// that's where CanGoBack and CanGoForward can be rechecked.
+			this_.title = toGC(&sender.get_DocumentTitle);
+			return S_OK;
+		}, this);
 
-					RC!ICoreWebView2Settings Settings = webview_window.Settings;
+		webview_window.add_NewWindowRequested!(typeof(this))((sender, args, this_) {
-					Settings.IsScriptEnabled = TRUE;
+			// args.get_Uri
-					Settings.AreDefaultScriptDialogsEnabled = TRUE;
+			// args.get_IsUserInitiated
-					Settings.IsWebMessageEnabled = TRUE;
+			// args.put_NewWindow();
-					Settings.IsStatusBarEnabled = enableStatusBar;
 
-					auto ert = webview_window.add_NavigationStarting(
+			string url = toGC(&args.get_Uri);
-						delegate (sender, args) {
+			int ret;
-							this.url = toGC(&args.get_Uri);
-							return S_OK;
-						});
 
-					RECT bounds;
+			WebViewWidget_WV2 widget;
-					GetClientRect(containerWindow.impl.hwnd, &bounds);
-					controller.Bounds = bounds;
-					//error = webview_window.Navigate("http://arsdnet.net/test.html"w.ptr);
-					//error = webview_window.NavigateToString("<html><body>Hello</body></html>"w.ptr);
-					//error = webview_window.Navigate("http://192.168.1.10/"w.ptr);
 
-					if(url !is null) {
+			runInGuiThread({
-						WCharzBuffer bfr = WCharzBuffer(url);
+				ret = 0;
-						webview_window.Navigate(bfr.ptr);
+
+				scope WebViewWidget delegate(Widget, BrowserSettings) accept = (parent, passed_settings) {
+					ret = 1;
+					if(parent !is null) {
+						auto widget = new WebViewWidget_WV2(url, this_.openNewWindow, passed_settings, parent);
+
+						return widget;
 					}
-
+					return null;
-					controller.IsVisible = true;
+				};
-
+				this_.openNewWindow(OpenNewWindowParams(url, accept));
-					initialized = true;
+				return;
-
-					return S_OK;
-				}));
 			});
+
+			if(ret) {
+				args.put_Handled(true);
+				// args.put_NewWindow(widget.webview_window.returnable);
+			}
+
+			return S_OK;
+
+		}, this);
+
+		// add_HistoryChanged
+		// that's where CanGoBack and CanGoForward can be rechecked.
+
+		RC!ICoreWebView2Settings Settings = this.webview_window.Settings;
+		Settings.IsScriptEnabled = TRUE;
+		Settings.AreDefaultScriptDialogsEnabled = TRUE;
+		Settings.IsWebMessageEnabled = TRUE;
+		Settings.IsStatusBarEnabled = enableStatusBar;
+
+		auto ert = this.webview_window.add_NavigationStarting!(typeof(this))(
+			function (sender, args, this_) {
+				this_.url = toGC(&args.get_Uri);
+				return S_OK;
+			}, this);
+
+		RECT bounds;
+		GetClientRect(this.containerWindow.impl.hwnd, &bounds);
+		controller.Bounds = bounds;
+		//error = webview_window.Navigate("http://arsdnet.net/test.html"w.ptr);
+		//error = webview_window.NavigateToString("<html><body>Hello</body></html>"w.ptr);
+		//error = webview_window.Navigate("http://192.168.1.10/"w.ptr);
+
+		if(url !is null) {
+			WCharzBuffer bfr = WCharzBuffer(url);
+			this.webview_window.Navigate(bfr.ptr);
+		}
+
+		controller.IsVisible = true;
+
+		this.initialized = true;
+
+		return S_OK;
+	}
+
+	private void delegate(scope OpenNewWindowParams) openNewWindow;
+
+	this(string url, void delegate(scope OpenNewWindowParams) openNewWindow, BrowserSettings settings, Widget parent) {
+		this.openNewWindow = openNewWindow;
+		super(parent);
+		// that ctor sets containerWindow
+
+		this.url = url;
+
+		Wv2App.useEnvironment((env) {
+			env.CreateCoreWebView2Controller(containerWindow.impl.hwnd,
+				callback!(ICoreWebView2CreateCoreWebView2ControllerCompletedHandler, typeof(this))(function(error, controller_raw, ctx) {
+					if(error || controller_raw is null)
+						return error;
+
+					return ctx.initializeWithController(controller_raw);
+				}, this));
+		});
 	}
 
 	override void registerMovementAdditionalWork() {

webview.d

@@ -63,17 +63,31 @@ import core.atomic;
 
 //import std.stdio;
 
-T callback(T)(typeof(&T.init.Invoke) dg) {
+private template InvokerArgFor(T, Context) {
-	return new class T {
+	alias invoker = typeof(&T.init.Invoke);
+	static if(is(invoker fntype == delegate))
+	static if(is(fntype Params == __parameters))
+		alias InvokerArgFor = HRESULT function(Params, Context);
+	else
+		static assert(0);
+}
+
+T callback(T, Context)(InvokerArgFor!(T, Context) dg, Context ctx) {
+	return new class(dg, ctx) T {
 		extern(Windows):
 
 		static if(is(typeof(T.init.Invoke) R == return))
 		static if(is(typeof(T.init.Invoke) P == __parameters))
   		override R Invoke(P _args_) {
-			return dg(_args_);
+			return dgMember(_args_, ctxMember);
 		}
 
-		this() {
+		InvokerArgFor!(T, Context) dgMember;
+		Context ctxMember;
+
+		this(typeof(dgMember) dg_, Context ctx_) {
+			this.dgMember = dg_;
+			this.ctxMember = ctx_;
 			AddRef();
 		}
 
@@ -238,10 +252,9 @@ mixin template ForwardMethod(string methodName) {
 	static if(methodName.length > 4 && methodName[0 .. 4] == "add_") {
 		static if(is(typeof(__traits(getMember, T, memberName)) Params == function))
 			alias Handler = Params[0];
-		alias HandlerDg = typeof(&Handler.init.Invoke);
+		mixin(q{ EventRegistrationToken } ~ memberName ~ q{ (Context)(InvokerArgFor!(Handler, Context) handler, Context ctx) {
-		mixin(q{ EventRegistrationToken } ~ memberName ~ q{ (HandlerDg handler) {
 			EventRegistrationToken token;
-			__traits(getMember, object, memberName)(callback!Handler(handler), &token);
+			__traits(getMember, object, memberName)(callback!(Handler, Context)(handler, ctx), &token);
 			return token;
 		}});
 	} else
@@ -296,26 +309,26 @@ struct Wv2App {
 			throw new Exception("CreateCoreWebView2EnvironmentWithOptions failed from WebView2Loader...");
 
 		auto result = func(null, null, null,
-			callback!(ICoreWebView2CreateCoreWebView2EnvironmentCompletedHandler)(
+			callback!(ICoreWebView2CreateCoreWebView2EnvironmentCompletedHandler, Wv2App*)(
-				delegate(error, env) {
+				function(error, env, this_) {
-					initialized = true;
+					this_.initialized = true;
-					code = error;
+					this_.code = error;
 
 					if(error)
 						return error;
 
-					webview_env = env;
+					this_.webview_env = env;
 
 					auto len = pending.length;
-					foreach(item; pending) {
+					foreach(item; this_.pending) {
-						item(webview_env);
+						item(this_.webview_env);
 					}
 
-					pending = pending[len .. $];
+					this_.pending = this_.pending[len .. $];
 
 					return S_OK;
 				}
-			)
+			, &this)
 		);
 
 		if(result != S_OK) {