Delete LayoutState - #11082
Conversation
layout_state.rs accumulated several groups of types with no relation to the layout state itself: the abspos input vocabulary, the C++ commit sink ABI, the per-run used-values records, and the payload types stored inside UsedValues. Move each group to a file named after what it is: abspos_inputs.rs, commit.rs, and run_records.rs are new, while ContainingBlockGeometry joins its only consumer in abspos_engine.rs, LineData and UsedValuesRareData join the UsedValues fields typed by them in used_values.rs, and InlineAncestorChainRelativeOffset moves to inline_formatting_context.rs.
Almost every method on LayoutState ignored self: the state existed only to answer is_measurement(), yet a &LayoutState was threaded through every facts lookup, used-values creation, and the commit walk. Turn the self-less methods into free functions and constructors next to the types they produce.
The only state a layout pass ever carried was its purpose: one bit distinguishing measurement runs from commit runs, consulted by seven is_measurement() sites. Everything else on LayoutState was already gone. Replace the struct with a Copy LayoutPurpose enum that lives next to the run types in formatting_context.rs and is threaded as a value: FormattingContextRun, SizingContext, AbsposEngine, and the per-formatting-context structs carry it to rebuild runs, the commit entry points pass LayoutPurpose::Commit directly, and MeasurementState is the Measurement purpose itself, so it shrinks to its callbacks.
📝 WalkthroughWalkthroughThe Rust layout engine removes Sequence Diagram(s)sequenceDiagram
participant LayoutEntry
participant FormattingContextRun
participant RunRecords
participant AbsposEngine
participant commit_replacing
LayoutEntry->>FormattingContextRun: create commit or measurement run
FormattingContextRun->>RunRecords: create and register used values
FormattingContextRun->>AbsposEngine: layout pending absolute-positioned children
AbsposEngine->>RunRecords: resolve child layout data
LayoutEntry->>commit_replacing: commit completed fragments
commit_replacing->>RunRecords: read committed used values
Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
Libraries/LibWeb/Rust/src/layout/used_values.rs (1)
624-631: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a SAFETY comment to the
read_paintable_geometrycall.This unsafe block calls a C++ function pointer and passes a raw out-pointer. Every other unsafe site in this module documents its invariants, for example
style_payloadsandtext_contentinLibraries/LibWeb/Rust/src/layout/formatting_context.rs. State that the sink writesgeometrysynchronously and that the shell and paintable pointers stay live for the call.♻️ Proposed comment
let mut geometry = FfiPaintableGeometry::default(); + // SAFETY: The C++ pass host keeps the shell and paintable live for this + // synchronous call, and the sink fills `geometry` before returning. let found = unsafe { (callbacks.read_paintable_geometry)(callbacks.context, callbacks.shell(node), paintable, &raw mut geometry) };🤖 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 624 - 631, Add a SAFETY comment immediately before the unsafe read_paintable_geometry invocation, documenting that the C++ sink writes to the geometry out-pointer synchronously and that the shell and paintable pointers remain valid for the duration of the call.Libraries/LibWeb/Rust/src/layout/formatting_context.rs (1)
1118-1127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
'passlifetime onFormattingContextImplementationis now vestigial.Four of the five boxed variants are lifetime-free after this change. Only
Flex(Box<FlexFormattingContext<'pass>>)still carries'pass. As a result, the'passparameter oncreate_formatting_context_implementationis not constrained by any argument, so the caller chooses it freely.
FlexFormattingContext::stylereturnsStyleValues<'pass>built fromStyleValues::for_node, which yieldsStyleValues<'static>. The flex context therefore does not borrow anything fromrunfor'pass; the parameter only labels data that is already'static. An unconstrained lifetime parameter on a constructor is easy to misread later as a real borrow.Consider making
FlexFormattingContextlifetime-free like the other four implementations, then dropping'passfrom both the enum and this function. This completes the stated goal that formatting-context implementations no longer carry state lifetimes.Also applies to: 1227-1231
🤖 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/formatting_context.rs` around lines 1118 - 1127, Make FlexFormattingContext lifetime-free, since its StyleValues data is already 'static and does not borrow from the caller. Remove the 'pass parameter from FlexFormattingContext, FormattingContextImplementation, and create_formatting_context_implementation, updating the Flex variant and related construction or type references while preserving behavior.Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs (1)
836-842: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
node_facts/style/sizingaccessor trio into a shared trait.This trio duplicates the identical logic in
BlockFormattingContext,FlexFormattingContext, andGridFormattingContext(see the corresponding comment onblock_formatting_context.rs, lines 247-298). Consolidate them into one shared trait implementation; this file could also renamenode_factstofactsfor naming consistency with the other three contexts once consolidated.Also applies to: 876-878
🤖 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/table_formatting_context.rs` around lines 836 - 842, Extract the shared node_facts (renaming it to facts if consistent), style, and sizing accessors from BlockFormattingContext, FlexFormattingContext, and GridFormattingContext into a common trait, preserving their existing callback-backed behavior. Implement the trait for each formatting context and update callers to use the shared accessor names.Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs (1)
172-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider dropping the now-vestigial
'passlifetime parameter.
FlexFormattingContext<'pass>still carries a lifetime parameter, used forFlexItem<'pass>fields andcomputed_main_size/computed_main_min_size/computed_main_max_sizereturn types (&'pass ComputedSize). Sincestyle()now sources data fromStyleValues::for_node, which returnsStyleValues<'static>, the underlyingComputedSizereferences are genuinely'static(the node'sComputedValuesoutlive the pass).BlockFormattingContext,GridFormattingContext, andTableFormattingContextalready dropped their lifetime parameters for the same reason.Replace
'passwith'staticthroughoutFlexFormattingContext,FlexItem, and thecomputed_*_sizehelper return types to match the other three contexts and simplify the generic signature.Also applies to: 234-236
🤖 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` at line 172, Remove the vestigial 'pass lifetime from FlexFormattingContext and FlexItem, replacing affected lifetime annotations with 'static. Update the computed_main_size, computed_main_min_size, and computed_main_max_size helper return types consistently, matching the lifetime-free signatures used by the other formatting contexts.Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs (1)
247-298: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the duplicated
facts/style/sizingaccessor trio. RemovingLayoutStatemoved the same three-method boilerplate (facts/node_facts,style,sizing, all built fromself.callbacks,self.purpose, andself.records) into every formatting context instead of one shared place. Define one trait with default implementations and implement it for each context.
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs#L247-L298: implement the shared trait here instead of the localfacts/style/sizingmethods.Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L234-L249: implement the shared trait here instead of the localstyle/facts/sizingmethods.Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs#L1203-L1220: implement the shared trait here instead of the localstyle/facts/sizingmethods.Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs#L836-L878: implement the shared trait here instead of the localnode_facts/style/sizingmethods, and renamenode_factstofactsfor consistency with the other three contexts.🤖 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/block_formatting_context.rs` around lines 247 - 298, Define a shared trait with default facts, style, and sizing implementations using each context’s callbacks, purpose, and records, then implement it for the formatting contexts. In Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs:247-298, remove the local trio and add the trait implementation; do likewise in flex_formatting_context.rs:234-249 and grid_formatting_context.rs:1203-1220. In table_formatting_context.rs:836-878, replace the local node_facts, style, and sizing methods with the trait implementation and rename node_facts callers to facts.
🤖 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/commit.rs`:
- Around line 116-124: Update commit_subtree around scopes.link_for_slot so
nodes with no commit entry explicitly call
callbacks.set_saved_abspos_layout_inputs with inputs: None. Preserve the
existing link.abspos_layout_inputs assignment for Some entries, ensuring stale
saved absolute-position inputs cannot survive across generations.
In `@Libraries/LibWeb/Rust/src/layout/used_values.rs`:
- Around line 545-566: Update the stretch-fit calculation in the inline-axis
branch to use resolved style metrics instead of the uninitialized `used` values.
Replace the margin and border reads with the corresponding `style.margin_*` and
`style.border_*_width()` accessors, and resolve `style.padding_*` against the
inline percentage basis before subtracting them from `available`.
---
Nitpick comments:
In `@Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs`:
- Around line 247-298: Define a shared trait with default facts, style, and
sizing implementations using each context’s callbacks, purpose, and records,
then implement it for the formatting contexts. In
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs:247-298, remove the
local trio and add the trait implementation; do likewise in
flex_formatting_context.rs:234-249 and grid_formatting_context.rs:1203-1220. In
table_formatting_context.rs:836-878, replace the local node_facts, style, and
sizing methods with the trait implementation and rename node_facts callers to
facts.
In `@Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs`:
- Line 172: Remove the vestigial 'pass lifetime from FlexFormattingContext and
FlexItem, replacing affected lifetime annotations with 'static. Update the
computed_main_size, computed_main_min_size, and computed_main_max_size helper
return types consistently, matching the lifetime-free signatures used by the
other formatting contexts.
In `@Libraries/LibWeb/Rust/src/layout/formatting_context.rs`:
- Around line 1118-1127: Make FlexFormattingContext lifetime-free, since its
StyleValues data is already 'static and does not borrow from the caller. Remove
the 'pass parameter from FlexFormattingContext, FormattingContextImplementation,
and create_formatting_context_implementation, updating the Flex variant and
related construction or type references while preserving behavior.
In `@Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs`:
- Around line 836-842: Extract the shared node_facts (renaming it to facts if
consistent), style, and sizing accessors from BlockFormattingContext,
FlexFormattingContext, and GridFormattingContext into a common trait, preserving
their existing callback-backed behavior. Implement the trait for each formatting
context and update callers to use the shared accessor names.
In `@Libraries/LibWeb/Rust/src/layout/used_values.rs`:
- Around line 624-631: Add a SAFETY comment immediately before the unsafe
read_paintable_geometry invocation, documenting that the C++ sink writes to the
geometry out-pointer synchronously and that the shell and paintable pointers
remain valid for the duration of the call.
🪄 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: 5116db28-9bd8-4fdb-8d74-e5ddb0a3aebb
📒 Files selected for processing (22)
Libraries/LibWeb/Rust/build.rsLibraries/LibWeb/Rust/src/layout/abspos_engine.rsLibraries/LibWeb/Rust/src/layout/abspos_inputs.rsLibraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/commit.rsLibraries/LibWeb/Rust/src/layout/flex_formatting_context.rsLibraries/LibWeb/Rust/src/layout/formatting_context.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/node_facts.rsLibraries/LibWeb/Rust/src/layout/replaced_with_children_formatting_context.rsLibraries/LibWeb/Rust/src/layout/run_records.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsLibraries/LibWeb/Rust/src/layout/style_values.rsLibraries/LibWeb/Rust/src/layout/svg_formatting_context.rsLibraries/LibWeb/Rust/src/layout/table_formatting_context.rsLibraries/LibWeb/Rust/src/layout/text_chunker.rsLibraries/LibWeb/Rust/src/layout/used_values.rs
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/Rust/src/layout/layout_state.rs
| let slot_index = callbacks.slot_index(node); | ||
| let entry = scopes.link_for_slot(slot_index); | ||
| if let Some(link) = entry { | ||
| callbacks.set_saved_abspos_layout_inputs(node, link.abspos_layout_inputs); | ||
| } | ||
| // SAFETY: The C++ sink owns paintables and copies every plain-data | ||
| // input synchronously. | ||
| let node_shell = callbacks.shell(node); | ||
| let paintable = unsafe { (sink.prepare_node)(sink.context, node_shell, entry.is_some()) }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace saved abspos input lifecycle against commit entries.
set -euo pipefail
# Find every writer and reader of the saved abspos inputs.
rg -nP --type=rust -C6 '\b(set_)?saved_abspos_layout_inputs\s*\(' Libraries/LibWeb/Rust/src/layout/
# Inspect CommitScopes to learn which nodes get a link.
rg -nP --type=rust -C10 '\b(fn link_for_slot|fn for_pass|fn open_scope|fn close_scope)\b' Libraries/LibWeb/Rust/src/layout/
# Check whether the arena clears the inputs between passes.
rg -nP --type=rust -C5 'abspos_layout_inputs' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rsRepository: LadybirdBrowser/ladybird
Length of output: 16805
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== commit.rs outline =="
ast-grep outline Libraries/LibWeb/Rust/src/layout/commit.rs --view expanded || true
echo "== commit.rs 80-150 =="
sed -n '80,150p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== FormattingContextRun methods/fields =="
ast-grep outline Libraries/LibWeb/Rust/src/layout/formatting_context.rs --match FormattingContextRun --view expanded || true
echo "== formatting_context.rs 2000-2095 =="
sed -n '2000,2095p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs | cat -n
echo "== fragment_tree.rs relevant section =="
sed -n '230,290p' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs | cat -n
echo "== abspos_engine replay section =="
sed -n '1776,1815p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs | cat -n
echo "== all layout_nodes_arena saved metadata/clearing =="
sed -n '300,340p' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs | cat -n
sed -n '615,645p' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs | cat -n
echo "== slot_index usages in Rust layout =="
rg -n --type=rust '\bslot_index\(|fn slot_index|struct Node|struct SlotMetadata|generation' Libraries/LibWeb/Rust/src/layout -C 3Repository: LadybirdBrowser/ladybird
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== commit.rs full file size =="
wc -l Libraries/LibWeb/Rust/src/layout/commit.rs Libraries/LibWeb/Rust/src/layout/fragment_tree.rs Libraries/LibWeb/Rust/src/layout/formatting_context.rs Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs
echo "== commit.rs 1-280 =="
sed -n '1,280p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== commit.rs 280-end =="
sed -n '280,600p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== formatting_context.rs saved_abspos layout invocation sites =="
rg -n --type=rust -C4 '\brust_layout_replay_saved_abspos_layout\(|saved_abspos_layout_inputs\(|set_saved_abspos_layout_inputs\(|AbsposLayoutInputs|HasSavedAbsposLayoutInputs|SavedAbspos' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== fragment_tree.rs SavedAbsposLayoutInputs assignment sites =="
rg -n --type=rust -C4 '\babspos_layout_inputs\b|\bsaved_abspos_layout\b|\bsave_abspos|PendingAbsposChild|build_fragment_for_placed_box|new_fragment_link' Libraries/LibWeb/Rust/src/layout/fragment_tree.rsRepository: LadybirdBrowser/ladybird
Length of output: 48223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== C++ FFI binding sites for replay_saved_abspos/clear saved abspos =="
rg -n -C4 'rust_layout_replay_saved_abspos_layout|saved_abspos_layout_inputs|HasSavedAbsposLayoutInputs|SavedAbsposLayout|saved_abspos' Libraries/LibWeb -g '*.cpp' -g '*.h' -g '*.c' -g '*.hpp'
echo "== C++ layout engine abspos commit path references =="
rg -n -C4 'replay.*abspos|abspos.*replay|rust_layout_replay|saved_abspos|committed_box|commit_replacing|drain_and_commit_entry' Libraries/LibWeb -g '*.cpp' -g '*.h' -g '*.c' -g '*.hpp'
echo "== Rust placement link initialization around abspos_link assignment =="
rg -n --type=rust -C5 'abspos_layout_inputs\s*:|abspos_layout_inputs:' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs
echo "== layout function entry that passes pass fragments into commit =="
rg -n --type=rust -C6 'commit_replacing\(|rust_layout_replay_saved_abspos_layout\(' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== inspect pass function around line 2043 =="
sed -n '2010,2088p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs | cat -nRepository: LadybirdBrowser/ladybird
Length of output: 25991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CommitScopes link_for_slot call sites =="
rg -n --type=rust -C5 'scopes\.link_for_slot|link_for_slot\(' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== commit_subtree call sites that use scopes root =="
rg -n --type=rust -C6 'commit_subtree\(\(|scopes\.(open_scope|for_pass)' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== node_intrinsic_layout_mode and abspos engine path =="
rg -n --type=rust -C4 'fn node_intrinsic_layout_mode|node_intrinsic_layout_mode\(|abspos_layout_inputs|is_absolutely_positioned|abspos_engine' Libraries/LibWeb/Rust/src/layout -g '*.rs' | head -n 300Repository: LadybirdBrowser/ladybird
Length of output: 31754
Clear saved abspos inputs for nodes without a commit entry.
commit_subtree only calls set_saved_abspos_layout_inputs when scopes.link_for_slot(slot_index) returns Some. For nodes that remain in the arena across pass generation without an entry, saved_abspos_layout_inputs can still return the previous pass via generation == metadata.generation. Since rust_layout_replay_saved_abspos_layout asserts saved inputs are present and AbsposEngine::replay then uses them, also update the no-entry path with inputs: None to prevent replaying stale containing-block/static-position geometry.
🤖 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/commit.rs` around lines 116 - 124, Update
commit_subtree around scopes.link_for_slot so nodes with no commit entry
explicitly call callbacks.set_saved_abspos_layout_inputs with inputs: None.
Preserve the existing link.abspos_layout_inputs assignment for Some entries,
ensuring stale saved absolute-position inputs cannot survive across generations.
| if matches!(axis, Axis::Inline) | ||
| && !facts.is_replaced_box() | ||
| && !facts.is_floating() | ||
| && !facts.is_absolutely_positioned() | ||
| && facts.display().is_block_outside() | ||
| && parent_facts.is_some_and(|parent| { | ||
| !parent.is_floating() | ||
| && (parent.display().is_flow_root_inside() || parent.display().is_flow_inside()) | ||
| }) | ||
| && containing_block_has_definite_size(Axis::Inline) | ||
| { | ||
| let available = containing_block_size_for_axis(Axis::Inline); | ||
| return Some(clamp_to_max_dimension_value( | ||
| available | ||
| - used.margin_left.get() | ||
| - used.margin_right.get() | ||
| - used.padding_left.get() | ||
| - used.padding_right.get() | ||
| - used.border_left.get() | ||
| - used.border_right.get(), | ||
| )); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether box-model cells are populated before the stretch-fit computation.
set -euo pipefail
# Find every writer of the margin/padding/border used cells.
rg -nP --type=rust -C3 '\b(margin_left|padding_left|border_left)\.set\s*\(' Libraries/LibWeb/Rust/src/layout/
# Confirm no writer runs inside create_used_values.
ast-grep run --pattern $'pub(crate) fn create_used_values($$$) -> $_ {
$$$
}' --lang rust Libraries/LibWeb/Rust/src/layout/used_values.rs
# Show the historical initialization for comparison.
git log --oneline -5 -- Libraries/LibWeb/Rust/src/layout/layout_state.rs || trueRepository: LadybirdBrowser/ladybird
Length of output: 36384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the CSS parser/value definitions for margin border padding pixel resolution.
rg -n --type=rust -C2 'fn margin_(left|right)\(\)|fn border_(left|right)_width\(\)|fn padding_(left|right)\(\)|fn to_px|CssPixels::default|ValueParser' Libraries/LibWeb/Rust/src/style
# Inspect whether border width exists outside box-sizing adjustment context.
rg -n --type=rust -C2 'border_left_width\s*\(|border_right_width\s*\(' Libraries/LibWeb/Rust/src/layout LibWeb/Rust/src/style 2>/dev/null || trueRepository: LadybirdBrowser/ladybird
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Style directories:\n'
fd -d 4 'src' Libraries/LibWeb | head -50
printf '\nCSS parser/value definitions:\n'
rg -nP --type=rust -C2 'fn margin_(left|right)\(|fn border_(left|right)_width\(|fn padding_(left|right)\(|fn to_px|struct .*Property|type .*Value' Libraries/LibWeb/Rust/src | head -250
printf '\nRelevant used_values.rs outline:\n'
ast-grep outline Libraries/LibWeb/Rust/src/layout/used_values.rs --match create_used_values --view expanded || trueRepository: LadybirdBrowser/ladybird
Length of output: 18260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Layout style_values.rs:\n'
cat -n Libraries/LibWeb/Rust/src/layout/style_values.rs
printf '\nComputedValueView definitions for margin/padding/border:\n'
ast-grep outline Libraries/LibWeb/Rust/src/css/computed_value_views.rs --match ComputedValuesView --view expanded || true
rg -nP --type=rust -C3 'pub(crate) fn (margin_left|margin_right|border_left_width|border_right_width|padding_left|padding_right)\(\)|fn to_px\(self, reference: CssPixels\)' Libraries/LibWeb/Rust/src/css/computed_value_views.rs
printf '\nUsedValues fields:\n'
sed -n '1,180p' Libraries/LibWeb/Rust/src/layout/used_values.rs | cat -nRepository: LadybirdBrowser/ladybird
Length of output: 18721
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'ComputedValuesView methods around surround/box_values:\n'
sed -n '88,132p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs | cat -n
sed -n '475,535p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs | cat -n
printf '\nSurroundValues and StyleValueData methods:\n'
rg -nP --type=rust -C3 'struct SurroundValues|impl StyleValueData|impl ComputedLengthPercentageOrAuto|fn border_.*width|fn padding_.*\(|fn margin_.*\(|pub enum StyleValueData' Libraries/LibWeb/Rust/src/css | head -300
printf '\nBehavioral probe: exact zero-gate condition in create_used_values range:\n'
python3 - <<'PY'
from pathlib import Path
p = Path("Libraries/LibWeb/Rust/src/layout/used_values.rs")
text = p.read_text()
before = text[:530]
stretch = text[530:570]
after = text[570:615]
for label, s in [("before", before), ("stretch", stretch), ("after", after)]:
count = s.count(".set(")
print(f"{label}: .set() count={count}")
for ident in ["margin_left", "padding_left", "border_left"]:
print(f"{label}: {ident} references={s.count(ident)}")
print("default() call count:", text[:620].count("UsedValues::default()"))
print("stretch branch references zero used margin/padding/border Left cells:",
stretch.count("used.margin_left.get()"),
stretch.count("used.padding_left.get()"),
stretch.count("used.border_left.get()"))
PYRepository: LadybirdBrowser/ladybird
Length of output: 8873
Use style-resolved box metrics for the stretch-fit calculation.
used is default-initialized and is not written before this branch, so used.margin_left.get(), used.padding_left.get(), and used.border_left.get() all read zero here. The branch should subtract style.margin_*(...), style.padding_*(...), and style.border_*_width() instead, using style.padding_*(...) resolved against the inline percentage basis.
🤖 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 545 - 566,
Update the stretch-fit calculation in the inline-axis branch to use resolved
style metrics instead of the uninitialized `used` values. Replace the margin and
border reads with the corresponding `style.margin_*` and
`style.border_*_width()` accessors, and resolve `style.padding_*` against the
inline percentage basis before subtracting them from `available`.
|
Hi brother
…On Tue, 11 Aug 2026 at 00:17 coderabbitai[bot] ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
*Actionable comments posted: 2*
🧹 Nitpick comments (5)
Libraries/LibWeb/Rust/src/layout/used_values.rs (1)
624-631: *📐 Maintainability & Code Quality* | *🔵 Trivial* | *💤 Low
value*
*Add a SAFETY comment to the read_paintable_geometry call.*
This unsafe block calls a C++ function pointer and passes a raw
out-pointer. Every other unsafe site in this module documents its
invariants, for example style_payloads and text_content in
Libraries/LibWeb/Rust/src/layout/formatting_context.rs. State that the
sink writes geometry synchronously and that the shell and paintable
pointers stay live for the call.
♻️ Proposed comment
let mut geometry = FfiPaintableGeometry::default();+ // SAFETY: The C++ pass host keeps the shell and paintable live for this+ // synchronous call, and the sink fills `geometry` before returning.
let found =
unsafe {
(callbacks.read_paintable_geometry)(callbacks.context, callbacks.shell(node), paintable, &raw mut geometry)
};
🤖 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 ***@***.***/LibWeb/Rust/src/layout/used_values.rs` around lines 624 - 631, Add
a SAFETY comment immediately before the unsafe read_paintable_geometry
invocation, documenting that the C++ sink writes to the geometry out-pointer
synchronously and that the shell and paintable pointers remain valid for the
duration of the call.
Libraries/LibWeb/Rust/src/layout/formatting_context.rs (1)
1118-1127: *📐 Maintainability & Code Quality* | *🔵 Trivial* | *⚡ Quick
win*
*The 'pass lifetime on FormattingContextImplementation is now vestigial.*
Four of the five boxed variants are lifetime-free after this change. Only
Flex(Box<FlexFormattingContext<'pass>>) still carries 'pass. As a result,
the 'pass parameter on create_formatting_context_implementation is not
constrained by any argument, so the caller chooses it freely.
FlexFormattingContext::style returns StyleValues<'pass> built from
StyleValues::for_node, which yields StyleValues<'static>. The flex
context therefore does not borrow anything from run for 'pass; the
parameter only labels data that is already 'static. An unconstrained
lifetime parameter on a constructor is easy to misread later as a real
borrow.
Consider making FlexFormattingContext lifetime-free like the other four
implementations, then dropping 'pass from both the enum and this
function. This completes the stated goal that formatting-context
implementations no longer carry state lifetimes.
Also applies to: 1227-1231
🤖 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 ***@***.***/LibWeb/Rust/src/layout/formatting_context.rs` around lines 1118 -
1127, Make FlexFormattingContext lifetime-free, since its StyleValues data is
already 'static and does not borrow from the caller. Remove the 'pass parameter
from FlexFormattingContext, FormattingContextImplementation, and
create_formatting_context_implementation, updating the Flex variant and related
construction or type references while preserving behavior.
Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs (1)
836-842: *📐 Maintainability & Code Quality* | *🔵 Trivial* | *⚡ Quick
win*
*Extract the node_facts/style/sizing accessor trio into a shared trait.*
This trio duplicates the identical logic in BlockFormattingContext,
FlexFormattingContext, and GridFormattingContext (see the corresponding
comment on block_formatting_context.rs, lines 247-298). Consolidate them
into one shared trait implementation; this file could also rename
node_facts to facts for naming consistency with the other three contexts
once consolidated.
Also applies to: 876-878
🤖 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 ***@***.***/LibWeb/Rust/src/layout/table_formatting_context.rs` around lines
836 - 842, Extract the shared node_facts (renaming it to facts if consistent),
style, and sizing accessors from BlockFormattingContext, FlexFormattingContext,
and GridFormattingContext into a common trait, preserving their existing
callback-backed behavior. Implement the trait for each formatting context and
update callers to use the shared accessor names.
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs (1)
172-172: *📐 Maintainability & Code Quality* | *🔵 Trivial* | *⚡ Quick
win*
*Consider dropping the now-vestigial 'pass lifetime parameter.*
FlexFormattingContext<'pass> still carries a lifetime parameter, used for
FlexItem<'pass> fields and computed_main_size/computed_main_min_size/
computed_main_max_size return types (&'pass ComputedSize). Since style()
now sources data from StyleValues::for_node, which returns
StyleValues<'static>, the underlying ComputedSize references are
genuinely 'static (the node's ComputedValues outlive the pass).
BlockFormattingContext, GridFormattingContext, and TableFormattingContext
already dropped their lifetime parameters for the same reason.
Replace 'pass with 'static throughout FlexFormattingContext, FlexItem,
and the computed_*_size helper return types to match the other three
contexts and simplify the generic signature.
Also applies to: 234-236
🤖 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 ***@***.***/LibWeb/Rust/src/layout/flex_formatting_context.rs` at line 172,
Remove the vestigial 'pass lifetime from FlexFormattingContext and FlexItem,
replacing affected lifetime annotations with 'static. Update the
computed_main_size, computed_main_min_size, and computed_main_max_size helper
return types consistently, matching the lifetime-free signatures used by the
other formatting contexts.
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs (1)
247-298: *📐 Maintainability & Code Quality* | *🔵 Trivial* | *⚡ Quick
win*
*Consolidate the duplicated facts/style/sizing accessor trio.* Removing
LayoutState moved the same three-method boilerplate (facts/node_facts,
style, sizing, all built from self.callbacks, self.purpose, and
self.records) into every formatting context instead of one shared place.
Define one trait with default implementations and implement it for each
context.
-
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs#L247-L298:
implement the shared trait here instead of the local facts/style/sizing
methods.
- Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L234-L249:
implement the shared trait here instead of the local style/facts/sizing
methods.
-
Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs#L1203-L1220:
implement the shared trait here instead of the local style/facts/sizing
methods.
-
Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs#L836-L878:
implement the shared trait here instead of the local node_facts/style/
sizing methods, and rename node_facts to facts for consistency with
the other three contexts.
🤖 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 ***@***.***/LibWeb/Rust/src/layout/block_formatting_context.rs` around lines
247 - 298, Define a shared trait with default facts, style, and sizing
implementations using each context’s callbacks, purpose, and records, then
implement it for the formatting contexts. In
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs:247-298, remove the
local trio and add the trait implementation; do likewise in
flex_formatting_context.rs:234-249 and grid_formatting_context.rs:1203-1220. In
table_formatting_context.rs:836-878, replace the local node_facts, style, and
sizing methods with the trait implementation and rename node_facts callers to
facts.
🤖 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 ***@***.***/LibWeb/Rust/src/layout/commit.rs`:
- Around line 116-124: Update commit_subtree around scopes.link_for_slot so
nodes with no commit entry explicitly call
callbacks.set_saved_abspos_layout_inputs with inputs: None. Preserve the
existing link.abspos_layout_inputs assignment for Some entries, ensuring stale
saved absolute-position inputs cannot survive across generations.
In ***@***.***/LibWeb/Rust/src/layout/used_values.rs`:
- Around line 545-566: Update the stretch-fit calculation in the inline-axis
branch to use resolved style metrics instead of the uninitialized `used` values.
Replace the margin and border reads with the corresponding `style.margin_*` and
`style.border_*_width()` accessors, and resolve `style.padding_*` against the
inline percentage basis before subtracting them from `available`.
---
Nitpick comments:
In ***@***.***/LibWeb/Rust/src/layout/block_formatting_context.rs`:
- Around line 247-298: Define a shared trait with default facts, style, and
sizing implementations using each context’s callbacks, purpose, and records,
then implement it for the formatting contexts. In
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs:247-298, remove the
local trio and add the trait implementation; do likewise in
flex_formatting_context.rs:234-249 and grid_formatting_context.rs:1203-1220. In
table_formatting_context.rs:836-878, replace the local node_facts, style, and
sizing methods with the trait implementation and rename node_facts callers to
facts.
In ***@***.***/LibWeb/Rust/src/layout/flex_formatting_context.rs`:
- Line 172: Remove the vestigial 'pass lifetime from FlexFormattingContext and
FlexItem, replacing affected lifetime annotations with 'static. Update the
computed_main_size, computed_main_min_size, and computed_main_max_size helper
return types consistently, matching the lifetime-free signatures used by the
other formatting contexts.
In ***@***.***/LibWeb/Rust/src/layout/formatting_context.rs`:
- Around line 1118-1127: Make FlexFormattingContext lifetime-free, since its
StyleValues data is already 'static and does not borrow from the caller. Remove
the 'pass parameter from FlexFormattingContext, FormattingContextImplementation,
and create_formatting_context_implementation, updating the Flex variant and
related construction or type references while preserving behavior.
In ***@***.***/LibWeb/Rust/src/layout/table_formatting_context.rs`:
- Around line 836-842: Extract the shared node_facts (renaming it to facts if
consistent), style, and sizing accessors from BlockFormattingContext,
FlexFormattingContext, and GridFormattingContext into a common trait, preserving
their existing callback-backed behavior. Implement the trait for each formatting
context and update callers to use the shared accessor names.
In ***@***.***/LibWeb/Rust/src/layout/used_values.rs`:
- Around line 624-631: Add a SAFETY comment immediately before the unsafe
read_paintable_geometry invocation, documenting that the C++ sink writes to the
geometry out-pointer synchronously and that the shell and paintable pointers
remain valid for the duration of the call.
🪄 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*: 5116db28-9bd8-4fdb-8d74-e5ddb0a3aebb
📥 Commits
Reviewing files that changed from the base of the PR and between 8f32f17
<8f32f17>
and c277415
<c277415>
.
📒 Files selected for processing (22)
- Libraries/LibWeb/Rust/build.rs
- Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
- Libraries/LibWeb/Rust/src/layout/abspos_inputs.rs
- Libraries/LibWeb/Rust/src/layout/block_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/commit.rs
- Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/inline_level_iterator.rs
- Libraries/LibWeb/Rust/src/layout/layout_state.rs
- Libraries/LibWeb/Rust/src/layout/line_builder.rs
- Libraries/LibWeb/Rust/src/layout/mod.rs
- Libraries/LibWeb/Rust/src/layout/node_facts.rs
-
Libraries/LibWeb/Rust/src/layout/replaced_with_children_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/run_records.rs
- Libraries/LibWeb/Rust/src/layout/sizing_context.rs
- Libraries/LibWeb/Rust/src/layout/style_values.rs
- Libraries/LibWeb/Rust/src/layout/svg_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/table_formatting_context.rs
- Libraries/LibWeb/Rust/src/layout/text_chunker.rs
- Libraries/LibWeb/Rust/src/layout/used_values.rs
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/Rust/src/layout/layout_state.rs
------------------------------
In Libraries/LibWeb/Rust/src/layout/commit.rs
<#11082 (comment)>
:
> + let slot_index = callbacks.slot_index(node);
+ let entry = scopes.link_for_slot(slot_index);
+ if let Some(link) = entry {
+ callbacks.set_saved_abspos_layout_inputs(node, link.abspos_layout_inputs);
+ }
+ // SAFETY: The C++ sink owns paintables and copies every plain-data
+ // input synchronously.
+ let node_shell = callbacks.shell(node);
+ let paintable = unsafe { (sink.prepare_node)(sink.context, node_shell, entry.is_some()) };
*🗄️ Data Integrity & Integration* | *🟠 Major* | *⚡ Quick win*
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Description: Trace saved abspos input lifecycle against commit entries.set -euo pipefail
# Find every writer and reader of the saved abspos inputs.
rg -nP --type=rust -C6 '\b(set_)?saved_abspos_layout_inputs\s*\(' Libraries/LibWeb/Rust/src/layout/
# Inspect CommitScopes to learn which nodes get a link.
rg -nP --type=rust -C10 '\b(fn link_for_slot|fn for_pass|fn open_scope|fn close_scope)\b' Libraries/LibWeb/Rust/src/layout/
# Check whether the arena clears the inputs between passes.
rg -nP --type=rust -C5 'abspos_layout_inputs' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs
Repository: LadybirdBrowser/ladybird
Length of output: 16805
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo "== commit.rs outline =="
ast-grep outline Libraries/LibWeb/Rust/src/layout/commit.rs --view expanded || true
echo "== commit.rs 80-150 =="
sed -n '80,150p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== FormattingContextRun methods/fields =="
ast-grep outline Libraries/LibWeb/Rust/src/layout/formatting_context.rs --match FormattingContextRun --view expanded || true
echo "== formatting_context.rs 2000-2095 =="
sed -n '2000,2095p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs | cat -n
echo "== fragment_tree.rs relevant section =="
sed -n '230,290p' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs | cat -n
echo "== abspos_engine replay section =="
sed -n '1776,1815p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs | cat -n
echo "== all layout_nodes_arena saved metadata/clearing =="
sed -n '300,340p' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs | cat -n
sed -n '615,645p' Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs | cat -n
echo "== slot_index usages in Rust layout =="
rg -n --type=rust '\bslot_index\(|fn slot_index|struct Node|struct SlotMetadata|generation' Libraries/LibWeb/Rust/src/layout -C 3
Repository: LadybirdBrowser/ladybird
Length of output: 50380
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo "== commit.rs full file size =="
wc -l Libraries/LibWeb/Rust/src/layout/commit.rs Libraries/LibWeb/Rust/src/layout/fragment_tree.rs Libraries/LibWeb/Rust/src/layout/formatting_context.rs Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs
echo "== commit.rs 1-280 =="
sed -n '1,280p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== commit.rs 280-end =="
sed -n '280,600p' Libraries/LibWeb/Rust/src/layout/commit.rs | cat -n
echo "== formatting_context.rs saved_abspos layout invocation sites =="
rg -n --type=rust -C4 '\brust_layout_replay_saved_abspos_layout\(|saved_abspos_layout_inputs\(|set_saved_abspos_layout_inputs\(|AbsposLayoutInputs|HasSavedAbsposLayoutInputs|SavedAbspos' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== fragment_tree.rs SavedAbsposLayoutInputs assignment sites =="
rg -n --type=rust -C4 '\babspos_layout_inputs\b|\bsaved_abspos_layout\b|\bsave_abspos|PendingAbsposChild|build_fragment_for_placed_box|new_fragment_link' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs
Repository: LadybirdBrowser/ladybird
Length of output: 48223
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo "== C++ FFI binding sites for replay_saved_abspos/clear saved abspos =="
rg -n -C4 'rust_layout_replay_saved_abspos_layout|saved_abspos_layout_inputs|HasSavedAbsposLayoutInputs|SavedAbsposLayout|saved_abspos' Libraries/LibWeb -g '*.cpp' -g '*.h' -g '*.c' -g '*.hpp'
echo "== C++ layout engine abspos commit path references =="
rg -n -C4 'replay.*abspos|abspos.*replay|rust_layout_replay|saved_abspos|committed_box|commit_replacing|drain_and_commit_entry' Libraries/LibWeb -g '*.cpp' -g '*.h' -g '*.c' -g '*.hpp'
echo "== Rust placement link initialization around abspos_link assignment =="
rg -n --type=rust -C5 'abspos_layout_inputs\s*:|abspos_layout_inputs:' Libraries/LibWeb/Rust/src/layout/fragment_tree.rs
echo "== layout function entry that passes pass fragments into commit =="
rg -n --type=rust -C6 'commit_replacing\(|rust_layout_replay_saved_abspos_layout\(' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== inspect pass function around line 2043 =="
sed -n '2010,2088p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs | cat -n
Repository: LadybirdBrowser/ladybird
Length of output: 25991
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo "== CommitScopes link_for_slot call sites =="
rg -n --type=rust -C5 'scopes\.link_for_slot|link_for_slot\(' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== commit_subtree call sites that use scopes root =="
rg -n --type=rust -C6 'commit_subtree\(\(|scopes\.(open_scope|for_pass)' Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== node_intrinsic_layout_mode and abspos engine path =="
rg -n --type=rust -C4 'fn node_intrinsic_layout_mode|node_intrinsic_layout_mode\(|abspos_layout_inputs|is_absolutely_positioned|abspos_engine' Libraries/LibWeb/Rust/src/layout -g '*.rs' | head -n 300
Repository: LadybirdBrowser/ladybird
Length of output: 31754
------------------------------
*Clear saved abspos inputs for nodes without a commit entry.*
commit_subtree only calls set_saved_abspos_layout_inputs when
scopes.link_for_slot(slot_index) returns Some. For nodes that remain in
the arena across pass generation without an entry,
saved_abspos_layout_inputs can still return the previous pass via generation
== metadata.generation. Since rust_layout_replay_saved_abspos_layout
asserts saved inputs are present and AbsposEngine::replay then uses them,
also update the no-entry path with inputs: None to prevent replaying
stale containing-block/static-position geometry.
🤖 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 ***@***.***/LibWeb/Rust/src/layout/commit.rs` around lines 116 - 124, Update
commit_subtree around scopes.link_for_slot so nodes with no commit entry
explicitly call callbacks.set_saved_abspos_layout_inputs with inputs: None.
Preserve the existing link.abspos_layout_inputs assignment for Some entries,
ensuring stale saved absolute-position inputs cannot survive across generations.
------------------------------
In Libraries/LibWeb/Rust/src/layout/used_values.rs
<#11082 (comment)>
:
> + if matches!(axis, Axis::Inline)
+ && !facts.is_replaced_box()
+ && !facts.is_floating()
+ && !facts.is_absolutely_positioned()
+ && facts.display().is_block_outside()
+ && parent_facts.is_some_and(|parent| {
+ !parent.is_floating()
+ && (parent.display().is_flow_root_inside() || parent.display().is_flow_inside())
+ })
+ && containing_block_has_definite_size(Axis::Inline)
+ {
+ let available = containing_block_size_for_axis(Axis::Inline);
+ return Some(clamp_to_max_dimension_value(
+ available
+ - used.margin_left.get()
+ - used.margin_right.get()
+ - used.padding_left.get()
+ - used.padding_right.get()
+ - used.border_left.get()
+ - used.border_right.get(),
+ ));
+ }
*🎯 Functional Correctness* | *🟠 Major* | *⚡ Quick win*
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Description: Check whether box-model cells are populated before the stretch-fit computation.set -euo pipefail
# Find every writer of the margin/padding/border used cells.
rg -nP --type=rust -C3 '\b(margin_left|padding_left|border_left)\.set\s*\(' Libraries/LibWeb/Rust/src/layout/
# Confirm no writer runs inside create_used_values.
ast-grep run --pattern $'pub(crate) fn create_used_values($$$) -> $_ { $$$}' --lang rust Libraries/LibWeb/Rust/src/layout/used_values.rs
# Show the historical initialization for comparison.
git log --oneline -5 -- Libraries/LibWeb/Rust/src/layout/layout_state.rs || true
Repository: LadybirdBrowser/ladybird
Length of output: 36384
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Inspect the CSS parser/value definitions for margin border padding pixel resolution.
rg -n --type=rust -C2 'fn margin_(left|right)\(\)|fn border_(left|right)_width\(\)|fn padding_(left|right)\(\)|fn to_px|CssPixels::default|ValueParser' Libraries/LibWeb/Rust/src/style
# Inspect whether border width exists outside box-sizing adjustment context.
rg -n --type=rust -C2 'border_left_width\s*\(|border_right_width\s*\(' Libraries/LibWeb/Rust/src/layout LibWeb/Rust/src/style 2>/dev/null || true
Repository: LadybirdBrowser/ladybird
Length of output: 297
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf 'Style directories:\n'
fd -d 4 'src' Libraries/LibWeb | head -50
printf '\nCSS parser/value definitions:\n'
rg -nP --type=rust -C2 'fn margin_(left|right)\(|fn border_(left|right)_width\(|fn padding_(left|right)\(|fn to_px|struct .*Property|type .*Value' Libraries/LibWeb/Rust/src | head -250
printf '\nRelevant used_values.rs outline:\n'
ast-grep outline Libraries/LibWeb/Rust/src/layout/used_values.rs --match create_used_values --view expanded || true
Repository: LadybirdBrowser/ladybird
Length of output: 18260
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf 'Layout style_values.rs:\n'
cat -n Libraries/LibWeb/Rust/src/layout/style_values.rs
printf '\nComputedValueView definitions for margin/padding/border:\n'
ast-grep outline Libraries/LibWeb/Rust/src/css/computed_value_views.rs --match ComputedValuesView --view expanded || true
rg -nP --type=rust -C3 'pub(crate) fn (margin_left|margin_right|border_left_width|border_right_width|padding_left|padding_right)\(\)|fn to_px\(self, reference: CssPixels\)' Libraries/LibWeb/Rust/src/css/computed_value_views.rs
printf '\nUsedValues fields:\n'
sed -n '1,180p' Libraries/LibWeb/Rust/src/layout/used_values.rs | cat -n
Repository: LadybirdBrowser/ladybird
Length of output: 18721
------------------------------
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf 'ComputedValuesView methods around surround/box_values:\n'
sed -n '88,132p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs | cat -n
sed -n '475,535p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs | cat -n
printf '\nSurroundValues and StyleValueData methods:\n'
rg -nP --type=rust -C3 'struct SurroundValues|impl StyleValueData|impl ComputedLengthPercentageOrAuto|fn border_.*width|fn padding_.*\(|fn margin_.*\(|pub enum StyleValueData' Libraries/LibWeb/Rust/src/css | head -300
printf '\nBehavioral probe: exact zero-gate condition in create_used_values range:\n'
python3 - <<'PY'from pathlib import Pathp = Path("Libraries/LibWeb/Rust/src/layout/used_values.rs")text = p.read_text()before = text[:530]stretch = text[530:570]after = text[570:615]for label, s in [("before", before), ("stretch", stretch), ("after", after)]: count = s.count(".set(") print(f"{label}: .set() count={count}") for ident in ["margin_left", "padding_left", "border_left"]: print(f"{label}: {ident} references={s.count(ident)}")print("default() call count:", text[:620].count("UsedValues::default()"))print("stretch branch references zero used margin/padding/border Left cells:", stretch.count("used.margin_left.get()"), stretch.count("used.padding_left.get()"), stretch.count("used.border_left.get()"))PY
Repository: LadybirdBrowser/ladybird
Length of output: 8873
------------------------------
*Use style-resolved box metrics for the stretch-fit calculation.*
used is default-initialized and is not written before this branch, so
used.margin_left.get(), used.padding_left.get(), and
used.border_left.get() all read zero here. The branch should subtract
style.margin_*(...), style.padding_*(...), and style.border_*_width()
instead, using style.padding_*(...) resolved against the inline
percentage basis.
🤖 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 ***@***.***/LibWeb/Rust/src/layout/used_values.rs` around lines 545 - 566,
Update the stretch-fit calculation in the inline-axis branch to use resolved
style metrics instead of the uninitialized `used` values. Replace the margin and
border reads with the corresponding `style.margin_*` and
`style.border_*_width()` accessors, and resolve `style.padding_*` against the
inline percentage basis before subtracting them from `available`.
—
Reply to this email directly, view it on GitHub
<#11082?email_source=notifications&email_token=CBJNDRXIOIYRSJEQ4ZBLW7D5JH7Q3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIOBZHEZDONJYGQZKM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#pullrequestreview-4899275842>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CBJNDRU7EU767WQNRKCBG4L5JH7Q3AVCNFSNUABFKJSXA33TNF2G64TZHM4DAOBQGQ2TIOBVHNEXG43VMU5TKMJRGI2TENJQGQ3KC5QC>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Now when LayoutState doesn't carry any actual state it's time to delete it and do little code reorganization.