macOS: fix popup over-releases (adopted tab close, native-window close) and free user-closed popup windows

demos/WebViewAdoptPopupDemo/README.md

@@ -32,6 +32,17 @@ and launches the demo.
     custom UA you set.
   - **window.open → popup** and the **target=_blank** link also open as tabs.
   - No native popup window appears at any point.
+  - **Teardown of an adopted tab** (Canvas 18 Op 10.3). Select an adopted
+    tab and click **Close tab**: the tab is removed and its
+    `WebViewComponent` is disposed, which destroys the native engine. The
+    button does nothing on the opener tab. Repeat several times with each
+    popup kind above. The app must keep running.
+  - **window.open → self-closing popup** opens an adopted tab with a
+    **window.close()** button. Click it (the page asks to close itself),
+    then close the tab with **Close tab**. Repeat several times.
+  - Run with `NSZombieEnabled=YES ./run-mac-adopt-popup-demo.sh` so a
+    message sent to a freed Objective-C object stops the app at once
+    instead of crashing it later.
 - **Native window** — the Canvas 15 behaviour: the popup opens in a separate
   native window (POST preserved, opener-linked).
 - **Block** — `window.open` returns `null`; the POST form submit is blocked.

demos/WebViewAdoptPopupDemo/src/ca/weblite/webview/demos/WebViewAdoptPopupDemo.java

@@ -140,13 +140,14 @@ private static void buildUi() {
         });
         opener.setUrl(openerPage());
 
-        frame.add(buildControls(opener), java.awt.BorderLayout.NORTH);
+        frame.add(buildControls(opener, tabs), java.awt.BorderLayout.NORTH);
         addTab(tabs, "opener", opener);
         frame.add(tabs, java.awt.BorderLayout.CENTER);
         frame.setVisible(true);
     }
 
