Replace the global UsedValues store with per run fragment trees - #11074
Merged
kalenikaliaksandr merged 14 commits intoAug 10, 2026
Merged
Conversation
Atomic roots that are not block formatting contexts by the facts predicate (inline math, replaced boxes with children) ignored the completed run's automatic content block size and fell through to the CSS 2 section 10.6.3 walk, re-deriving the height from the finished run's children while threading live collapsed-margin state across a boundary margins never collapse through. The returned size now wins before the walk, which fixes a masked bug — an inline <math> box's height included the enclosing context's pending collapsed margin — and css-namespace-tag-name-selector.html is rebaselined to the child's exact border box. It also removes a reader of a finished run's interior records ahead of records becoming private to their run.
The committed-metric seal fired only at placement, leaving a window between a run returning and the parent placing its root in which the root's sizes, margins, borders, and paddings could still change — a window a full-suite probe confirmed nothing uses. The run tail now seals those fourteen metrics and the parent keeps writing only the offset family up to placement, gated to normal-mode commit runs since intrinsic sizing and measurement legitimately rewrite the same record. This makes "what a run returns is final at return" an enforced invariant, the guarantee a later commit relies on to snapshot metrics into immutable fragments at placement.
Two SVG sites wrote a box's payloads after placing the box: a path-like element stored its computed path after place_child, and a nested viewport with its own viewBox was placed before the inner run whose percentage lengths must not see the new viewport size early. Both writes move before the placement — run, then write, then place — so a later commit can capture a box's payloads into an immutable fragment at the moment it is placed.
FlexItem, GridItem, and FloatingBox carried &UsedValues borrows of their box's record, and a stored borrow forces the record to live at least as long as the borrowing structure — in practice, the whole pass. They now carry just the box, consumers look the record up per use through the accessors the contexts already funnel through, and the lifetime parameters disappear. This clears the way for records to be owned by their creating run and freed when the run completes.
A flow inline box's record is complete the moment the inline iterator finishes writing its box model and insets: a flow inline is never a containing block, its geometry is assigned from line pieces after commit, and its committed offset is zero by construction. Placing the record right there makes inline boxes placed and sealed like every other box a committing run creates, with the seal enforcing in every build that nothing writes the record afterwards; measurement runs skip the placement so intrinsic re-runs keep an unsealed record.
Every committing run now owns a RunFragmentBuilder, and place_child records each placement in it: placing a box folds the fragments its child run handed over into the box's own pending fragment, converts that single source into an immutable Fragment, and links it under the containing block's pending fragment. A pending fragment may only open for a containing block the run will still place; links naming a sealed or foreign containing block attach under the run root, a layout ancestor of every box the run places, and committed offsets stay containing-block-relative wherever a link attaches. This records structure only and changes no behavior — the foundation for committing layout output from per-run results instead of a pass-global store.
Fragments now capture what commit emits for a box: the fourteen sealed metric cells, the committed offset with the committed delta folded in, the inset family, and the containing-line-box index, resolved at placement where the containing block's line data is already final. Never-placed records are swept into parentless links snapshotted from their records, and a materialized subtree root builds its fragment directly from its sealed record, so the recorded tree carries exactly what the store-fed commit walk reads today. Nothing consumes the captured fields yet; switching commit emission over comes next.
The per-containing-block registration store was the last pass-global coordination structure. Registrations now travel on the same structure the fragments do and drain at their containing block's placement, before its fragment seals, so every drained child nests under the still-open pending fragment. An entry rides the pending fragment of the box whose content space its static position rect is expressed in, translates at each placement, and is taken in document order by the first placement whose builder placed the containing block — builder-scoped truth, since a containing block placed by an outer scope still has unplaced boxes between the entry and itself. Entries whose containing block lies outside a finishing run return with the run's result, composed toward the run root as far as the run's own placements allow; a grid stamps its grid-area containing-block geometry onto contained entries at its completion; the entry accumulator sweeps the rest under the same predicate at pass end. The store, the target walk with its table- internal escape hatch, and the completion-order override machinery are deleted, and new layout tests pin the drain paths for relative table rows and row groups, a cell inside a relative table, a four-deep abspos chain, and a relative block inside a grid item.
An absolutely positioned box whose containing block is a relatively positioned inline needed two pass-global side tables: a set gating first/last-line rect collection and a per-box slot the producing inline run wrote for positioning to read back through an ancestor walk. The rect now travels on the registration entry itself: the producing inline formatting context asks its builder whether any pending registration names an inline containing block — the same question the set answered — and writes the finished rect onto those entries while their coordinate space is still the producing run's containing block. The rect then translates together with the static position rect, positioning receives it as a value, and the set, the slot, its ancestor walk, and the ordering dependency between registration and inline post-processing are deleted.
Boxes with anchor names registered into a pass-global registry at record creation, and anchor_rect re-derived a candidate's geometry at resolution time by walking records of runs that completed long before. Candidates now travel on the builders like every other propagated payload: a placed box with anchor names contributes its border-box rect in its containing block's content space (boxes only — a flow inline's zero-sized record would volunteer garbage geometry now that inline boxes place too), and the run laying out an absolutely positioned box answers name lookups from candidates placed in its own subtree — a tighter set than "has a record", pinned equivalent by the anchor resolution-timing tests. The registry, its record-creation hook, and the resolution-time geometry helper are deleted; no pass-global coordination tables remain.
Commit emission read every box's geometry back out of the pass store and re-derived the containing-line-box index through an emission-time line-fragment lookup. The commit walk now carries the pass's fragment structure as open containing-block scopes: visiting a node with a fragment opens its children for the node's layout subtree, and the per-node probe reads a union map over the open scopes — sound because every link attaches under a fragment whose box is a layout ancestor, or at the pass's top level, which stays open throughout. One fragment per box is asserted at scope opening, presence keys paintable preparation, metrics come from the fragment and its link, the line index is the one placement resolved, and the three FFI entry points share one close-and-commit tail. Line data, rare payloads, and SVG payloads still emit from the store; they move into fragments next.
With geometry already committed from fragments, the remaining store reads on the commit path were the payload families. Placement now moves line data, grid and flex layout data, used grid tracks, table cell coordinates, and override borders out of the record into the snapshot, commit emits them from the fragment, and push_line_data becomes a non-destructive walk over the carried data. The saved absolutely-positioned layout inputs ride the link, finalized before the box is placed, and the offset family leaves the record: the committed offset is computed at placement with the delta folded in as a value, the containing-line-box coordinate arrives as a place_child parameter, and the committed-delta cell, the line-coordinate cell, the record's node back-pointer, and the emission-time line-fragment lookup are deleted.
The SVG payload trio — computed transforms, viewport size, and the retained path handle — was the last store read on the commit path. It now travels on fragments: placement snapshots the trio, and because SVG resource subtrees are laid out once per consumer and legally rewrite these record fields after the owning box was placed, an SVG run's close refreshes every fragment in its structure from the records before returning, gated by a flag the payload writers set so non-SVG runs never pay for the walk. The path handle stays move-only exactly as the store owned it, and with commit's rare-data accessor deleted, commit reads only fragments; the next commit deletes the store.
Every used-values access now goes through the run that owns the record: a run scope registers each record it creates plus the root its parent hands in, lookups return owning handles, and looking up a record the run does not own panics in every build. Parents consume only what completed child runs return — results, held fragments, propagated registrations — so a finished run's interior records become unreachable and are freed at the return, and peak record memory follows the open containing-block chain instead of the whole pass. The abspos drain predicate becomes ownership-scoped (a run places exactly the records it owns, so the placed-slot set returns to a debug assertion), drain-time containing-block geometry composes from owned placed records, and the pass-global store with its lookup surface is deleted — records become plain reference-counted allocations created by their spawning side, with no lifetime parameter threading through the scopes.
📝 WalkthroughWalkthroughThe layout engine now uses Sequence Diagram(s)sequenceDiagram
participant LayoutEntry
participant FormattingContextRun
participant RunFragmentBuilder
participant AbsposEngine
participant LayoutState
LayoutEntry->>FormattingContextRun: create records and run layout
FormattingContextRun->>RunFragmentBuilder: build normal fragments and register pending children
RunFragmentBuilder->>AbsposEngine: drain absolute-positioned payloads
AbsposEngine->>RunFragmentBuilder: resolve and place child fragments
RunFragmentBuilder->>LayoutState: commit completed pass fragments
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Libraries/LibWeb/Rust/src/layout/used_values.rs (1)
248-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
seal_own_metricsinsideseal_committed_box_metrics.
seal_own_metricsrepeats the first fourteen seals ofseal_committed_box_metricsverbatim. A future field addition must be applied in both places.♻️ Proposed deduplication
pub(crate) fn seal_committed_box_metrics(&self) { - self.content_inline_size.seal(); - self.content_block_size.seal(); - self.margin_left.seal(); - self.margin_right.seal(); - self.margin_top.seal(); - self.margin_bottom.seal(); - self.border_left.seal(); - self.border_right.seal(); - self.border_top.seal(); - self.border_bottom.seal(); - self.padding_left.seal(); - self.padding_right.seal(); - self.padding_top.seal(); - self.padding_bottom.seal(); + self.seal_own_metrics(); self.inset_left.seal();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/Rust/src/layout/used_values.rs` around lines 248 - 263, Update seal_committed_box_metrics to call seal_own_metrics instead of duplicating the fourteen own-metric seal operations, while preserving any additional committed-box-specific sealing performed by seal_committed_box_metrics.Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs (1)
3067-3081: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReconstructed runs are rebuilt on every loop iteration.
formatting_context_run()clones twoRchandles per call, and both placement loops call it once per child. Build the run once before each loop.
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L3067-L3081: bind the run before thefor index in 0..self.flex_items.len()loop and pass the binding toplace_child.Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs#L3239-L3249: bind the run before thefor item_index in 0..self.items.len()loop inlayout_itemsand pass the binding toplace_child.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs` around lines 3067 - 3081, Avoid rebuilding the formatting-context run for each child: in flex_formatting_context.rs lines 3067-3081, bind formatting_context_run() once before the loop and pass that binding to place_child; apply the same change in grid_formatting_context.rs lines 3239-3249 within layout_items, with no direct changes needed elsewhere.Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs (1)
577-578: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplain the discarded
context.used(node)call.The call result is unused. With records,
used()panics when the run does not own the record, so this line is now an ownership assertion. State that intent in a comment, or remove the line if it is dead.♻️ Proposed change
for (index, node) in without_fragments.into_iter().enumerate() { - context.used(node); + // Assert that this run owns the record for every fragmented inline it emits a placeholder for. + context.used(node);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs` around lines 577 - 578, Clarify the intent of context.used(node) in the without_fragments iteration: add a concise comment stating that the call intentionally asserts the run owns each record because used() panics otherwise, or remove the call if that ownership assertion is not required.Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs (1)
975-979: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecord handles are re-derived while an equivalent handle is already in scope. After the migration,
used_values()performs aRefCellborrow, aHashMaplookup, and anRcclone. Each site below already holds the handle it needs.
Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs#L975-L979: writecomputed_svg_paththrough the existingusedbinding instead of callingself.used_values(graphics_box)again.Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs#L1067-L1075: writeoverride_borders_datathrough the existingusedbinding instead of callingself.used_values(cell.box_)again.Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs#L839-L843: replaceused_pointer.clone()with&used_pointer.Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs#L1071-L1083: replaceused_pointer.clone()with&used_pointerand drop the duplicate re-borrow three lines later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs` around lines 975 - 979, Avoid re-deriving used-value handles: in Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs:975-979, write computed_svg_path through the existing used binding; in Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs:1067-1075, write override_borders_data through its existing used binding; in svg_formatting_context.rs:839-843 and 1071-1083, replace used_pointer.clone() with &used_pointer, removing the duplicate re-borrow in the latter site.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/Rust/src/layout/fragment_tree.rs`:
- Around line 127-183: Update the placement flow involving
PlacementData::from_record and snapshot_fragment so
used.rare_data.abspos_layout_inputs is read before snapshot_fragment drains the
owned rare payloads, or copied into an independent value first. Preserve the
resulting abspos layout input for PlacementData::from_record and avoid relying
on the emptied rare-data references after snapshot_fragment.
---
Nitpick comments:
In `@Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs`:
- Around line 3067-3081: Avoid rebuilding the formatting-context run for each
child: in flex_formatting_context.rs lines 3067-3081, bind
formatting_context_run() once before the loop and pass that binding to
place_child; apply the same change in grid_formatting_context.rs lines 3239-3249
within layout_items, with no direct changes needed elsewhere.
In `@Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs`:
- Around line 577-578: Clarify the intent of context.used(node) in the
without_fragments iteration: add a concise comment stating that the call
intentionally asserts the run owns each record because used() panics otherwise,
or remove the call if that ownership assertion is not required.
In `@Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs`:
- Around line 975-979: Avoid re-deriving used-value handles: in
Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs:975-979, write
computed_svg_path through the existing used binding; in
Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs:1067-1075, write
override_borders_data through its existing used binding; in
svg_formatting_context.rs:839-843 and 1071-1083, replace used_pointer.clone()
with &used_pointer, removing the duplicate re-borrow in the latter site.
In `@Libraries/LibWeb/Rust/src/layout/used_values.rs`:
- Around line 248-263: Update seal_committed_box_metrics to call
seal_own_metrics instead of duplicating the fourteen own-metric seal operations,
while preserving any additional committed-box-specific sealing performed by
seal_committed_box_metrics.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9611a5d-127e-4585-a709-987de7b53472
📒 Files selected for processing (27)
Libraries/LibWeb/Rust/src/layout/abspos_engine.rsLibraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/flex_formatting_context.rsLibraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/fragment_tree.rsLibraries/LibWeb/Rust/src/layout/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_level_iterator.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsLibraries/LibWeb/Rust/src/layout/mod.rsLibraries/LibWeb/Rust/src/layout/replaced_with_children_formatting_context.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsLibraries/LibWeb/Rust/src/layout/svg_formatting_context.rsLibraries/LibWeb/Rust/src/layout/table_formatting_context.rsLibraries/LibWeb/Rust/src/layout/used_values.rsTests/LibWeb/Layout/expected/abspos-direct-child-of-relative-table-row-group.txtTests/LibWeb/Layout/expected/abspos-direct-child-of-relative-table-row.txtTests/LibWeb/Layout/expected/abspos-in-cell-relative-to-relative-table.txtTests/LibWeb/Layout/expected/css-namespace-tag-name-selector.txtTests/LibWeb/Layout/expected/grid-abspos-under-relative-block-inside-item.txtTests/LibWeb/Layout/expected/nested-abspos-containing-block-chain.txtTests/LibWeb/Layout/input/abspos-direct-child-of-relative-table-row-group.htmlTests/LibWeb/Layout/input/abspos-direct-child-of-relative-table-row.htmlTests/LibWeb/Layout/input/abspos-in-cell-relative-to-relative-table.htmlTests/LibWeb/Layout/input/grid-abspos-under-relative-block-inside-item.htmlTests/LibWeb/Layout/input/nested-abspos-containing-block-chain.html
Comment on lines
+127
to
+183
| fn snapshot_fragment( | ||
| node: crate::layout::node_data::NodeSlotId, | ||
| children: Vec<FragmentLink>, | ||
| used: &UsedValues, | ||
| ) -> Box<Fragment> { | ||
| let line_data = used.line_data.get().map(|cell| Box::new(cell.take())); | ||
| let rare_payloads = used.rare_data.get().map(|cell| { | ||
| let mut rare = cell.borrow_mut(); | ||
| ( | ||
| rare.table_cell_coordinates, | ||
| rare.override_borders_data, | ||
| rare.grid_layout_data.take(), | ||
| rare.flex_layout_data.take(), | ||
| rare.used_grid_tracks.take(), | ||
| rare.computed_svg_transforms, | ||
| rare.svg_viewport_size, | ||
| rare.computed_svg_path.take(), | ||
| ) | ||
| }); | ||
| let ( | ||
| table_cell_coordinates, | ||
| override_borders_data, | ||
| grid_layout_data, | ||
| flex_layout_data, | ||
| used_grid_tracks, | ||
| computed_svg_transforms, | ||
| svg_viewport_size, | ||
| computed_svg_path, | ||
| ) = rare_payloads.unwrap_or_default(); | ||
| Box::new(Fragment { | ||
| node, | ||
| content_inline_size: used.content_inline_size.get(), | ||
| content_block_size: used.content_block_size.get(), | ||
| margin_left: used.margin_left.get(), | ||
| margin_right: used.margin_right.get(), | ||
| margin_top: used.margin_top.get(), | ||
| margin_bottom: used.margin_bottom.get(), | ||
| border_left: used.border_left.get(), | ||
| border_right: used.border_right.get(), | ||
| border_top: used.border_top.get(), | ||
| border_bottom: used.border_bottom.get(), | ||
| padding_left: used.padding_left.get(), | ||
| padding_right: used.padding_right.get(), | ||
| padding_top: used.padding_top.get(), | ||
| padding_bottom: used.padding_bottom.get(), | ||
| table_cell_coordinates, | ||
| override_borders_data, | ||
| line_data, | ||
| grid_layout_data, | ||
| flex_layout_data, | ||
| used_grid_tracks, | ||
| computed_svg_transforms, | ||
| svg_viewport_size, | ||
| computed_svg_path: Cell::new(computed_svg_path), | ||
| children, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List readers of line data and rare payloads, to check for post-placement reads.
set -euo pipefail
rg -nP -C4 '\bline_data_ref\s*\(|\bline_data_cell\s*\(|\brare_data(_mut)?\s*\(' --type=rust Libraries/LibWeb/Rust/src/layoutRepository: LadybirdBrowser/ladybird
Length of output: 18331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map and inspect the relevant fragment-snapshot and layout ordering code.
echo "== file sizes =="
wc -l Libraries/LibWeb/Rust/src/layout/fragment_tree.rs \
Libraries/LibWeb/Rust/src/layout/used_values.rs \
Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs \
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs \
Libraries/LibWeb/Rust/src/layout/formatting_context.rs
echo
echo "== fragment_tree outline =="
ast-grep outline Libraries/LibWeb/Rust/src/layout/fragment_tree.rs --view expanded || true
echo
echo "== fragment_tree relevant excerpts =="
sed -n '1,240p' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs | cat -n
echo
echo "== usages of snapshot_fragment and placement helpers =="
rg -nP -C3 '\bsnapshot_fragment\b|\bplace_fragment\s*\(|\bplace_child\s*\(' Libraries/LibWeb/Rust/src/layout --type=rustRepository: LadybirdBrowser/ladybird
Length of output: 41678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build_fragment_for_placed_box excerpt =="
sed -n '530,615p' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs | cat -n
echo
echo "== refresh_svg_payloads excerpt =="
sed -n '650,740p' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs | cat -n
echo
echo "== line_data_ref readers context =="
for f in Libraries/LibWeb/Rust/src/layout/used_values.rs Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs Libraries/LibWeb/Rust/src/layout/layout_state.rs Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs Libraries/LibWeb/Rust/src/layout/formatting_context.rs Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs; do
echo "--- $f"
rg -n -C3 'line_data_ref\s*\(|line_data_cell\s*\(' "$f" || true
done
echo
echo "== rare_data readers context beyond write sites =="
python3 - <<'PY'
import pathlib, re
files = sorted(pathlib.Path('Libraries/LibWeb/Rust/src/layout').glob('*.rs'))
patterns = [re.compile(r'\brare_data\s*\('), re.compile(r'\brare_data_mut\s*\(')]
for f in files:
text = f.read_text()
hits = []
for pat in patterns:
hits.extend([(m.group(), m.start() + 1, m.end() + 1) for m in pat.finditer(text)])
if hits:
lines = text.splitlines()
print(f'--- {f}')
for s, e, raw in hits:
line = text.count('\n', 0, e - 1) + 1
print(f'line {line}: {raw.strip()}')
start = max(1, line - 4)
end = min(len(lines), line + 4)
for i in range(start, end + 1):
print(f'{i}: {lines[i-1]}')
print()
PY
echo
echo "== derive_baselines line_data_ref context =="
sed -n '555,605p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs | cat -nRepository: LadybirdBrowser/ladybird
Length of output: 14012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all snapshot_fragment callers =="
rg -n -C5 'snapshot_fragment\(' Libraries/LibWeb/Rust/src --type=rust
echo
echo "== line_data_ref usages with caller context =="
rg -n -C8 'line_data_ref\s*\(' Libraries/LibWeb/Rust/src --type=rust
echo
echo "== derive_baselines and content offset readers =="
rg -n -C6 'derive_baselines\s*\(|content_offset\.get\(\)' Libraries/LibWeb/Rust/src/layout --type=rustRepository: LadybirdBrowser/ladybird
Length of output: 50380
Avoid draining rare payload reads after build_fragment_for_placed_box.
PlacementData::from_record reads used.rare_data.borrow().abspos_layout_inputs, but snapshot_fragment moves owned payloads and leaves empty references behind. The subsequent abspos_layout_inputs read returns None, so this box should not be treated as having an abspos layout input after placement. Adjust the order of these two reads or copy abspos_layout_inputs before draining.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Libraries/LibWeb/Rust/src/layout/fragment_tree.rs` around lines 127 - 183,
Update the placement flow involving PlacementData::from_record and
snapshot_fragment so used.rare_data.abspos_layout_inputs is read before
snapshot_fragment drains the owned rare payloads, or copied into an independent
value first. Preserve the resulting abspos layout input for
PlacementData::from_record and avoid relying on the emptied rare-data references
after snapshot_fragment.
This was referenced Aug 10, 2026
Merged
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
1 participant
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.
Layout no longer coordinates through a pass-global used-values table.
Each formatting context run owns the records it creates, and accessing
a record another run owns is a hard error in every build. In place of
the shared store, every run returns an immutable fragment tree: box
metrics are snapshotted from sealed records at placement, placement
data and payloads ride the fragments, and commit emits everything from
the returned trees without reaching back into layout state. Cross-run
concerns — absolutely positioned children, anchor candidates — travel
on the same structure and drain at their containing block's placement.
Together these give each formatting context run a closed contract:
inputs arrive as values, and everything the run produces leaves
through its returned result and fragment tree, with no reads or
writes against shared pass state. That is the precondition for FC run
memoization — a future change can skip a run whose inputs are
unchanged and reuse its fragment tree, which would be unsound as long
as runs communicated through a global store. Rendering is unchanged
across the layout test suite except one deliberate rebaseline (an
inline math box's auto height no longer absorbs the enclosing
context's pending collapsed margin), and new layout tests pin the
placement-time abspos drain paths.