Repository navigation
Base red: the app no longer crashes at launch on macOS 14 (WebKit navigation with no source frame) - #36290
Conversation
…ot a trap (red) macOS 14's WebKit gives a nil WKNavigationAction.sourceFrame for some navigations although WebKit declares it nonnull. PageWebView and the agent pane read sourceFrame.request.url through the Swift type, so the nil NSURLRequest bridges to URLRequest and traps: the cmux-next app ended at launch on macOS 14.8.7 (Tart smoke of 66339d3). FakeNavigationAction (CmuxNextPagesTestSupport) answers the sourceFrame getter with nil through the Objective-C runtime, as that WebKit does. Both delegates trap on it at this commit.
…S 14 macOS 14's WebKit gives a nil WKNavigationAction.sourceFrame for some navigations, although the header declares it nonnull. PageWebView (since #26286, 1e27057) and AgentPaneNavigation read sourceFrame.request.url through the Swift type: the nil frame answers a nil NSURLRequest, and its bridge to URLRequest traps (URLRequest._unconditionallyBridgeFromObjectiveC). The cmux-next app ended at launch on macOS 14.8.7 in PageWebView.webView(_:decidePolicyFor:) (fleet build 83e04e0d8b97c62c796fc5aa, Tart smoke 48cb9eceb51bb6143325b239, crashes=1); 15 and 26 pass. CmuxNextDesign's WKNavigationAction.sourceFrameIfPresent / sourceFrameURL / targetFrameURL and WKFrameInfo.requestURLIfPresent ask the Objective-C runtime for the getters, whose answer is typed optional, so the optimizer cannot fold a nil check away (as it may for `sourceFrame as WKFrameInfo?`, which the browser's sourcePage used). Both navigation delegates and sourcePage read through them. With a frame present, the URL is the same as before; with none, the policy treats the source as unknown, as it already does for a nil URL (the agent pane refuses an artifact frame it did not make). Regression: PageNavigationNoSourceFrameTests and AgentPaneNavigationNoSourceFrameTests (previous commit) trap at the base and pass here.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Shipped in:
|
|
For the PageWebView and agent-pane navigation area: #26286 (1e27057, artifact isolation on the shared WK page host) added the |
|
Review (independent subagent): SHIP at b53b9f8 Correctness first. I read the diff, the policy code it feeds ( 1. The helper (
2. Fail-closed
3. Test double
4. Other bridged reads of nonnull WebKit properties
5. Repo rules
CI on this head: all checks are green or skipped, except Review (independent subagent): SHIP at b53b9f8 |
macOS 14's WebKit gives no source frame for some navigations (#36290). In PageNavigation.policy a nil source URL skipped the source checks: a srcdoc frame of the agent page sent to another document of the page was cancelled from a known source but allowed with none. On the agent page, a load into a frame inside the page from an unknown source is now cancelled first, as the agent pane's rules deny an unknown source. The main frame is not affected, so the launch load of the page's own document still loads with no source frame (the load that crashed on macOS 14 before #36290). Loads with a known source take the same path as before. Chromium calls the policy for the main frame only. Regression: AgentPageNoSourceSubframeTests (previous commits) fails 4 expectations at 2781f61 (step 99ca5fd136639b8811d1fcb6) and passes here.
…4 PageWebView fix #36290)
… 14 PageWebView fix #36290)
…cOS 14 PageWebView fix #36290)
Summary
Base red: the cmux-next app ends at launch on macOS 14.8.7, its oldest supported OS. 15.7.7 and 26.6.2 pass the same smoke.
SMOKE FAIL os=14.8.7 ... crashes=1. The app died after thechromiumstep; every later step found no socket.Cause
PageWebView.webView(_:decidePolicyFor:)readsaction.sourceFrame.request.url. #26286 (1e27057) added this line. WebKit declaressourceFramenonnull, but macOS 14's WebKit gives nil for some navigations. In Swift, the nil frame then gives a nilNSURLRequest, and the bridge toURLRequesttraps. The trapping frame is exactly that bridge, called directly from the delegate closure. The iOS app already hit the same WebKit behavior (BrowserTab.swift: "sourceFrameis nil for some navigations despite its Swift type").AgentPaneNavigationreads the same property the same way, so it has the same hazard. The browser'sWKNavigationAction.sourcePageusedsourceFrame as WKFrameInfo?. That cast does not prove safety, because the optimizer may fold the nonnull value's.some.Fix
The new
CmuxNextDesign/WKNavigationAction+Frames.swiftaddssourceFrameIfPresent,sourceFrameURL,targetFrameURLandWKFrameInfo.requestURLIfPresent. They call the Objective-C getters throughperform(_:), which returns an optional. Thus no code assumes that the frame or its request is nonnull. All three call sites now use them:PageWebView,AgentPaneNavigationandsourcePage. When the frame exists, the URL is the same as before. When there is no frame, the policy treats the source as unknown (nil), which is a path it already handles. A nil source never grants the artifact or srcdoc allowances, so it fails closed.Other WebKit delegate reads in the app are safe:
targetFrameis declared optional,WKScriptMessage.frameInfoand theinitiatedByFrame:parameters are not navigation actions, and no other code readssourceFrame.Regression test
FakeNavigationAction(CmuxNextPagesTestSupport) is aWKNavigationActionsubclass. ItssourceFramegetter returns nil through the Objective-C runtime, as on macOS 14. The tests call the real delegate methods with it:PageNavigationNoSourceFrameTests:PageWebViewallows its own document, opens a clicked outside link externally, and cancels an unclicked one.AgentPaneNavigationNoSourceFrameTests:AgentPaneNavigationallows the pane's document and cancels another main-frame document.Command:
cmux-ci run --class light --script scripts/ci/package-test-lane.sh --ref <sha> --arg=suite --arg=Packages/macOS/CmuxNext --arg=PageNavigationNoSourceFrameTests,AgentPaneNavigationNoSourceFrameTestsunexpected signal code 5(SIGTRAP, the trap in the crash report).✔ aNavigationWithNoSourceFrameGetsThePolicyNotATrap()and✔ aNavigationWithNoSourceFrameGetsTheDecisionNotATrap()pass.macOS matrix (Tart,
scripts/cmux-next/os-matrix.sh --suite smoke)Build of b53b9f8: fleet job 7f4ba96af2200a8fcfb1b297 (tag
base-red-mac14-b53b9f860-v1). Smoke step 57a668a56b1a7651b1834b67:Observed / Not run
Observed:
chromiumstep.python3 scripts/cmux-next/prepush-gate.py: no new violations.Not run:
sourcePagechange has no test of its own. It reads through the same helper that the two tests cover.Changelog
No capture: there is no UI change. This fixes a crash in a WebKit navigation delegate.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes a launch crash on macOS 14 where a WebKit navigation with no source frame trapped the app.
WKNavigationAction.sourceFrameon some navigations even though it is declared nonnull; readingsourceFrame.request.urlbridged a nilNSURLRequesttoURLRequestand trapped, killing the app at launch on the oldest supported OS (14.8.7). macOS 15 and 26 passed the same smoke.CmuxNextDesignhelpers (sourceFrameIfPresent,sourceFrameURL,targetFrameURL,requestURLIfPresent) read the frames through the Objective-C runtime so a missing frame becomes nil instead of a trap.PageWebView,AgentPaneNavigation, and the browser'ssourcePagenow use them; with a frame present the URL is unchanged, and with none the policy treats the source as unknown, which the existing paths already handle and which fails closed on artifact and srcdoc allowances.FakeNavigationActiondouble that returns nil forsourceFramethrough the runtime;PageNavigationNoSourceFrameTestsandAgentPaneNavigationNoSourceFrameTeststrap at the base commit and pass with this fix.Tart smoke on macOS 14.8.7 goes from FAIL (crashes=1) to PASS (crashes=0, hangs=0, faults=0); 15.7.7 stays PASS.
Written for commit b53b9f8. Summary will update on new commits.