ReviewOS

also looking at this

craft-native/craft

feat(lifecycle): an app delegate, so closing the window is not the end

#73
Merged glennmichael123 wants to merge feat/app-delegate into main
9 files +497 -2
packages/typescript/src/__tests__/keep-running.test.tsadded+53-0
Changes to packages/typescript/src/__tests__/keep-running.test.ts
@@ -0,0 +1,53 @@
1/**
2 * `keepRunning` — whether the app outlives its last window (craft-native/craft#63).
3 *
4 * The flag is tri-state on purpose. Unset is not the same as `false`: unset
5 * lets craft decide from the shape of the app (a tray or menubar-only app
6 * stays, a windowed app quits), while `false` is an app saying it wants to go
7 * away with its window even though it has a tray. Collapsing the two would
8 * make the setting unable to express the case most likely to need it.
9 */
10
11import type { AppConfig } from '../types'
12import { describe, expect, it } from 'bun:test'
13
14async function argsFor(config: AppConfig): Promise<string[]> {
15 const { CraftApp } = await import('../index')
16 const app = new (CraftApp as unknown as { new (c: AppConfig): { buildArgs: () => string[] } })(config)
17 return (app as unknown as { buildArgs: () => string[] }).buildArgs()
18}
19
20describe('keepRunning', () => {
21 it('passes --keep-running when the app asks to stay', async () => {
22 expect(await argsFor({ window: { keepRunning: true } })).toContain('--keep-running')
23 })
24
25 it('passes --quit-on-close when the app asks to go', async () => {
26 const args = await argsFor({ window: { systemTray: true, keepRunning: false } })
27 expect(args).toContain('--quit-on-close')
28 expect(args).not.toContain('--keep-running')
29 })
30
31 it('passes neither when the app has not said', async () => {
32 // Craft's own default then applies, which is the point of leaving it unset.
33 const args = await argsFor({ window: { width: 900 } })
34 expect(args).not.toContain('--keep-running')
35 expect(args).not.toContain('--quit-on-close')
36 })
37
38 it('does not confuse an unset value with false', async () => {
39 // `if (x)` would send nothing for false; `if (x !== undefined)` would send
40 // --quit-on-close for unset. Both are wrong and both are easy to write.
41 const unset = await argsFor({ window: { systemTray: true } })
42 const explicitFalse = await argsFor({ window: { systemTray: true, keepRunning: false } })
43 expect(unset).not.toContain('--quit-on-close')
44 expect(explicitFalse).toContain('--quit-on-close')
45 })
46
47 it('leaves the rest of the window config alone', async () => {
48 const args = await argsFor({ window: { keepRunning: true, title: 'T', width: 900 } })
49 expect(args).toContain('--keep-running')
50 expect(args).toContain('--title')
51 expect(args).toContain('T')
52 })
53})
packages/typescript/src/index.tsmodified+7-0
Changes to packages/typescript/src/index.ts
@@ -429,6 +429,13 @@ export class CraftApp {
429429 args.push('--titlebar-hidden')
430430 if (window?.headless)
431431 args.push('--headless')
432 // Tri-state: unset means craft decides from the shape of the app, so
433 // neither flag is passed. `false` has to be passed explicitly, or a tray
434 // app could never be told to quit with its window.
435 if (window?.keepRunning === true)
436 args.push('--keep-running')
437 else if (window?.keepRunning === false)
438 args.push('--quit-on-close')
432439 if (window?.webSidebarMaterial) {
433440 args.push('--web-sidebar-material')
434441 if (window?.webSidebarWidth)
packages/typescript/src/types.tsmodified+17-0
Changes to packages/typescript/src/types.ts
@@ -76,6 +76,23 @@ export interface WindowOptions {
7676 */
7777 headless?: boolean
7878
79 /**
80 * Keep the process running after the last window closes (macOS).
81 *
82 * Craft decides this from the shape of the app when it is not set: an app
83 * with a tray icon or in menubar-only mode stays running, because outliving
84 * its window is what those are for; an ordinary windowed app quits.
85 *
86 * Set it to `true` for a windowed app that should stay resident and be
87 * brought back by clicking the Dock icon, or `false` for a tray app that
88 * should genuinely go away when its window closes.
89 *
90 * Before this existed the answer was AppKit's default, `false`, for every
91 * app: closing the last window left a process with no window and no way to
92 * get one back.
93 */
94 keepRunning?: boolean
95
7996 /**
8097 * Remember this window's size and position across launches, under this name
8198 * (macOS).
packages/zig/build.zigmodified+28-0
Changes to packages/zig/build.zig
@@ -650,6 +650,16 @@ pub fn build(b: *std.Build) void {
650650 }),
651651 });
652652
653 // Whether closing the last window quits the app. Pure rule, exhaustively
654 // tested, so the default and its opt-out are stated once.
655 const lifecycle_policy_tests = b.addTest(.{
656 .root_module = b.createModule(.{
657 .root_source_file = b.path("src/lifecycle_policy.zig"),
658 .target = target,
659 .optimize = optimize,
660 }),
661 });
662
653663 // The reload budget that brings a window back after WebKit's content
654664 // process dies. Pure, and takes its clock as an argument, so a crash loop
655665 // is testable without crashing anything.
@@ -670,6 +680,20 @@ pub fn build(b: *std.Build) void {
670680 .optimize = optimize,
671681 }),
672682 });
683 // The CLI's own tests. `src/cli.zig` was only ever built as a *module*, so
684 // the tests inside it — flag parsing, and the manifest keys those flags
685 // mirror — had no artifact to run in and never ran at all.
686 const cli_unit_tests = b.addTest(.{
687 .root_module = b.createModule(.{
688 .root_source_file = b.path("src/cli.zig"),
689 .target = target,
690 .optimize = optimize,
691 .imports = &.{
692 .{ .name = "build_options", .module = build_options.createModule() },
693 },
694 }),
695 });
696 cli_unit_tests.root_module.link_libc = true;
673697
674698 // The conformance test: what craft declares, against what it dispatches.
675699 const capability_conformance_tests = b.addTest(.{
@@ -1045,6 +1069,8 @@ pub fn build(b: *std.Build) void {
10451069 const run_external_link_tests = b.addRunArtifact(external_link_tests);
10461070 const run_webview_recovery_tests = b.addRunArtifact(webview_recovery_tests);
10471071 const run_request_context_tests = b.addRunArtifact(request_context_tests);
1072 const run_cli_unit_tests = b.addRunArtifact(cli_unit_tests);
1073 const run_lifecycle_policy_tests = b.addRunArtifact(lifecycle_policy_tests);
10481074 const run_prefs_tests = b.addRunArtifact(prefs_tests);
10491075 const run_prefs_macos_tests = b.addRunArtifact(prefs_macos_tests);
10501076 const run_key_codes_tests = b.addRunArtifact(key_codes_tests);
@@ -1161,6 +1187,8 @@ pub fn build(b: *std.Build) void {
11611187 test_step.dependOn(&run_external_link_tests.step);
11621188 test_step.dependOn(&run_webview_recovery_tests.step);
11631189 test_step.dependOn(&run_request_context_tests.step);
1190 test_step.dependOn(&run_cli_unit_tests.step);
1191 test_step.dependOn(&run_lifecycle_policy_tests.step);
11641192 // macOS only: the registry describes the dispatch chain in macos.zig, and
11651193 // compiling it elsewhere drags the whole native graph into a test binary
11661194 // for a platform it does not describe.
packages/zig/src/cli.zigmodified+78-0
Changes to packages/zig/src/cli.zig
@@ -50,6 +50,11 @@ pub const WindowOptions = struct {
5050 // — so every app launched through the shared `craft` binary called itself
5151 // "craft". macOS only.
5252 app_name: ?[]const u8 = null,
53 // Keep the process alive after the last window closes, instead of quitting.
54 // Null means craft decides from the shape of the app: a tray or
55 // menubar-only app stays, an ordinary windowed app goes. See
56 // `lifecycle_policy.zig`.
57 keep_running: ?bool = null,
5358 // Name AppKit remembers this window's size and position under, so the app
5459 // opens where the user last left it. Null means it forgets every launch.
5560 frame_autosave: ?[]const u8 = null,
@@ -165,6 +170,7 @@ const BundleConfig = struct {
165170 menubarOnly: ?bool = null,
166171 titlebarHidden: ?bool = null,
167172 headless: ?bool = null,
173 keepRunning: ?bool = null,
168174};
169175
170176/// Absolute path of the running executable's directory.
@@ -264,6 +270,7 @@ fn applyScalarConfig(cfg: BundleConfig, options: *WindowOptions) void {
264270 if (cfg.hideDockIcon) |v| options.hide_dock_icon = v;
265271 if (cfg.titlebarHidden) |v| options.titlebar_hidden = v;
266272 if (cfg.headless) |v| options.headless = v;
273 if (cfg.keepRunning) |v| options.keep_running = v;
267274 // A menubar app with a dock icon and no tray is not a menubar app, so the
268275 // one flag sets all three rather than making every manifest repeat them.
269276 if (cfg.menubarOnly) |v| {
@@ -490,6 +497,13 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Wind
490497 options.hide_dock_icon = true;
491498 } else if (std.mem.eql(u8, arg, "--headless")) {
492499 options.headless = true;
500 } else if (std.mem.eql(u8, arg, "--keep-running")) {
501 options.keep_running = true;
502 } else if (std.mem.eql(u8, arg, "--quit-on-close")) {
503 // The other direction, for a tray app that does want to go away
504 // with its window. Without it, `--keep-running` would be a flag
505 // whose default cannot be restored from the command line.
506 options.keep_running = false;
493507 } else if (std.mem.eql(u8, arg, "--menubar-only")) {
494508 options.menubar_only = true;
495509 options.system_tray = true; // Menubar-only implies system tray
@@ -592,6 +606,11 @@ fn printHelp() void {
592606 \\ --system-tray Show system tray icon
593607 \\ --hide-dock-icon Hide dock icon (menubar-only mode, macOS)
594608 \\ --menubar-only Menubar-only mode (no window, system tray only)
609 \\ --keep-running Stay running after the last window closes.
610 \\ The default for tray and menubar-only apps;
611 \\ an ordinary windowed app quits.
612 \\ --quit-on-close Quit when the last window closes, even for
613 \\ a tray app.
595614 \\ --dev-tools Enable WebKit DevTools and make the webview
596615 \\ inspectable from Safari's Develop menu
597616 \\ --no-devtools Disable WebKit DevTools
@@ -691,3 +710,62 @@ pub fn formatVersion(buffer: []u8, version: []const u8, platform_name: []const u
691710 .{ version, platform_name },
692711 );
693712}
713
714test "a manifest can ask the app to outlive its window" {
715 const source =
716 \\{"title":"Tray thing","keepRunning":true}
717 ;
718 const parsed = try std.json.parseFromSlice(BundleConfig, std.testing.allocator, source, .{
719 .ignore_unknown_fields = true,
720 .allocate = .alloc_always,
721 });
722 defer parsed.deinit();
723
724 var options = WindowOptions{};
725 // Null before, so the shape decides. That distinction is the whole point
726 // of the optional: `false` is "quit", not "nobody said".
727 try std.testing.expectEqual(@as(?bool, null), options.keep_running);
728 applyScalarConfig(parsed.value, &options);
729 try std.testing.expectEqual(@as(?bool, true), options.keep_running);
730}
731
732test "a manifest that says nothing about it leaves the decision to the shape" {
733 const source =
734 \\{"title":"Plain"}
735 ;
736 const parsed = try std.json.parseFromSlice(BundleConfig, std.testing.allocator, source, .{
737 .ignore_unknown_fields = true,
738 .allocate = .alloc_always,
739 });
740 defer parsed.deinit();
741
742 var options = WindowOptions{};
743 applyScalarConfig(parsed.value, &options);
744 try std.testing.expectEqual(@as(?bool, null), options.keep_running);
745}
746
747test "--keep-running and --quit-on-close both say so explicitly" {
748 {
749 var args = [_][:0]const u8{ "craft", "--keep-running" };
750 const options = try parseArgs(std.testing.allocator, &args);
751 try std.testing.expectEqual(@as(?bool, true), options.keep_running);
752 }
753 {
754 var args = [_][:0]const u8{ "craft", "--quit-on-close" };
755 const options = try parseArgs(std.testing.allocator, &args);
756 // Not null: a tray app passing this has said something, and the shape
757 // must not overrule it.
758 try std.testing.expectEqual(@as(?bool, false), options.keep_running);
759 }
760 {
761 var args = [_][:0]const u8{"craft"};
762 const options = try parseArgs(std.testing.allocator, &args);
763 try std.testing.expectEqual(@as(?bool, null), options.keep_running);
764 }
765}
766
767test "the last of the two flags wins" {
768 var args = [_][:0]const u8{ "craft", "--keep-running", "--quit-on-close" };
769 const options = try parseArgs(std.testing.allocator, &args);
770 try std.testing.expectEqual(@as(?bool, false), options.keep_running);
771}
packages/zig/src/lifecycle_policy.zigadded+93-0
Changes to packages/zig/src/lifecycle_policy.zig
@@ -0,0 +1,93 @@
1//! Whether closing the last window should quit the app.
2//!
3//! With no `NSApplicationDelegate`, AppKit answers this NO, and craft never
4//! installed one: closing the last window left a process running with no
5//! window, no Dock response, and no way to get either back. Force-quit was the
6//! only exit.
7//!
8//! YES is right for an ordinary windowed app and wrong for the apps that exist
9//! precisely to outlive their window — a menubar app, or anything with a tray
10//! icon. Those close their window and carry on, which is the feature.
11//!
12//! So the rule has to be derived rather than fixed, and the derivation lives
13//! here, once, instead of as a boolean expression at the call site that
14//! nothing checks.
15
16const std = ;
17
18/// What the app looks like, as far as this decision is concerned.
19pub const Shape = struct {
20 /// A status-bar item: the app has somewhere to live without a window.
21 has_tray: bool = false,
22 /// No window at all, only a menubar item.
23 menubar_only: bool = false,
24 /// `--keep-running` or `keepRunning` in the manifest. Null means the app
25 /// did not say, and the shape decides.
26 explicit_keep_running: ?bool = null,
27};
28
29/// True when the app should quit once its last window closes.
30pub fn quitOnLastWindowClosed(shape: Shape) bool {
31 // An explicit answer is an answer. A tray app that wants to quit with its
32 // window is unusual but not wrong, and second-guessing it here would make
33 // the flag a lie for exactly the apps most likely to set it.
34 if (shape.explicit_keep_running) |keep| return !keep;
35
36 // Anything with a place to live without a window keeps living.
37 return !(shape.has_tray or shape.menubar_only);
38}
39
40const testing = std.testing;
41
42test "an ordinary windowed app quits with its window" {
43 // The bug this fixes: before, this returned NO by AppKit default and the
44 // process was stranded.
45 try testing.expect(quitOnLastWindowClosed(.{}));
46}
47
48test "an app with a tray icon outlives its window" {
49 try testing.expect(!quitOnLastWindowClosed(.{ .has_tray = true }));
50}
51
52test "a menubar-only app outlives its window" {
53 try testing.expect(!quitOnLastWindowClosed(.{ .menubar_only = true }));
54 // `--menubar-only` implies a tray in the CLI; neither alone should change
55 // the answer, and together they must not cancel out.
56 try testing.expect(!quitOnLastWindowClosed(.{ .menubar_only = true, .has_tray = true }));
57}
58
59test "asking to keep running overrides the shape" {
60 try testing.expect(!quitOnLastWindowClosed(.{ .explicit_keep_running = true }));
61}
62
63test "asking to quit overrides the shape too" {
64 // A tray app that genuinely wants to go away with its window. Unusual, but
65 // the flag has to mean what it says or it is worse than not having it.
66 try testing.expect(quitOnLastWindowClosed(.{
67 .has_tray = true,
68 .explicit_keep_running = false,
69 }));
70 try testing.expect(quitOnLastWindowClosed(.{
71 .menubar_only = true,
72 .has_tray = true,
73 .explicit_keep_running = false,
74 }));
75}
76
77test "every shape has an answer" {
78 // Exhaustive over the whole input space, so no combination is left to
79 // whatever the last `if` happened to fall through to.
80 for ([_]bool{ false, true }) |tray| {
81 for ([_]bool{ false, true }) |menubar| {
82 for ([_]?bool{ null, false, true }) |explicit| {
83 const shape: Shape = .{
84 .has_tray = tray,
85 .menubar_only = menubar,
86 .explicit_keep_running = explicit,
87 };
88 const expected = if (explicit) |keep| !keep else !(tray or menubar);
89 try testing.expectEqual(expected, quitOnLastWindowClosed(shape));
90 }
91 }
92 }
93}
packages/zig/src/macos.zigmodified+201-1
Changes to packages/zig/src/macos.zig
@@ -651,6 +651,7 @@ pub fn createWindowWithStyle(title: []const u8, width: u32, height: u32, html: ?
651651 // Allocate and initialize window
652652 const window_alloc = msgSend0(WindowClass, "alloc");
653653 const window = msgSend4(window_alloc, "initWithContentRect:styleMask:backing:defer:", frame, styleMask, backing, defer_flag);
654 keepWindowAfterClose(window);
654655
655656 // Create title NSString
656657 const title_cstr = try .dupeZ(std.heap.c_allocator, u8, title);
@@ -1962,6 +1963,7 @@ pub fn createWindowWithSidebar(
19621963 // Create window
19631964 const window_alloc = msgSend0(NSWindow, "alloc");
19641965 const window = msgSend4(window_alloc, "initWithContentRect:styleMask:backing:defer:", frame, styleMask, backing, defer_flag);
1966 keepWindowAfterClose(window);
19651967
19661968 // Set window title
19671969 const title_cstr = try .dupeZ(std.heap.c_allocator, u8, title);
@@ -3077,6 +3079,7 @@ pub fn createWindowWithSidebarURL(
30773079
30783080 const window_alloc = msgSend0(NSWindow, "alloc");
30793081 const window = msgSend4(window_alloc, "initWithContentRect:styleMask:backing:defer:", frame, styleMask, backing, defer_flag);
3082 keepWindowAfterClose(window);
30803083
30813084 // Set window title
30823085 const title_str = createNSString(title);
@@ -3657,8 +3660,22 @@ pub fn showNotification(title: []const u8, message: []const u8) !void {
36573660}
36583661
36593662// Hot reload - reload URL in webview
3663/// Reload, as `craft.window.reload()` and the Reload menu item mean it.
3664///
3665/// Not `-reload:`, which is wrong for half of craft's windows for the same
3666/// reason it was wrong as crash recovery: HTML loaded through
3667/// `loadHTMLString:baseURL:` has no request to repeat, so reloading navigates
3668/// to the synthetic base URL — `http://localhost/`, where nothing is
3669/// listening. An app started from `--html` reloaded itself into a connection
3670/// error. `restoreContent` re-applies what was actually loaded, and falls back
3671/// to `-reload` for a webview craft did not load.
3672///
3673/// Reloading also ends the crash burst. A person clicking Reload is not a
3674/// crash loop, and leaving the budget spent would mean the next renderer
3675/// crash went straight to the give-up page.
36603676pub fn reloadWindow(webview: objc.id) void {
3661 msgSendVoid0(webview, "reload:");
3677 webview_recovery.forget(@intFromPtr(webview));
3678 restoreContent(webview);
36623679}
36633680
36643681pub fn reloadWindowIgnoringCache(webview: objc.id) void {
@@ -6372,11 +6389,189 @@ pub const Installer = struct {
63726389 }
63736390};
63746391
6392// =============================================================================
6393// NSApplicationDelegate — app-level lifecycle
6394// =============================================================================
6395
6396/// Keep an `NSWindow` alive after it is closed.
6397///
6398/// `releasedWhenClosed` defaults to **YES** for a window created with
6399/// `initWithContentRect:`, and that default is what made "reopen the window"
6400/// impossible rather than merely unimplemented: closing the window
6401/// deallocated it, so there was nothing left to show and `[NSApp windows]` no
6402/// longer listed it. Reopening a released window is a use-after-free, not a
6403/// missing feature.
6404///
6405/// Measured with the flag off: the window, its `WKWebView`, the DOM and the
6406/// JavaScript context all survive a close — a value assigned to `window` before
6407/// closing was still there after reopening. So reopen restores the app as the
6408/// user left it rather than reloading it.
6409///
6410/// The cost is that a closed window is never freed. Craft opens one; when #67
6411/// opens many, whatever closes them for real will need to release them, and
6412/// this is the line it will have to pair with.
6413fn keepWindowAfterClose(window: objc.id) void {
6414 _ = msgSend1(window, "setReleasedWhenClosed:", false);
6415 rememberCraftWindow(window);
6416}
6417
6418/// Every window craft opened, so reopen can tell them from AppKit's own.
6419///
6420/// Craft opens one today; #67 opens more. Sixteen is far past any real app and
6421/// bounded so this needs no allocator on a path that runs during window
6422/// creation.
6423var craft_windows: [16]objc.id = @splat(null);
6424
6425fn rememberCraftWindow(window: objc.id) void {
6426 if (@intFromPtr(window) == 0) return;
6427 for (&craft_windows) |*slot| {
6428 if (slot.* == window) return;
6429 if (@intFromPtr(slot.*) == 0) {
6430 slot.* = window;
6431 return;
6432 }
6433 }
6434 // Full. Reopen will not restore this window, which is a worse outcome than
6435 // the table being bigger — say so rather than failing silently.
6436 std.log.warn("more than {d} windows; the newest will not be restored by a Dock click", .{craft_windows.len});
6437}
6438
6439/// Whether the app should quit when its last window closes.
6440///
6441/// Set from the parsed options before the run loop starts; read by the
6442/// delegate each time AppKit asks. A variable rather than a compile-time
6443/// answer because the delegate is installed before the options that decide it
6444/// have been applied — the delegate has to exist early enough for #65's
6445/// notification callbacks, which arrive before anything else runs.
6446var quit_on_last_window_closed: bool = true;
6447
6448/// Tell the delegate what to answer. See `lifecycle_policy.zig` for the rule.
6449pub fn setQuitOnLastWindowClosed(value: bool) void {
6450 quit_on_last_window_closed = value;
6451}
6452
6453/// `applicationShouldTerminateAfterLastWindowClosed:`
6454///
6455/// Without a delegate AppKit answers NO, which for a plain windowed app means
6456/// closing the window strands the process: no window, no Dock icon response,
6457/// and force-quit as the only way out.
6458fn appShouldTerminateAfterLastWindowClosed(_: objc.id, _: objc.SEL, _: objc.id) callconv(.c) bool {
6459 return quit_on_last_window_closed;
6460}
6461
6462/// `applicationShouldHandleReopen:hasVisibleWindows:`
6463///
6464/// Sent when the Dock icon is clicked, or the app is reopened while already
6465/// running. Craft implemented nothing, so the click did nothing.
6466///
6467/// Only acts when there is nothing visible — with a window already up, AppKit's
6468/// own behaviour (bring it forward) is the right one and returning true lets it
6469/// happen.
6470fn appShouldHandleReopen(_: objc.id, _: objc.SEL, app: objc.id, has_visible: bool) callconv(.c) bool {
6471 if (has_visible) return true;
6472
6473 const windows = msgSend0(app, "windows");
6474 if (@intFromPtr(windows) == 0) return true;
6475
6476 const countFn = @as(
6477 *const fn (objc.id, objc.SEL) callconv(.c) c_ulong,
6478 @ptrCast(&objc.objc_msgSend),
6479 );
6480 const count = countFn(windows, sel("count"));
6481
6482 var i: c_ulong = 0;
6483 while (i < count) : (i += 1) {
6484 const window = msgSend1(windows, "objectAtIndex:", i);
6485 // `[NSApp windows]` includes panels and the offscreen windows AppKit
6486 // keeps for menus and tooltips. Ordering one of those front puts an
6487 // empty frame on screen, so only windows craft made are reopened.
6488 if (!isCraftWindow(window)) continue;
6489 _ = msgSend1(window, "makeKeyAndOrderFront:", @as(objc.id, null));
6490 }
6491
6492 // Reopening without activating leaves the window behind whatever the user
6493 // clicked away to, which is not what clicking a Dock icon means.
6494 _ = msgSend1(app, "activateIgnoringOtherApps:", true);
6495 return true;
6496}
6497
6498/// Whether this is a window craft opened, rather than one AppKit keeps for its
6499/// own purposes.
6500///
6501/// Answered from the list of windows craft actually created, not by inspecting
6502/// one. The first version of this asked whether the content view *was* a
6503/// `WKWebView`, which is true only of the plain window: `--web-sidebar-material`
6504/// installs a backdrop container (macos.zig:959), `createWindowWithSidebar` sets
6505/// a split view controller (:2214), and `createWindowWithSidebarURL` installs
6506/// its own container (:3363). All three still keep their closed window alive, so
6507/// the reopen loop found them and skipped them — a Dock click on a sidebar app
6508/// activated the process and restored no window, which is the exact dead end
6509/// this delegate exists to remove.
6510///
6511/// A test cannot see that. It needs a real reopen event against a sidebar
6512/// window, so the fix is to stop inferring the answer at all.
6513fn isCraftWindow(window: objc.id) bool {
6514 if (@intFromPtr(window) == 0) return false;
6515 for (craft_windows) |known| {
6516 if (known == window) return true;
6517 }
6518 return false;
6519}
6520
6521var app_delegate_installed = false;
6522
6523/// Install craft's `NSApplicationDelegate`.
6524///
6525/// Called from both startup paths, and before `finishLaunching` on the tray
6526/// path, because a delegate installed after launch has already missed the
6527/// callbacks that only fire during it.
6528pub fn installAppDelegate() void {
6529 if (app_delegate_installed) return;
6530
6531 const className = "CraftAppDelegate";
6532 var cls = objc.objc_getClass(className);
6533 if (cls == null) {
6534 cls = objc.objc_allocateClassPair(getClass("NSObject"), className, 0);
6535 if (cls == null) return;
6536
6537 // BOOL return, so "B" rather than "c": on arm64 a BOOL is a real
6538 // `bool` and the two are not interchangeable in a method signature.
6539 _ = objc.class_addMethod(
6540 cls,
6541 sel("applicationShouldTerminateAfterLastWindowClosed:"),
6542 @as(objc.IMP, @ptrCast(@constCast(&appShouldTerminateAfterLastWindowClosed))),
6543 "B@:@",
6544 );
6545 _ = objc.class_addMethod(
6546 cls,
6547 sel("applicationShouldHandleReopen:hasVisibleWindows:"),
6548 @as(objc.IMP, @ptrCast(@constCast(&appShouldHandleReopen))),
6549 "B@:@B",
6550 );
6551
6552 objc.objc_registerClassPair(cls);
6553 }
6554
6555 const app = msgSend0(getClass("NSApplication"), "sharedApplication");
6556 // `delegate` is a weak reference, so the +1 from alloc/init is what keeps
6557 // this alive. One object for the life of the process, deliberately never
6558 // released: freeing it would leave NSApp pointing at freed memory the next
6559 // time the Dock icon is clicked.
6560 const delegate = msgSend0(msgSend0(@as(objc.id, @ptrCast(@alignCast(cls))), "alloc"), "init");
6561 msgSendVoid1(app, "setDelegate:", delegate);
6562 app_delegate_installed = true;
6563
6564 if (comptime builtin.mode == .debug)
6565 std.debug.print("[App] Installed NSApplicationDelegate\n", .{});
6566}
6567
63756568/// Initialize the NSApplication for regular apps (with Dock icon)
63766569pub fn initApp() void {
63776570 const NSApplication = getClass("NSApplication");
63786571 const app = msgSend0(NSApplication, "sharedApplication");
63796572
6573 installAppDelegate();
6574
63806575 const NSApplicationActivationPolicyRegular: c_long = 0;
63816576 _ = msgSend1(app, "setActivationPolicy:", NSApplicationActivationPolicyRegular);
63826577}
@@ -6386,6 +6581,7 @@ pub fn initAppWithoutLaunching() void {
63866581 const NSApplication = getClass("NSApplication");
63876582 const app = msgSend0(NSApplication, "sharedApplication");
63886583 _ = app; // Just ensure app exists
6584 installAppDelegate();
63896585}
63906586
63916587/// Set the dock icon for the running NSApplication. `path` may point to any
@@ -6705,6 +6901,10 @@ pub fn initAppForTray() void {
67056901 const NSApplicationActivationPolicyAccessory: c_long = 1;
67066902 _ = msgSend1(app, "setActivationPolicy:", NSApplicationActivationPolicyAccessory);
67076903
6904 // Before `finishLaunching`, not after: a delegate installed afterwards has
6905 // already missed every callback that only fires during launch.
6906 installAppDelegate();
6907
67086908 // MUST call finishLaunching BEFORE creating status bar items!
67096909 // This is the key - the working test does it in this order
67106910 msgSendVoid0(app, "finishLaunching");
packages/zig/src/minimal.zigmodified+13-0
Changes to packages/zig/src/minimal.zig
@@ -2,6 +2,7 @@ const std = ;
22const builtin = ;
33const craft = ;
44const cli = ;
5const lifecycle_policy = ;
56const io_context = craft.io_context;
67// Reached through the craft module: a file cannot belong to both `root` and
78// `craft`, and macos.zig already owns it there.
@@ -56,6 +57,18 @@ pub fn main(init: std.process.Init) !void {
5657 std.process.exit(1);
5758 }
5859
60 // Whether closing the last window ends the process. Decided here, before
61 // either startup path, because both need the same answer and only this
62 // scope knows the shape of the app. See `lifecycle_policy.zig` for the
63 // rule; the delegate that reads it is installed a moment later.
64 if (builtin.os.tag == .macos) {
65 craft.macos.setQuitOnLastWindowClosed(lifecycle_policy.quitOnLastWindowClosed(.{
66 .has_tray = options.system_tray,
67 .menubar_only = options.menubar_only,
68 .explicit_keep_running = options.keep_running,
69 }));
70 }
71
5972 // In benchmark mode, disable dev_tools for lower overhead
6073 const effective_dev_tools = if (options.benchmark) false else options.dev_tools;
6174
packages/zig/src/webview_recovery.zigmodified+7-1
Changes to packages/zig/src/webview_recovery.zig
@@ -151,7 +151,7 @@ pub const give_up_page =
151151 \\<main>
152152 \\<h1>This page stopped responding</h1>
153153 \\<p>It was reloaded a few times and kept failing, so it has been left alone.</p>
154 \\<button onclick="location.reload()">Reload</button>
154 \\<button onclick="if(window.craft&&window.craft.window)window.craft.window.reload();else location.reload()">Reload</button>
155155 \\</main>
156156;
157157
@@ -277,6 +277,12 @@ test "more windows than the table holds still each get tracked" {
277277test "the give-up page carries its own styling and a way back" {
278278 // It is shown when there is no server, no bundle path and possibly no
279279 // network, so anything it references it must contain.
280 // `location.reload()` alone cannot work here: the page is installed with
281 // `loadHTMLString:baseURL:` and a nil base, so reloading navigates to the
282 // base rather than re-rendering the substitute document. The button has to
283 // go through the bridge, which re-applies what the window actually had.
284 try testing.expect(std.mem.indexOf(u8, give_up_page, "window.craft.window.reload()") != null);
285 // The fallback still has to be there for a page served without the bridge.
280286 try testing.expect(std.mem.indexOf(u8, give_up_page, "location.reload()") != null);
281287 try testing.expect(std.mem.indexOf(u8, give_up_page, "<style>") != null);
282288 try testing.expect(std.mem.indexOf(u8, give_up_page, "prefers-color-scheme") != null);