`gpui_harness_tests.rs` was the repo's largest file (1062 lines) and the
worst violator of the 500-line / no-god-component audit. Its 13
`#[gpui::test]`s each build their own local fixtures and share only a
pair of type aliases, an `impl super::ElyShell` test helper, and two free
fns (`active_tab_overlay_state`, `example_url`).
Keep those shared items plus tests 1-4 in the root module; move tests 5-8
to `gpui_harness_tests_b.rs` and tests 9-13 to `gpui_harness_tests_c.rs`,
each opening with `use super::*;` so they inherit the parent's imports
and shared items with no per-item churn. Declared via `#[path]` mod, the
established sibling-test pattern in this crate.
475 / 319 / 276 lines. No tests added, removed, or renamed (paths gain a
`gpui_harness_tests_{b,c}::` segment). clippy --all-targets -D warnings
clean; `cargo test -p ely_app` 167 passed / 0 failed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`scripts/audit_source_lines.sh` (a CI gate) flagged `tests/commands.rs`
(527) and `tests/splits.rs` (524) over the 500-line ceiling. Both are
flat lists of independent `#[test]` fns with no shared helpers, so each
splits cleanly into a sibling integration-test binary (the crate already
uses a topical file-per-concern layout under `tests/`).
- commands.rs (29 tests) -> commands.rs (20: tab/space/profile/search)
+ command_pages.rs (9: internal-page-opening commands). 527 -> 389.
- splits.rs (25 tests) -> splits.rs (13: layout/axis/detach mechanics)
+ saved_split_lifecycle.rs (12: close/archive/restore/group). 524 -> 266.
No tests added or removed; each new file carries only the imports it
uses. `cargo clippy -p ely_browser_core --tests -- -D warnings` clean;
all ely_browser_core tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`runtime.rs` had grown past the 500-line ceiling enforced by
`scripts/audit_source_lines.sh` once the grid-pref comment landed (522
lines). Peel the repaint/present pair — `paint_without_readback`,
`paint_without_readback_with_completion`, the private `paint_webview`,
and `paint_with_readback` — into a sibling `runtime_paint.rs`, exactly
the `paint.rs` boundary the embedding architecture doc prescribes.
`runtime_paint` is a child module of `runtime` (declared via `#[path]`,
mirroring `runtime_context`), so it keeps access to the private
`SoftwareServoHost` fields and the `webview()` / `wait_for_paint_completion`
/ `read_rendered_frame` helpers without widening any visibility. No
behaviour change: public API and call sites are identical.
runtime.rs 522 -> 469 lines. Build + clippy clean; full workspace test
suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`Preferences::default()` is Servo's conservative library default and
ships `layout_grid_enabled: false`. `Servo::new` forwards prefs to Stylo
(`prefs::set` -> `stylo_static_prefs::set_pref!("layout.grid.enabled")`),
so with the gate off Stylo blockifies `display: grid`: every grid
container collapses to `display: block` and grid-based page layouts
stack into a single column — the "broken" rendering reported on modern
sites.
`ely_servo_preferences()` only flipped `dom_intersection_observer_enabled`
and inherited the grid default, so ELY rendered grid pages collapsed
while Servo's own servoshell (which enables the pref) renders them
correctly. The layout path is implemented — servo-layout drives
`DisplayInside::Grid` through Taffy — so enabling the pref is the real
fix, not a workaround.
Verified with a deterministic `display: grid; grid-template-columns:
1fr 1fr 1fr` page: 9 stacked full-width bars before, a 3x3 grid after.
Wikipedia/HN/GitHub/google.com re-checked unchanged; full workspace
test suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Servo's \`WebView::set_history\` fires \`notify_url_changed\` on **every**
history mutation — full navigations, redirects, in-page link clicks,
and JS-driven \`history.pushState\` / \`history.replaceState\`.
Our pipeline fans that delegate signal back through
\`WebSurfaceUrlChange\` into \`tab.url\`. The next \`ensure_surface\`
observes the new tab URL, hashes a fresh ensure key, and calls
\`ServoLiveClient::ensure\` → \`apply_navigation\`. Until this commit
\`apply_navigation\` compared the request against the local
\`session.requested_url\` cache and, on mismatch, sent Servo a
\`webview.load(url)\` — even when Servo was the one who *just*
told us about that URL.
\`WebView::load\` is a hard navigation: it tells the constellation
to abort the current document, clear the surface, and refetch.
google.com's homepage \`replaceState\`s a fresh \`?zx=<timestamp>\`
roughly once a second to bust caches; with this round-trip we
were turning each of those into a full load-clear-refetch and
producing one visible white flash per second.
Use \`host.snapshot(webview_id).url()\` (which Servo keeps in lock-step
with \`set_history\`) as the source of truth. If Servo's WebView is
already at the requested URL, just sync our \`session.requested_url\`
bookkeeping and return — no \`load\` message, no surface clear, no
flash. Genuine embedder-initiated navigations (chrome URL bar typed,
in-app link click) still take the \`should_navigate\` path because
Servo's URL hasn't caught up to the requested target yet.
Servo's reference embedder (\`examples/winit_minimal.rs\`) is reactive:
- \`user_event\` only calls \`servo.spin_event_loop()\` — never paints
- \`notify_new_frame_ready\` is what flips the embedder into the
redraw path (calls \`window.request_redraw()\`)
- \`RedrawRequested\` is the **single** site that runs
\`webview.paint() + rendering_context.present()\`
Painting is therefore at-most-once-per-real-frame: a paint happens
only when Servo has actually composited new content.
Our \`ServoLiveClient::ensure\` was forcing
\`paint_without_readback_with_completion\` after every setup pass.
\`ensure\` runs on every \`ensure_surface\` invocation — viewport
debounce settle, URL redirect (google.com → / → /?zx=…), permission
update, native-surface re-attach, page-zoom change. Each invocation
called Servo's painter while it had no new composited frame ready:
WebRender's \`clear_background\` wiped the surface to the configured
shell background colour, the empty scene rendered, \`present\` swapped
that blank surface to the NSView's CALayer — and the user saw a
white flash. A chain of redirects produced a chain of flashes.
Drop the unconditional \`paint\` from \`ensure\`. Keep the
\`host.tick()\` (Servo's spin) so the navigate / viewport / input
messages reach the constellation in this turn, but defer painting to
\`poll\`, which already gates \`paint_without_readback_with_completion\`
on \`snapshot.has_pending_frame()\` — exactly mirroring the
reference embedder's reactive model. \`ServoLiveFrame\` returned from
\`ensure\` only carries snapshot metadata, so callers that depend on
the frame value (worker queue, UI state) are unaffected.
Servo's reference embedder (\`examples/winit_minimal.rs\`) handles
resize with a single call: \`webview.resize(new_size)\`. The Servo
paint pipeline behind that call:
1. early-returns if \`rendering_context.size() == new_size\`,
2. otherwise calls \`rendering_context.resize\` itself,
3. updates \`webview_renderer.rect\` so the compositor relays out
the page at the new device viewport,
4. sends \`transaction.set_document_view(...)\` so WebRender's
document viewport matches the surface,
5. flags the painter \`needs_repaint(RepaintReason::Resize)\`.
Our \`runtime.rs::resize\` called \`webview.rendering_context.resize\`
*before* \`webview.webview.resize\`. surfman accepted the new size, so
the painter saw \`rendering_context.size() == new_size\` and took the
early-return path — steps 3, 4, and 5 never ran. The compositor
kept the original (creation-time) viewport rect while we presented
a much larger surface. The page ended up laid out for a tiny
viewport and either rendered into the top-left of a sea of cleared
background (StableLance) or never painted any pipeline at all
(google.com appeared totally blank).
Drop the direct \`rendering_context.resize\` call and route through
\`WebView::resize\` exactly as the reference embedder does. The
debounce in \`record_viewport_size\` still collapses a sidebar /
window-edge animation into a single trailing-edge resize, so we
also avoid hammering Servo with per-frame surface mutations.
The web canvas is a GPUI `native_surface` element, which on macOS
attaches an `NSView` (with a Metal-backed `CALayer`) as an AppKit
overlay on top of the GPUI render layer. GPUI's `overflow: hidden`
clips its own children but cannot reach into AppKit, so the canvas
painted square corners on top of the main panel's rounded edge —
the white surface visibly overshot the panel's rounded bottom.
Thread a per-corner radius through the GPUI patch:
- `Window::sync_native_surface` and the `PlatformWindow` trait carry
a `Corners<Pixels>`.
- The macOS implementation sets `CALayer.cornerRadius` to the maximum
requested radius and `maskedCorners` to the bitmask of corners that
actually requested rounding (GPUI is Y-down while NSView is Y-up,
so a GPUI "top" maps to a CALayer "MaxY" corner — comment locks
the mapping). `setMasksToBounds: YES` lets Core Animation actually
honor the corner radius for layered content.
- The `NativeSurface` element resolves all four corner radii from its
own `StyleRefinement` (so callers attach `.rounded_bl(...)` /
`.rounded_br(...)` exactly as on any other GPUI element).
Application side wires the right radius per context:
- The main canvas asks for `RADIUS_CARD` (18 px) on its bottom corners
so it follows the surrounding panel's rounded bottom edge while
the top stays flush against the toolbar.
- A split pane's canvas asks for the pane's own `SPLIT_PANE_RADIUS`
(10 px) so the canvas hugs the rounded frame the pane already paints.
`render_failed_web_surface` doesn't need rounding wiring — the error
page is a GPUI div and is clipped normally by the surrounding
`overflow: hidden`.
Sidebar / window-edge animations emit a new viewport bounds on every
GPUI paint. The previous flow fired `runtime.ensure_tab` synchronously
on every change, which reached down into Servo's surfman backend and
destroyed + reallocated the rendering framebuffer 50+ times per second
— each cycle leaving the NSView with an empty surface until Servo's
next paint completed. The user saw the page strobe blank-then-content
on every gesture, plus a one-time Metal driver warning about an
unloadable texture (the previous framebuffer caught mid-recreate).
Track `viewport_size_changed_at` on `PerTabSurface` and propagate the
debounce on two seams:
- `record_viewport_size` returns `Buffered` for a transition that
arrives inside the 80 ms window of the previous one, so the
synchronous `flush_external_web_surface_tick` from the GPUI paint
callback skips the resize altogether. The very first measurement
and the first transition after a quiet period still return `Applied`
so a single drag step or page-load is not delayed.
- `ensure_surface` skips while the viewport is settling, but only
*after* the initial ensure has installed `last_ensure_key`. The
first ensure must fire even mid-gesture or the page never loads.
- `next_tick_delay` clamps the poll cadence to `ACTIVE_POLL_INTERVAL`
while any visible tab is settling, so the trailing-edge resize
fires within a frame of the gesture stopping instead of waiting
for an 80 ms idle tick.
The synchronous `record + ensure` pattern in
`failed_surface_ensure_waits_for_a_new_key_before_retrying` can't
advance an `Instant`, so it now calls a `#[cfg(test)]`
`clear_viewport_resize_debounce_for_test` to simulate the
trailing edge and exercise the retry-on-new-key business rule in
isolation. Adds `rapid_viewport_changes_buffer_until_gesture_settles`
to lock the new behaviour.
web_surface_runtime.rs hit 628 lines because it carried four unrelated
concerns:
- session domain types (`WebSurfaceRuntimeScope`, `WebSurfaceSession`,
`WebSurfaceUrlChange`, …) and their config-dir / `session_for_scope`
helpers — these are the runtime's input vocabulary, not its control
flow.
- wire-side glue (scroll-input field marshalling, `pending_input_kind`
classifier, the latency tracing call, and the `From<&Site...>` impl
that lowers permissions to the Servo client) — these are the runtime's
output vocabulary.
- the runtime itself, plus its tests harness.
Split into:
- `web_surface_runtime.rs` (441 lines): WebSurfaceRuntime + Drop.
- `web_surface_runtime_session.rs`: the session types + helpers, with
`pub(super)`-exposed methods so the runtime can drive them.
- `web_surface_runtime_wire.rs`: the wire helpers and the From impl.
`web_surface_runtime` keeps re-exporting the URL-change / frame /
ensure-result types so existing call sites
(`web_surface_state.rs`, `web_surface.rs`, `web_surface_controller.rs`)
need no rewiring.
`web_surface.rs` had crept to 548 lines; six unrelated input-recorder
methods (scroll delta, viewport size, native surface, hover point,
click point, typed text) dominated the file. Move them into a sibling
`web_surface_input.rs` so the store file (now 338 lines) is the
ensure / tick / lifecycle facade and the input file is a flat set of
`record_*` methods on the same struct.
`surfaces`, `keyboard_focus`, and `surface_mut` become `pub(super)` so
the sibling can reach them; everything else stays private.
tabs.rs had grown to 538 lines — past the 500-line ceiling — and was
mixing four unrelated concerns:
- `tab_navigation` (URL changes + history back/forward)
- `tab_metadata` (title, favicon, zoom, favorite/pin, sort, sync flag)
- `tab_archive_restore` (un-archive entry points + query matching)
- core CRUD (open / close / move-to-space + private helpers)
Each split file lives below 250 lines and only pulls the `ely_domain`
types and crate helpers it actually uses. `active_tab_mut` and the
`TabUrlUpdate` enum get `pub(super)` so siblings can reach them; no
behaviour change.
Lift `apply_viewport`'s three-way diff (resize / set_hidpi / set_zoom)
into a `ViewportChange` value so the decision is testable without a
running `SoftwareServoHost`. Four unit cases pin the contract:
- Retina creation (DPR=2.0): only the hidpi push fires (the bug above).
- Standard-DPI creation (DPR=1.0): nothing to push, session already
matches Servo's defaults.
- Mid-session zoom change: only `set_page_zoom` fires.
- Cross-monitor DPR change: only the hidpi push fires.
This is a regression guard for the previous commit — any future change
that re-initializes a session from request values instead of Servo's
post-build defaults will fail `fresh_retina_session_pushes_hidpi_only`.
The newly-created session was stamped with the request's hidpi factor,
but `WebViewBuilder` defaults `hidpi_scale_factor` to 1.0 and we never
override it. `apply_viewport`'s diff check then saw
`session.dpr == request.dpr` and skipped `set_hidpi_scale`, leaving Servo
at hidpi=1.0 forever. On Retina that collapses CSS pixels onto device
pixels — the page lays out for a 2× viewport and renders at half size.
Same shape applies to page zoom.
Initialize the session with Servo's actual post-build defaults so the
viewport diff is the source of truth for whether `set_hidpi_scale` and
`set_page_zoom` need to run.
ZacharyZhang-NYandClaude Opus 4.8