fix: drive split-resize test through explicit layout, un-quarantine on CI (#169)#170
Merged
Conversation
…n CI testExternalSplitResizeDoesNotForceHostedWebViewPresentationRefresh relied on NSSplitView.didResizeSubviewsNotification to trigger the portal's resync, but that observer defers the actual work via DispatchQueue.main.async (WindowBrowserPortal.scheduleExternalGeometrySynchronize). On the CI runner image that deferred block never drains within the test's RunLoop window, so the portal never resyncs after the divider move and the test no-ops deterministically (#169). Call portal.synchronizeWebViewForAnchor(anchor) directly after the layout pass instead of only posting the notification and waiting on the deferred observer. This exercises the same production sync path (WindowBrowserPortal.synchronizeWebView(withId:source:)) synchronously, matching the pattern the sibling anchor-resize test already uses. It does not change what the assertions measure: the geometryOnly refresh they check for still only fires because the anchor's frame genuinely changed. Delete the CI-only XCTSkipIf added in e78b4bb.
…n CI testExternalSplitResizeDoesNotForceHostedWebViewPresentationRefresh relied on NSSplitView.didResizeSubviewsNotification to trigger the portal's resync, but that observer defers the actual work via DispatchQueue.main.async (WindowBrowserPortal.scheduleExternalGeometrySynchronize). On the CI runner image that deferred block never drains within the test's RunLoop window, so the portal never resyncs after the divider move and the test no-ops deterministically (#169). Call portal.synchronizeWebViewForAnchor(anchor) directly after the layout pass instead of only posting the notification and waiting on the deferred observer. This exercises the same production sync path (WindowBrowserPortal.synchronizeWebView(withId:source:)) synchronously, matching the pattern the sibling anchor-resize test already uses. It does not change what the assertions measure: the geometryOnly refresh they check for still only fires because the anchor's frame genuinely changed. Delete the CI-only XCTSkipIf added in e78b4bb.
arzafran
marked this pull request as ready for review
July 21, 2026 19:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
The split-resize browser-portal test was quarantined on CI in e78b4bb because it no-op'd on the current runner image. It relied on
NSSplitView.didResizeSubviewsNotificationto trigger the portal's resync, but that observer defers the actual resync work viaDispatchQueue.main.async. On the CI runner that deferred block never drained within the test'sadvanceAnimations()window, so the portal never resynced after the divider move and the test failed deterministically while passing locally.Important update since this branch was created
While this fix was in progress,
mainmoved past the single-test skip:dda57e17a0quarantines the entireBrowserWindowPortalLifecycleTestssuite (21 tests) via a class-levelsetUpWithErrorskip, after three individual per-test fix attempts each failed the same way on the runner. That commit's message frames it as a broader "never-shown-window layout behavior changed on the runner image" regression (setPosition layout pass and overlay visibility), not confined to this one test's notification-timing issue.This PR is rebased onto that state. My fix is still correct and verified locally (see test plan), and it's a real, defensible root-cause fix for this specific test's failure mode — but the class-level skip still covers it on CI, since un-quarantining the whole suite is a bigger, runner-side investigation that's out of scope for this change. I did not touch the class-level skip.
Summary
portal.synchronizeWebViewForAnchor(anchor)directly after the layout pass instead of only posting the notification and waiting on the deferred observer.WindowBrowserPortal.synchronizeWebView(withId:source:)) synchronously, matching the pattern the siblingtestPortalAnchorResizeDoesNotForceHostedWebViewPresentationRefreshtest already uses.geometryOnlyrefresh they check for (displayIfNeededincrements, no reattach) still only fires because the anchor's frame genuinely changed.Fixes the notification-timing root cause tracked in #169 for this one test. Does not by itself un-quarantine the suite on CI — that requires the broader runner-side investigation
dda57e17a0flagged as pending.Test Plan
xcodebuild -project GhosttyTabs.xcodeproj -scheme programa-unit -destination 'platform=macOS' -only-testing:programaTests/BrowserWindowPortalLifecycleTests test— all 21 tests pass locally, including this one