base red: Takes back #28704: NSMenu tracking goes through one seam adapter (compile + assume_isolated/observer rows)

Merged
LA
6d ago
+81-37

Summary

Base red, priority zero (STATUS 2638, 2643). a8502bea464 (#28704, "menus opened inside a native popup keep the popup open") breaks the feat-cmux-next tip twice:

  • compile: NativePopupHost.swift:235:46 and :239:46 "sending 'menu' risks causing data races". The NSMenu read from the notification was sent into MainActor.assumeIsolated (tip-watcher job ec729ac24847cb52bde83a60).
  • arch: assume_isolated_outside_adapters +3 and observer_sites +2. The cause is two raw NSMenu did{Begin,End}Tracking observers with assumeIsolated in the popup host, plus a DispatchQueue.main.async + assumeIsolated.

Takes back #28704: its menu observation moves into one seam adapter. The popup's behavior is unchanged: it still stays open while a menu it opened tracks, and it takes the keys back after.

Root fix (PATTERNS.md P2 and P3; no escape comment added):

  • MenuTracking (new, in CmuxNextCompat) is the one seam for the two NSMenu tracking notices. It observes both through MainActorObservation and calls its subscribers on the main actor with .began(menu) / .ended(menu).

  • MainActorObservation gains atOnce: (queue nil). With it, the handler runs in the poster's own call, as #28704 needs: a press that opens a menu sees it before its key-loss check. Only a post made on the main thread is delivered. The type's one reviewed main-delivery proof covers both modes. The NSMenu is read on the main actor inside that proof and is never sent across isolation.

  • NativePopupHost keeps a MenuTracking while it watches and drops it in unwatch, as it did with the raw observers. Its deferred dismiss check is now a Task { @MainActor } instead of DispatchQueue.main.async + assumeIsolated.

  • The two other NSMenu tracking observers use MenuTracking too, so those notices are observed in one place:

    • ChromiumWarmup's wait for a menu to end: a raw observer with assumeIsolated before.
    • TabMenuTracking's end of the strip's menu: it now filters the ended menu in its subscriber instead of by object:.

    Both now hear the end inside AppKit's post instead of right after it. CmuxNextChromiumWarmup declares its CmuxNextCompat dependency (core).

Testing

arch-ratchet.py against the merge base 07efcedc1bb: assume_isolated_outside_adapters -5, observer_sites -2, no architecture count rises.

On the current tip 94cabb63a38, measured from the last green 6d639a72684a: this PR removes the two #28704 rows. Two rows stay red, and neither is from #28704:

  • global_mutable_statics +1 in CmuxNextCompat, from a later commit;
  • app_lines +351, from #29532, #29058 and later commits. Those are separate items.

defaults_guard.py check: exit 0. layer_gate.py: ok. The namespace lint: OK. prepush-gate.py: no new violations.

Observed / Not run

Observed: the checks above. Pending, posted as a comment when they finish:

  • the fleet compile with tests (--app --package CmuxNext --recipe-version 2, job 19c8460528747fe4526b50dd);
  • the suites on a display worker (step 9a2d7b42544ba82f7c7f09ef, cmux7s, at c3d86964b02c): PASSED: NativePopupHostChildMenuTests (#28704's own), PagePopupMotionTests, CaptureBackdropTests, ChromiumWarmupTests, MainActorObservationTests, MainActorObservationReleaseTests. The step's exit 1 came only from my list entry TabStripMenuTests: no suite has that name (0 tests ran). The real suite for the tab-strip menu path (TabMenuTracking) is PaneTabBarControlsTests: display step 8e6349ea70b75456015500b6, result posted as a comment when it ends.

Not run: no local Swift build (fleet only).

Changelog

none

Proof

No capture: no visible change; the same popup behavior goes through one menu-tracking seam.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Takes back #28704: a popup stays open while a menu it opened tracks and then retakes the keys, without the compile error or the new assumeIsolated and observer rows it caused. NSMenu tracking notifications now flow through one seam adapter (MenuTracking) instead of raw observers with assumeIsolated.

Changes

  • MenuTracking (new, in CmuxNextCompat) observes didBeginTrackingNotification and didEndTrackingNotification through MainActorObservation and calls subscribers on the main actor with .began(menu) / .ended(menu).
  • MainActorObservation gains atOnce: so the handler runs in the poster's own call on the main thread; the menu object is read on the main actor inside the main-delivery proof and never crosses isolation.
  • NativePopupHost holds one MenuTracking while it watches and drops it in unwatch; its deferred dismiss check becomes a Task { @MainActor }.
  • ChromiumWarmup and TabMenuTracking use MenuTracking too and hear the menu end inside AppKit's post instead of right after it; CmuxNextChromiumWarmup declares its CmuxNextCompat dependency.