ReviewOS

also looking at this

craft-native/craft

feat: add Linux and Windows named-window routing

#325
Merged glennmichael123 wants to merge codex/multi-window-linux-windows into main
29 files +2250 -200

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

docs/architecture/multi-window-ownership.mdmodified+24-17
Changes to docs/architecture/multi-window-ownership.md
@@ -1,7 +1,7 @@
11# Multi-window ownership
22
33This document records Craft's current multi-window ownership contract. It is
4both a map of the implemented macOS behavior and a boundary around the choices
4both a map of the implemented desktop behavior and a boundary around the choices
55that are still open. A process-global Zig object is not automatically a
66single-window bug: the important question is whether it is a shared service or
77whether it stores a window-specific target.
@@ -10,17 +10,25 @@ whether it stores a window-specific target.
1010
1111| Area | macOS | Linux | Windows |
1212| --- | --- | --- | --- |
13| Runtime creation from `createWindow()` | Implemented | Not wired to the SDK handle contract | Not wired to the SDK handle contract |
14| Stable named handles | Implemented | Not implemented | Not implemented |
15| Sender-authenticated local actions | Implemented | Not implemented | Not implemented |
16| Named cross-window actions | Implemented | Not implemented | Not implemented |
17| Per-window lifecycle events | Implemented | Not implemented | Not implemented |
18| Permanent destroy and cleanup | Implemented for named windows | Not implemented | Not implemented |
13| Runtime creation from `createWindow()` | Implemented | Implemented; GUI smoke in CI | Implemented; cross-build in CI |
14| Stable named handles | Implemented | Implemented | Implemented |
15| Sender-authenticated local actions | Implemented | Implemented through WebKitGTK | Implemented through WebView2 |
16| Named cross-window actions | Implemented | Implemented for core controls | Implemented for core controls |
17| Per-window lifecycle events | Implemented | Implemented | Implemented |
18| Permanent destroy and cleanup | Implemented for named windows | Implemented on close/destroy | Implemented on close/destroy |
1919| Modal and parent relationships | Unspecified | Unspecified | Unspecified |
2020
21Issue #67 therefore remains open. Its macOS core is substantially implemented,
22but closing it would imply cross-platform and modal/parent semantics that Craft
23does not yet provide.
21Issue #67 remains open while the portable runtime smoke and the still-unspecified
22modal/parent semantics are reviewed. A green cross-build alone does not prove
23that WebView2 opens and tears down a real window on Windows.
24
25Linux and Windows keep their live native window/webview pairs in
26`desktop_window_registry.zig`. Page messages are authenticated by the sending
27WebKitGTK/WebView2 view; names select a target only after that sender is known.
28Replies go back to the requesting view, and lifecycle events go to the changed
29window plus its named handle's creator. Linux must register its `GtkApplication`
30before constructing the first `GtkApplicationWindow`, because the CLI creates
31that window before entering `g_application_run()`.
2432
2533## Per-window state
2634
@@ -118,13 +126,12 @@ The next multi-window milestone needs product decisions in addition to code:
118126 application-level asynchronous source above.
1191273. Decide whether Touch Bar should stay primary-window scoped or follow the
120128 key window with separately owned item definitions and callbacks.
1214. Bring Linux and Windows runtime creation, stable handles, sender routing,
122 lifecycle events and destroy semantics up to the macOS contract. Their
123 platform backends can create native windows today, but the TypeScript bridge
124 deliberately reports runtime creation as unsupported outside macOS.
1255. Add platform-native integration coverage. Source conformance and pure state
126 tests defend macOS invariants without requiring a GUI runner, but they do
127 not substitute for real Windows WebView2 and Linux WebKitGTK lifecycle tests.
1294. Finish advanced Linux/Windows window actions and clarify which are portable
130 versus platform-specific, without conflating close semantics: macOS retains
131 a closed page, while Linux and Windows release it.
1325. Complete platform-native integration coverage. Linux WebKitGTK runs under
133 Xvfb in CI; Windows currently has a cross-build but still needs a real
134 WebView2 lifecycle smoke on a capable Windows runner.
128135
129136## Review checklist
130137
packages/zig/src/linux.zigmodified+23-2
Changes to packages/zig/src/linux.zig
@@ -16,7 +16,9 @@ const window_reply_target = ;
1616// GTK and WebKit C bindings
1717pub extern "c" fn gtk_init(argc: ?*c_int, argv: ?*anyopaque) void;
1818pub extern "c" fn gtk_application_new(application_id: [*:0]const u8, flags: c_int) ?*anyopaque;
19pub extern "c" fn g_application_register(app: *anyopaque, cancellable: ?*anyopaque, err: ?*?*anyopaque) c_int;
1920pub extern "c" fn g_application_run(app: *anyopaque, argc: c_int, argv: [*c][*c]u8) c_int;
21pub extern "c" fn g_object_unref(object: *anyopaque) void;
2022pub extern "c" fn gtk_application_window_new(app: *anyopaque) *anyopaque;
2123pub extern "c" fn gtk_window_set_title(window: *anyopaque, title: [*:0]const u8) void;
2224pub extern "c" fn gtk_window_set_default_size(window: *anyopaque, width: c_int, height: c_int) void;
@@ -517,6 +519,13 @@ fn handleWindowAction(action: []const u8, data: ?[]const u8) !void {
517519// Application state
518520var app_instance: ?*anyopaque = null;
519521
522fn onApplicationActivated(_: *anyopaque, _: ?*anyopaque) callconv(.c) void {
523 // The CLI creates and shows its first window before g_application_run().
524 // Activation presents that already-owned window instead of constructing
525 // another one.
526 if (desktop_windows.latest()) |entry| gtk_window_present(@ptrFromInt(entry.window));
527}
528
520529pub const WindowStyle = struct {
521530 frameless: bool = false,
522531 transparent: bool = false,
@@ -552,7 +561,19 @@ pub const Window = struct {
552561 // Initialize GTK if not already done
553562 if (app_instance == null) {
554563 gtk_init(null, null);
555 app_instance = gtk_application_new("com.craft.app", 0);
564 const app = gtk_application_new("com.craft.app", 0) orelse return error.ApplicationCreationFailed;
565 if (g_signal_connect_data(app, "activate", @ptrCast(&onApplicationActivated), null, null, 0) == 0) {
566 g_object_unref(app);
567 return error.SignalConnectionFailed;
568 }
569 // GtkApplicationWindow must not be added before the application's
570 // startup signal. Register now because Craft creates its first
571 // window before entering g_application_run().
572 if (g_application_register(app, null, null) == 0) {
573 g_object_unref(app);
574 return error.ApplicationRegistrationFailed;
575 }
576 app_instance = app;
556577 }
557578
558579 const window = gtk_application_window_new(app_instance.?);
@@ -758,7 +779,7 @@ pub const Window = struct {
758779pub const App = struct {
759780 pub fn run() !void {
760781 if (app_instance) |app| {
761 _ = g_application_run(app, 0, undefined);
782 _ = g_application_run(app, 0, null);
762783 }
763784 }
764785
scripts/verify-linux-multi-window.tsmodified+3-1
Changes to scripts/verify-linux-multi-window.ts
@@ -90,6 +90,7 @@ const childPage = `<!doctype html><script>
9090</script>`
9191
9292const steps = new Set<string>()
93const requests: string[] = []
9394let resolveDone!: () => void
9495let rejectDone!: (error: Error) => void
9596const done = new Promise<void>((resolve, reject) => {
@@ -98,6 +99,7 @@ const done = new Promise<void>((resolve, reject) => {
9899})
99100const server = createServer((request, response) => {
100101 const url = new URL(request.url ?? '/', 'http://127.0.0.1')
102 requests.push(url.pathname)
101103 if (url.pathname === '/main' || url.pathname === '/child') {
102104 response.setHeader('content-type', 'text/html; charset=utf-8')
103105 response.end(url.pathname === '/main' ? mainPage : childPage)
@@ -145,7 +147,7 @@ try {
145147 console.log(`Linux multi-window smoke passed: ${[...steps].join(', ')}`)
146148}
147149catch (error) {
148 throw new Error(`${String(error)}\nCraft output:\n${output}`)
150 throw new Error(`${String(error)}\nHTTP requests: ${requests.join(', ')}\nCraft output:\n${output}`)
149151}
150152finally {
151153 clearTimeout(timer)