fix(webview): recover from renderer crashes, and stop dropping window.open()

packages/zig/src/macos.zigmodified+269-23

Changes to packages/zig/src/macos.zig

		⋯3 linesShow the 3 lines above this hunk@@ -4,6 +4,8 @@ const javascript = ;
4	4	const native_sidebar_bootstrap = ;
5	5	const local_tls = ;
6	6	const menu_roles = ;
	7	+const external_link = ;
	8	+const webview_recovery = ;
7	9
8	10	// Objective-C runtime types and functions (manual declarations to avoid @cImport issues)
9	11	pub const objc = struct {
		⋯833 linesShow the 833 lines above this hunk@@ -843,6 +845,7 @@ pub fn createWindowWithStyle(title: []const u8, width: u32, height: u32, html: ?
843	845	const nsurl = msgSend1(getClass("NSURL"), "URLWithString:", url_str);
844	846	const request = msgSend1(getClass("NSURLRequest"), "requestWithURL:", nsurl);
845	847	_ = msgSend1(webview, "loadRequest:", request);
	848	+ rememberContent(webview, .{ .request = request });
846	849	startup_timing.mark(.load_started);
847	850	} else if (html) |h| {
848	851	if (style.benchmark) {
		⋯17 linesShow the 17 lines above this hunk@@ -866,6 +869,7 @@ pub fn createWindowWithStyle(title: []const u8, width: u32, height: u32, html: ?
866	869	const base_url = msgSend1(getClass("NSURL"), "URLWithString:", base_url_string);
867	870
868	871	_ = msgSend2(webview, "loadHTMLString:baseURL:", html_str, base_url);
	872	+ rememberContent(webview, .{ .html = .{ .string = html_str, .base_url = base_url } });
869	873	}
870	874	}
871	875
		⋯10 linesShow the 10 lines above this hunk@@ -882,21 +886,34 @@ pub fn createWindowWithStyle(title: []const u8, width: u32, height: u32, html: ?
882	886	.install();
883	887	}
884	888
885		- // Set up UI delegate for camera/microphone permission handling (skip if not needed)
886		- if (style.dev_tools) {
	889	+ // Both delegates, in every non-benchmark window.
	890	+ //
	891	+ // The UI delegate used to be installed only with `--dev-tools`, which is
	892	+ // not a condition any of its callbacks have anything to do with: a
	893	+ // packaged app calling `getUserMedia()` got no delegate and so was denied
	894	+ // the camera outright, and `window.open()` had nowhere to be handled. The
	895	+ // navigation delegate used to install only under `CRAFT_ALLOW_LOCAL_TLS`,
	896	+ // which left every shipped window without the callback that notices the
	897	+ // content process has died.
	898	+ //
	899	+ // Skipped in benchmark mode alongside the file-drop hook and the bridge,
	900	+ // so the startup numbers keep measuring the same window they always did.
	901	+ //
	902	+ // After the load, not before, and that is safe: `loadRequest:` only
	903	+ // schedules the fetch, and every delegate callback arrives through the run
	904	+ // loop, which is not entered until much later. (The note that used to be
	905	+ // here said "before the load below" — the load moved above it.)
	906	+ if (!style.benchmark) {
887	907	setupUIDelegate(webview) catch |err| {
888	908	if (comptime builtin.mode == .debug)
889	909	std.debug.print("[Media] Failed to setup UI delegate: {}\n", .{err});
890	910	};
891		- }
892	911
893		- // Not gated on dev_tools: whether the inspector is available and whether a
894		- // local certificate is trusted are unrelated questions. Before the load
895		- // below, so it is in place for the first request's challenge.
896		- setupNavigationDelegate(webview) catch |err| {
897		- if (comptime builtin.mode == .debug)
898		- std.debug.print("[Nav] Failed to setup navigation delegate: {}\n", .{err});
899		- };
	912	+ setupNavigationDelegate(webview) catch |err| {
	913	+ if (comptime builtin.mode == .debug)
	914	+ std.debug.print("[Nav] Failed to setup navigation delegate: {}\n", .{err});
	915	+ };
	916	+ }
900	917
901	918	if (style.benchmark) {
902	919	// Benchmark mode: minimal setup — just set content view and return
		⋯4,250 linesShow the 4,250 lines above this hunk@@ -5153,7 +5170,69 @@ fn handleMediaCapturePermission(
5153	5170	}
5154	5171	}
5155	5172
5156		-/// Set up WKUIDelegate to handle media capture permission requests
	5173	+/// `webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:`
	5174	+///
	5175	+/// WKWebView asks for this whenever the page wants a second window —
	5176	+/// `window.open()`, `target="_blank"`, `<form target>`. If the delegate does
	5177	+/// not implement it, WebKit's answer is to do nothing at all: `window.open()`
	5178	+/// returns null and the link dies with no error, no console message and no
	5179	+/// log. Every link in a rendered transcript or feed simply did nothing.
	5180	+///
	5181	+/// Craft has no second window to give (that is #67), so the answer is the one
	5182	+/// Electron settled on: hand the URL to the system browser and return nil, so
	5183	+/// no child webview is created.
	5184	+///
	5185	+/// Which URLs get handed over is decided by `external_link.zig`, and it is
	5186	+/// deliberately narrower than `shell.openExternal`: this is a *page* asking,
	5187	+/// possibly on behalf of content nobody vetted, and `NSWorkspace openURL:`
	5188	+/// means "ask LaunchServices to do whatever this URL means".
	5189	+fn handleCreateWebView(
	5190	+ _: objc.id,
	5191	+ _: objc.SEL,
	5192	+ _: objc.id,
	5193	+ _: objc.id,
	5194	+ navigationAction: objc.id,
	5195	+ _: objc.id,
	5196	+) callconv(.c) objc.id {
	5197	+ // Returning nil is the point of the method, so every path out returns nil.
	5198	+ const request = msgSend0(navigationAction, "request");
	5199	+ if (request == null) return null;
	5200	+ const nsurl = msgSend0(request, "URL");
	5201	+ if (nsurl == null) return null;
	5202	+
	5203	+ const absolute = msgSend0(nsurl, "absoluteString");
	5204	+ if (absolute == null) return null;
	5205	+ const raw = msgSend0(absolute, "UTF8String") orelse return null;
	5206	+ const cstr: [*:0]const u8 = @ptrCast(@alignCast(raw));
	5207	+ const url = std.mem.span(cstr);
	5208	+
	5209	+ switch (external_link.decide(url)) {
	5210	+ .open_externally => {
	5211	+ const NSWorkspace = getClass("NSWorkspace");
	5212	+ const workspace = msgSend0(NSWorkspace, "sharedWorkspace");
	5213	+ _ = msgSend1(workspace, "openURL:", nsurl);
	5214	+ },
	5215	+ // Logged, never silent: a dropped link that says nothing is the bug
	5216	+ // this method exists to fix, and refusing one quietly is the same
	5217	+ // experience by a different route.
	5218	+ .drop => std.log.warn(
	5219	+ "refused to open a new window for a URL the page cannot send to the browser: {s}",
	5220	+ .{url},
	5221	+ ),
	5222	+ }
	5223	+
	5224	+ return null;
	5225	+}
	5226	+
	5227	+/// Attach craft's `WKUIDelegate`.
	5228	+///
	5229	+/// Two callbacks, neither of which has anything to do with the other:
	5230	+/// `getUserMedia()` permission, and the page asking for a second window.
	5231	+///
	5232	+/// Installed in every non-benchmark window. It used to be installed only under
	5233	+/// `--dev-tools`, which meant a packaged app was denied the camera outright
	5234	+/// and had `window.open()` silently dropped — neither of which is a debugging
	5235	+/// concern.
5157	5236	pub fn setupUIDelegate(webview: objc.id) !void {
5158	5237	if (comptime builtin.mode == .debug)
5159	5238	std.debug.print("[Media] Setting up WKUIDelegate for media permissions...\n", .{});
		⋯37 linesShow the 37 lines above this hunk@@ -5197,6 +5276,19 @@ pub fn setupUIDelegate(webview: objc.id) !void {
5197	5276	}
5198	5277	}
5199	5278
	5279	+ // `window.open()` / `target="_blank"`. Returns an object (the child
	5280	+ // webview, or nil): @ = id return, self, _cmd, webView, configuration,
	5281	+ // navigationAction, windowFeatures.
	5282	+ if (!objc.class_addMethod(
	5283	+ @ptrCast(@alignCast(delegateClass)),
	5284	+ objc.sel_registerName("webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:"),
	5285	+ @as(objc.IMP, @ptrCast(@constCast(&handleCreateWebView))),
	5286	+ "@@:@@@@",
	5287	+ )) {
	5288	+ if (comptime builtin.mode == .debug)
	5289	+ std.debug.print("[UI] Failed to add createWebView method\n", .{});
	5290	+ }
	5291	+
5200	5292	// Register the class
5201	5293	objc.objc_registerClassPair(@ptrCast(delegateClass));
5202	5294	if (comptime builtin.mode == .debug)
		⋯86 linesShow the 86 lines above this hunk@@ -5289,6 +5381,14 @@ fn handleAuthChallenge(
5289	5381	challenge: objc.id,
5290	5382	completionHandler: objc.id,
5291	5383	) callconv(.c) void {
	5384	+ // The opt-in used to be enforced by not installing the delegate at all.
	5385	+ // The delegate is installed in every window now — it is what notices the
	5386	+ // content process has died — so the check has to live here instead.
	5387	+ // Without this line, ungating the delegate would silently start trusting
	5388	+ // local development certificates in every craft app ever shipped.
	5389	+ if (!allowsLocalDevTLS())
	5390	+ return finishAuthChallenge(completionHandler, .perform_default_handling, null);
	5391	+
5292	5392	const protectionSpace = msgSend0(challenge, "protectionSpace");
5293	5393	if (protectionSpace == null)
5294	5394	return finishAuthChallenge(completionHandler, .perform_default_handling, null);
		⋯14 linesShow the 14 lines above this hunk@@ -5309,19 +5409,154 @@ fn handleAuthChallenge(
5309	5409	finishAuthChallenge(completionHandler, .use_credential, credential);
5310	5410	}
5311	5411
5312		-/// Attach a `WKNavigationDelegate` that trusts local development certificates.
	5412	+// =============================================================================
	5413	+// Recovering a window after the WebKit content process dies
	5414	+// =============================================================================
	5415	+
	5416	+/// What craft put in a webview, kept so it can be put back.
5313	5417	///
5314		-/// A no-op unless the process opted in: without `CRAFT_ALLOW_LOCAL_TLS` craft
5315		-/// installs no navigation delegate at all, which is exactly the behaviour it
5316		-/// has always had. Implementing only the auth-challenge method leaves every
5317		-/// other navigation decision — policy, redirects, provisional failures — at its
5318		-/// AppKit default.
	5418	+/// `-reload` is Apple's documented recovery and it is wrong for half of
	5419	+/// craft's windows. HTML loaded through `loadHTMLString:baseURL:` has no
	5420	+/// request to repeat: reloading navigates to the synthetic base URL,
	5421	+/// `http://localhost/`, where there is usually nothing listening — so a window
	5422	+/// started from `--html` would come back as a connection error instead of the
	5423	+/// app it was showing. Re-applying what was loaded is right for both paths.
	5424	+const LoadedContent = union(enum) {
	5425	+ none,
	5426	+ request: objc.id,
	5427	+ html: struct { string: objc.id, base_url: objc.id },
	5428	+};
	5429	+
	5430	+const ContentSlot = struct {
	5431	+ key: usize = 0,
	5432	+ content: LoadedContent = .none,
	5433	+};
	5434	+
	5435	+/// Sized to match `webview_recovery`'s budget table: the two are keyed the
	5436	+/// same way and a window in one should be in the other.
	5437	+var content_slots: [8]ContentSlot = @splat(.{});
	5438	+
	5439	+/// Hold on to what a webview was given, retaining it for as long as the window
	5440	+/// might need it back.
5319	5441	///
5320		-/// Call before loading, so the delegate is in place for the first request's
5321		-/// challenge.
5322		-pub fn setupNavigationDelegate(webview: objc.id) !void {
5323		- if (!allowsLocalDevTLS()) return;
	5442	+/// `requestWithURL:` and `URLWithString:` both hand back autoreleased objects,
	5443	+/// so without the retain these are dangling by the time the run loop next
	5444	+/// drains — which is long before any crash. The matching release would be at
	5445	+/// window teardown; craft never tears one down today, and one request or one
	5446	+/// HTML string per window is not a leak worth the risk of releasing early.
	5447	+fn rememberContent(webview: objc.id, content: LoadedContent) void {
	5448	+ switch (content) {
	5449	+ .none => {},
	5450	+ .request => |r| _ = msgSend0(r, "retain"),
	5451	+ .html => |h| {
	5452	+ _ = msgSend0(h.string, "retain");
	5453	+ if (h.base_url != null) _ = msgSend0(h.base_url, "retain");
	5454	+ },
	5455	+ }
	5456	+
	5457	+ const key = @intFromPtr(webview);
	5458	+ // Existing entry first, then a free one. Taking whichever comes first would
	5459	+ // leave a stale duplicate behind for a webview that loads twice.
	5460	+ for (&content_slots) |*slot| {
	5461	+ if (slot.key == key) {
	5462	+ slot.content = content;
	5463	+ return;
	5464	+ }
	5465	+ }
	5466	+ for (&content_slots) |*slot| {
	5467	+ if (slot.key == 0) {
	5468	+ slot.* = .{ .key = key, .content = content };
	5469	+ return;
	5470	+ }
	5471	+ }
	5472	+ // Full. Slot zero holds the earliest window registered, which is the one
	5473	+ // least likely to still be around.
	5474	+ content_slots[0] = .{ .key = key, .content = content };
	5475	+}
	5476	+
	5477	+fn contentFor(webview: objc.id) LoadedContent {
	5478	+ const key = @intFromPtr(webview);
	5479	+ for (&content_slots) |slot| {
	5480	+ if (slot.key == key) return slot.content;
	5481	+ }
	5482	+ return .none;
	5483	+}
	5484	+
	5485	+/// Put the window's content back after the content process was replaced.
	5486	+fn restoreContent(webview: objc.id) void {
	5487	+ switch (contentFor(webview)) {
	5488	+ .request => |r| _ = msgSend1(webview, "loadRequest:", r),
	5489	+ .html => |h| _ = msgSend2(webview, "loadHTMLString:baseURL:", h.string, h.base_url),
	5490	+ // Nothing recorded — a webview craft did not load. `reload` is the
	5491	+ // best available guess and does no harm if there is nothing to reload.
	5492	+ .none => _ = msgSend0(webview, "reload"),
	5493	+ }
	5494	+}
	5495	+
	5496	+/// `webViewWebContentProcessDidTerminate:`
	5497	+///
	5498	+/// WKWebView renders in a separate process, and when that process is killed
	5499	+/// the view does not error or close — it goes white and stays white. This is
	5500	+/// the only notification of it. Without this method craft had no route back
	5501	+/// except the Reload menu item, which a user has to know to look for, and
	5502	+/// which nothing on the blank screen suggests.
	5503	+///
	5504	+/// Logged at warning level rather than behind a debug check: an app that quietly
	5505	+/// reloaded itself at 3am is something whoever reads the logs should be able to
	5506	+/// find out about.
	5507	+fn handleContentProcessTerminated(_: objc.id, _: objc.SEL, webview: objc.id) callconv(.c) void {
	5508	+ const key = @intFromPtr(webview);
	5509	+ const state = webview_recovery.stateFor(key);
	5510	+ // `compat.nanoTimestamp` is CLOCK_MONOTONIC here, which is what a rate
	5511	+ // budget wants: a clock the user can move is a clock that can hand out
	5512	+ // extra reloads. It returns 0 if the clock cannot be read at all, which
	5513	+ // reads as "no time passed" and keeps the burst intact — the conservative
	5514	+ // direction.
	5515	+ const now = .nanoTimestamp();
	5516	+ const action = webview_recovery.onCrash(state, now, .{});
	5517	+
	5518	+ switch (action) {
	5519	+ .reload => {
	5520	+ std.log.warn(
	5521	+ "WebKit content process terminated; restoring the window (attempt {d})",
	5522	+ .{state.attempts},
	5523	+ );
	5524	+ restoreContent(webview);
	5525	+ },
	5526	+ .give_up => {
	5527	+ std.log.warn(
	5528	+ "WebKit content process terminated {d} times in quick succession; " ++
	5529	+ "leaving the window alone so it stops looping",
	5530	+ .{state.attempts},
	5531	+ );
	5532	+ const NSString = getClass("NSString");
	5533	+ const html = msgSend1(
	5534	+ NSString,
	5535	+ "stringWithUTF8String:",
	5536	+ @as([*:0]const u8, webview_recovery.give_up_page),
	5537	+ );
	5538	+ _ = msgSend2(webview, "loadHTMLString:baseURL:", html, @as(objc.id, null));
	5539	+ },
	5540	+ }
	5541	+}
5324	5542
	5543	+/// Attach craft's `WKNavigationDelegate`.
	5544	+///
	5545	+/// Two methods, for two unrelated jobs:
	5546	+///
	5547	+/// - `webViewWebContentProcessDidTerminate:` — the only notification that
	5548	+/// WebKit's renderer died. Without it the window is white and stays white.
	5549	+/// Every window needs this.
	5550	+/// - `webView:didReceiveAuthenticationChallenge:completionHandler:` — trusts
	5551	+/// a local development CA, and only when `CRAFT_ALLOW_LOCAL_TLS` says so.
	5552	+/// That opt-in is enforced inside the handler, because the delegate itself
	5553	+/// is no longer optional.
	5554	+///
	5555	+/// It used to install only under `CRAFT_ALLOW_LOCAL_TLS`, which meant every
	5556	+/// shipped app ran without the crash notification. Everything not implemented
	5557	+/// here — navigation policy, redirects, provisional failures — stays at its
	5558	+/// AppKit default, as before.
	5559	+pub fn setupNavigationDelegate(webview: objc.id) !void {
5325	5560	const superclass = getClass("NSObject");
5326	5561	const className = "CraftNavigationDelegate";
5327	5562
		⋯11 linesShow the 11 lines above this hunk@@ -5339,6 +5574,15 @@ pub fn setupNavigationDelegate(webview: objc.id) !void {
5339	5574	if (!objc.class_addMethod(@ptrCast(@alignCast(delegateClass)), method_sel, method_imp, method_types))
5340	5575	return error.MethodAdditionFailed;
5341	5576
	5577	+ // void; self, _cmd, webView
	5578	+ if (!objc.class_addMethod(
	5579	+ @ptrCast(@alignCast(delegateClass)),
	5580	+ objc.sel_registerName("webViewWebContentProcessDidTerminate:"),
	5581	+ @as(objc.IMP, @ptrCast(@constCast(&handleContentProcessTerminated))),
	5582	+ "v@:@",
	5583	+ ))
	5584	+ return error.MethodAdditionFailed;
	5585	+
5342	5586	objc.objc_registerClassPair(@ptrCast(delegateClass));
5343	5587	}
5344	5588
		⋯5 linesShow the 5 lines above this hunk@@ -5350,8 +5594,10 @@ pub fn setupNavigationDelegate(webview: objc.id) !void {
5350	5594	const delegate = msgSend0(msgSend0(delegate_class_id, "alloc"), "init");
5351	5595	msgSendVoid1(webview, "setNavigationDelegate:", delegate);
5352	5596
5353		- if (comptime builtin.mode == .debug)
5354		- std.debug.print("[Nav] Local development TLS enabled via {s}\n", .{local_tls.env_var});
	5597	+ if (comptime builtin.mode == .debug) {
	5598	+ if (allowsLocalDevTLS())
	5599	+ std.debug.print("[Nav] Local development TLS enabled via {s}\n", .{local_tls.env_var});
	5600	+ }
5355	5601	}
5356	5602
5357	5603	// ============================================================================