-    private static JPanel buildControls(WebViewComponent opener) {
+    private static JPanel buildControls(WebViewComponent opener,
+                                        JTabbedPane tabs) {
         // Canvas 21 Op 14.4: three rows, not one long FlowLayout line. A row
         // wider than the frame silently wraps its trailing components out of
         // view, and in a fixed-height row they become unreachable -- the
@@ -206,6 +207,25 @@ private static JPanel buildControls(WebViewComponent opener) {
         });
         bar.add(clearCache);
 
+        // Canvas 18 Op 10.3: close the selected ADOPTED tab and dispose it.
+        // Removing a heavyweight component from its parent does not destroy
+        // its native peer; dispose() is what reaches cocoa_destroy_engine, so
+        // this is the step that exercises an adopted child's teardown.  The
+        // opener tab is never closed by this button.
+        JButton closeTab = new JButton("Close tab");
+        closeTab.addActionListener(e -> {
+            java.awt.Component sel = tabs.getSelectedComponent();
+            if (sel == null || sel == opener
+                    || !(sel instanceof WebViewComponent)) {
+                System.out.println("[demo] Close tab: select an adopted tab");
+                return;
+            }
+            tabs.remove(sel);
+            ((WebViewComponent) sel).dispose();
+            System.out.println("[demo] closed and disposed an adopted tab");
+        });
+        bar.add(closeTab);
+
         // Row 2: the User-Agent field takes the slack, its buttons pin right,
         // so the row's width never depends on the field's preferred size.
         JPanel uaRow = fieldRow("User-Agent:", uaField, setUa, resetUa);
@@ -331,6 +351,15 @@ private static String openerPage() {
           + "window.open &rarr; popup</button>"
           + "<h3>3. target=_blank link</h3>"
           + "<a href='https://example.com' target='_blank'>open example.com</a>"
+          // Canvas 18 Op 10.3: a popup that closes itself, so the adopted
+          // child's webViewDidClose: path runs before its tab is closed.
+          + "<h3>4. window.open &rarr; page that closes itself</h3>"
+          + "<button onclick=\"var w=window.open('about:blank','selfclose');"
+          + "if(w){w.document.write('<!doctype html><body>"
+          + "<p>Adopted popup. Click to call window.close():</p>"
+          + "<button onclick=window.close()>window.close()</button></body>');"
+          + "w.document.close();}\">"
+          + "window.open &rarr; self-closing popup</button>"
           + "<script>document.getElementById('ua').textContent="
           + "navigator.userAgent;</script>"
           + "</body></html>";

spdd/prompt/15-20260802-1000-[Feat]-Browser-Initiated-Popups-Window-Open.md

@@ -444,7 +444,9 @@ WebViewPopupCallback ..> PopupDispatcher : delegates to
      (W/H default to 500×650 — a typical auth-popup size — when
      unspecified). Create an `NSWindow`
      (`NSWindowStyleMaskTitled|Closable|Resizable`, contentRect W×H),
-     set its `contentView` to `child`, `center`, and
+     **set `releasedWhenClosed` to NO** (see the "The library owns the
+     popup window" safeguard), set its `contentView` to `child`,
+     `center`, and
      `makeKeyAndOrderFront:`. Build a child `Engine` (webview=child,
      jvm, config=configuration, manager=child's
      `configuration.userContentController`, `popup_callback` and
@@ -459,7 +461,18 @@ WebViewPopupCallback ..> PopupDispatcher : delegates to
      `Engine`, `[popup_window close]`, fire
      `onPopupClosed(popupId, …)` async, then tear down the child
      engine (remove from `g_webview_map`, `setUIDelegate:nil`, delete
-     global refs, release window + delegate).
+     global refs, release window + delegate). Before `-close` it clears
+     the window's delegate, so its own close does not re-enter the
+     window-close path below.
+   - **The window's close button tears the popup down too.** The popup's
+     UI-delegate object also adopts `NSWindowDelegate` and is set as the
+     popup `NSWindow`'s delegate. Its `windowWillClose:` recovers the
+     engine through the `"eng"` association; for an engine-owned
+     native-window popup (not `java_owned`, `popup_window` set) it
+     schedules the `webViewDidClose:` teardown on the AppKit main queue,
+     retaining the delegate and the web view across the hop (a window
+     must not be released inside its own `windowWillClose:`). Java
+     receives `onPopupClosed` exactly as for `window.close()`.
 
 5. **Linux (WebKitGTK) signal strategy.** Connect three signals on
    every `WebKitWebView` created by `gtk_create_engine`
@@ -857,6 +870,26 @@ File: `test/ca/weblite/webview/PopupDispatcherTest.java`
 
 ## S · Safeguards
 
+- **A native popup is torn down once, however it closes (macOS).** A
+  page's `window.close()` and the window's own close button both reach
+  the single `webViewDidClose:` teardown, which clears the delegate's
+  `"eng"` association before anything else; whichever path runs second
+  recovers nil and does nothing. Without the close-button path, a
+  user-closed window (created with `releasedWhenClosed` = NO) left the
+  window, its web view, delegate, configuration, Java global refs and
+  `Engine` alive, and Java never heard `onPopupClosed`.
+
+- **The library owns the popup window (macOS).** The engine-owned
+  popup `NSWindow` is created by `alloc` (+1, held in `popup_window`)
+  and freed by the single `release` in the `webViewDidClose:` teardown.
+  It MUST therefore be created with `releasedWhenClosed` set to NO:
+  left at its default (YES), `-close` releases the window as well, and
+  the teardown's `release` is one too many. Observed on-device with
+  `NSZombieEnabled`: a native-window popup that calls `window.close()`
+  stopped with `-[NSKVONotifying_NSWindow release]: message sent to
+  deallocated instance` in `objc_autoreleasePoolPop` on the AppKit main
+  thread.
+
 - **Constructor null-checks.** `WebViewPopupEvent` and
   `PopupDispatcher` reject null `source` with an NPE named
   `"source"`. Other Strings coerce null → empty; sizes accept `-1`.

spdd/prompt/18-20260806-1200-[Feat]-Popup-Adoption-Into-Webviewcomponent-Tab.md

@@ -573,6 +573,12 @@ File: `src_c/webview_embed.cpp` (Cocoa)
    - `NATIVE_WINDOW` (1): exactly today's window path (create child,
      `NSWindow`, `makeKeyAndOrderFront:`, register child engine, fire
      `onPopupOpened`).
+   - **Both non-BLOCK branches**: the child `Engine` **retains** the
+     `configuration` WebKit passed before storing it as `child_e->config`
+     (see the "An engine owns every object it releases" safeguard). WebKit
+     only lends that object for the duration of the delegate call (+0); the
+     child `WKWebView` keeps its own copy, so the passed object is freed
+     soon after the call returns unless the engine holds a reference.
    - `ADOPT` (2): **isolate the child's script world first** — before
      `initWithFrame:configuration:`, replace the passed
      `configuration`'s `userContentController` with a **fresh, empty
@@ -603,8 +609,12 @@ File: `src_c/webview_embed.cpp` (Cocoa)
    remove from `g_retained_popups`; return the child engine handle.
 4. `cocoa_discard_popup(jlong popupId)`: look up + remove from
    `g_retained_popups`; `setUIDelegate:nil`; delete inherited global
-   refs; release the child view; delete the engine (mirror
+   refs; release the child view; **release the retained
+   `child_e->config`**; delete the engine (mirror
    `impl_web_view_did_close` teardown ordering, minus the window).
+   `impl_web_view_did_close`'s engine-owned path (not `java_owned`) likewise
+   releases the retained `config` before `delete e`; its `java_owned` path
+   leaves it for `cocoa_destroy_engine`, which releases it exactly once.
 5. JNI bridges
    `Java_ca_weblite_webview_WebViewNative_webview_1embed_1adopt_1popup`
    (resolve `parent`'s peer/`NSView` from the AWT `Component` via the
@@ -646,6 +656,21 @@ Files: `demos/WebViewAdoptPopupDemo/…` (or a mode on
    switches handler modes: Adopt (host in a `JFrame`-hosted tab/panel
    created on `popupAdoptable`), Native window (Canvas 15 allow),
    Block. Adopt mode logs POST-body round-trip to a console line.
+3. **Adopted-child teardown is testable by hand.** The controls carry a
+   **Close tab** button: when the selected tab is an adopted popup (never
+   the opener tab, which the button leaves alone), it removes that tab
+   from the `JTabbedPane` and calls `WebViewComponent.dispose()` on it —
+   removing a heavyweight component from its parent does not destroy its
+   native peer, so `dispose()` is the call that reaches
+   `cocoa_destroy_engine`. The opener page gains a fourth case,
+   **"window.open → page that closes itself"**: it opens a named popup on
+   `about:blank` and, through the returned window handle, writes into it a
+   button that calls `window.close()`, so the adopted child's
+   `webViewDidClose:` path runs before its tab is closed. The demo README's
+   checklist lists both steps (repeat each several times) and suggests
+   running with `NSZombieEnabled=YES` so a remaining over-release faults at
+   once. These steps verify the "An engine owns every object it releases"
+   and "An adopted child is freed exactly once" safeguards on macOS.
 
 ### 11. README + Tests
 Files: `README.md`, `test/ca/weblite/webview/PopupDispatcherTest.java`
@@ -749,6 +774,21 @@ Files: `README.md`, `test/ca/weblite/webview/PopupDispatcherTest.java`
   `"eng"` association before freeing, so a `window.close()` that
   drains after the engine is gone recovers `nil` and no-ops. The
   two paths are mutually exclusive on `delete`, never racing it.
+- **An engine owns every object it releases (no over-release on
+  destroy).** Every Objective-C object a teardown path releases
+  (`cocoa_destroy_engine`, `cocoa_discard_popup`, the engine-owned
+  `impl_web_view_did_close` path) MUST be held at +1 by that `Engine`.
+  A popup child's `config` is the `WKWebViewConfiguration` WebKit passes
+  to `webView:createWebViewWithConfiguration:forNavigationAction:
+  windowFeatures:` — a borrowed (+0) object WebKit frees soon after the
+  call — so the child engine retains it when storing it, and each
+  teardown path releases it exactly once. Storing it unretained let
+  `cocoa_destroy_engine` release a freed object when an adopted tab was
+  closed: `objc_msgSend` on a garbage isa on the AppKit main thread
+  (`EXC_BAD_ACCESS` at isa + 0x10), seen in the field after an Okta
+  dashboard tile opened Jira as an adopted tab. `child_e->manager` stays
+  a borrowed pointer: it is owned by the child `WKWebView`'s own
+  configuration and is never released by a teardown path.
 - **Opener linkage + POST are mandatory (AC1/AC2).** The adopted child
   MUST be the WebKit-created child from the passed configuration;
   creating a fresh view or re-navigating from Java is a defect.

src_c/webview_embed.cpp

@@ -6764,6 +6764,12 @@ static id impl_create_web_view(id self, SEL, id webView, id configuration,
         win = msg<id, CGRect, unsigned long, unsigned long, BOOL>(
             win, sel("initWithContentRect:styleMask:backing:defer:"),
             CGRectMake(0, 0, W, H), (unsigned long)15, (unsigned long)2, NO);
+        // Canvas 15 safeguard "The library owns the popup window": this alloc
+        // is balanced by the single release in impl_web_view_did_close.  Left
+        // at its default (YES), -close would release the window too, and that
+        // release would be one too many (NSZombie: -[NSWindow release]: message
+        // sent to deallocated instance, after a popup's window.close()).
+        msg<void, BOOL>(win, sel("setReleasedWhenClosed:"), NO);
         msg<void, id>(win, sel("setTitle:"), ns_str("Popup"));
         msg<void, id>(win, sel("setContentView:"), child);
         msg<void>(win, sel("center"));
@@ -6776,7 +6782,15 @@ static id impl_create_web_view(id self, SEL, id webView, id configuration,
     Engine *child_e = new Engine();
     child_e->webview = child;
     child_e->jvm = jvm;
-    child_e->config = configuration;
+    // Canvas 18 safeguard "An engine owns every object it releases": WebKit
+    // only lends `configuration` for this delegate call (+0) and frees it soon
+    // after; the child WKWebView keeps its own copy.  Every teardown path
+    // releases e->config, so the engine must hold its own +1 here.  (Stored
+    // unretained, cocoa_destroy_engine released a freed object when an adopted
+    // tab closed -- objc_msgSend on a garbage isa on the AppKit main thread.)
+    // `manager` stays borrowed: the child WKWebView's own configuration owns
+    // that controller, and no teardown path releases it.
+    child_e->config = msg(configuration, sel("retain"));
     child_e->manager = msg(configuration, sel("userContentController"));
     child_e->popup_window = win;              // nil for ADOPT
     child_e->popup_id = (jlong)child_e;
@@ -6802,6 +6816,12 @@ static id impl_create_web_view(id self, SEL, id webView, id configuration,
     // the child engine's inherited download_callback (Canvas 23).
     msg<void, id>(child, sel("setNavigationDelegate:"), ui);
     child_e->ui_delegate = ui;
+    // Canvas 15: the same delegate object hears the NATIVE_WINDOW popup's
+    // window close (its close button), so a user-closed window is torn down
+    // like window.close() -- see impl_popup_window_will_close.
+    if (win) {
+        msg<void, id>(win, sel("setDelegate:"), ui);
+    }
     {
         std::lock_guard<std::mutex> lk(g_webview_map_mutex);
         g_webview_map[child] = child_e;
@@ -6918,6 +6938,9 @@ static void impl_web_view_did_close(id self, SEL, id webView) {
         g_retained_popups.erase(pid);
     }
     if (e->popup_window) {
+        // Clear the window's delegate first so this -close does not re-enter
+        // impl_popup_window_will_close (Canvas 15).
+        msg<void, id>(e->popup_window, sel("setDelegate:"), (id)nullptr);
         msg<void>(e->popup_window, sel("close"));
     }
 
@@ -6953,9 +6976,39 @@ static void impl_web_view_did_close(id self, SEL, id webView) {
     // here is a prime candidate for on-device zombie/Instruments validation.)
     if (e->webview) { msg(e->webview, sel("release")); }
     e->webview = nullptr;
+    // Balance the retain taken on the passed configuration in
+    // impl_create_web_view (Canvas 18 safeguard "An engine owns every object
+    // it releases").
+    if (e->config) { msg(e->config, sel("release")); e->config = nullptr; }
     delete e;
 }
 
+// NSWindowDelegate windowWillClose: -- the user closed an engine-owned
+// NATIVE_WINDOW popup with its own close button (Canvas 15 safeguard "A native
+// popup is torn down once, however it closes").  Runs on AppKit main.  The
+// teardown is the webViewDidClose: one, scheduled for after this close
+// finishes: a window must not be released inside its own windowWillClose:.
+// cocoa_run_on_main_async would run it inline here (we are on main), so the
+// hop goes through dispatch_async_f.  The delegate and the web view are
+// retained across the hop.  The teardown clears the "eng" association first,
+// so if window.close() already tore the popup down, the hop finds nil and
+// does nothing.
+static void impl_popup_window_will_close(id self, SEL, id /*notification*/) {
+    Engine *e = (Engine *)objc_getAssociatedObject(self, "eng");
+    if (!e || e->java_owned || !e->popup_window || !e->webview) return;
+    struct Hop { id ui; id webview; };
+    Hop *h = new Hop{self, e->webview};
+    msg(h->ui, sel("retain"));
+    msg(h->webview, sel("retain"));
+    dispatch_async_f(dispatch_get_main_queue(), h, +[](void *p) {
+        Hop *g = static_cast<Hop *>(p);
+        impl_web_view_did_close(g->ui, sel("webViewDidClose:"), g->webview);
+        msg(g->webview, sel("release"));
+        msg(g->ui, sel("release"));
+        delete g;
+    });
+}
+
 // Register (or clear, when cb is null) the Java WebViewPopupCallback for this
 // engine.  Mirrors cocoa_set_dialog_callback.
 static void cocoa_set_popup_callback(Engine *e, JNIEnv *env, jobject cb) {
@@ -7902,6 +7955,15 @@ static Class get_webview_embed_ui_delegate_cls() {
             c,
             sel("webViewDidClose:"),
             (IMP)impl_web_view_did_close, "v@:@");
+        // Canvas 15: a NATIVE_WINDOW popup's window close button.
+        {
+            Protocol *wd = objc_getProtocol("NSWindowDelegate");
+            if (wd) class_addProtocol(c, wd);
+        }
+        class_addMethod(
+            c,
+            sel("windowWillClose:"),
+            (IMP)impl_popup_window_will_close, "v@:@");
         // Download support -- Canvas 23.  The same object serves as the
         // WKNavigationDelegate and the WKDownloadDelegate: it already
         // carries the per-engine "eng" associated object every selector
@@ -9300,6 +9362,13 @@ static void cocoa_discard_popup(jlong popupId) {
             msg(e->webview, sel("release"));
             e->webview = nullptr;
         }
+        // Balance the retain taken on the passed configuration in
+        // impl_create_web_view (Canvas 18 safeguard "An engine owns every
+        // object it releases").
+        if (e->config) {
+            msg(e->config, sel("release"));
+            e->config = nullptr;
+        }
         delete e;
     });
 }