LibWeb: Add shared layout node storage for Rust - #10870
Merged
kalenikaliaksandr merged 2 commits intoJul 25, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughAdds a Rust-backed layout-node arena with stable slot IDs and mirrored node data, integrates arena ownership into ChangesLayout node arena migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Document
participant LayoutTreeBuildBridge
participant NodeArena
participant rust_build_layout_tree
participant TreeBuilderHost
Document->>NodeArena: create or retrieve arena
LayoutTreeBuildBridge->>NodeArena: get arena handle
LayoutTreeBuildBridge->>rust_build_layout_tree: pass callbacks and handle
rust_build_layout_tree->>TreeBuilderHost: resolve NodeSlotId values
TreeBuilderHost->>NodeArena: read NodeData and shell pointers
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
kalenikaliaksandr
force-pushed
the
layout-node-arena
branch
from
July 25, 2026 12:44
b6a6cbb to
26f4aae
Compare
![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
🤖 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/Layout/Rust/src/layout_node_arena.rs`:
- Around line 235-241: Update the test module containing
node_data_slots_are_cache_line_aligned to import std::mem::align_of, or fully
qualify the align_of::<Chunk>() call, so the alignment assertion resolves
without changing the test behavior.
🪄 Autofix (Beta)
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: d205210f-b444-431b-92ca-5934abd92a10
📒 Files selected for processing (19)
Libraries/LibWeb/CMakeLists.txtLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/Forward.hLibraries/LibWeb/Layout/AudioBox.hLibraries/LibWeb/Layout/Box.hLibraries/LibWeb/Layout/Node.cppLibraries/LibWeb/Layout/Node.hLibraries/LibWeb/Layout/NodeArena.cppLibraries/LibWeb/Layout/NodeArena.hLibraries/LibWeb/Layout/ReplacedBox.cppLibraries/LibWeb/Layout/Rust/cbindgen.tomlLibraries/LibWeb/Layout/Rust/src/layout_node_arena.rsLibraries/LibWeb/Layout/Rust/src/lib.rsLibraries/LibWeb/Layout/Rust/src/node_data.rsLibraries/LibWeb/Layout/Rust/src/tree_builder.rsLibraries/LibWeb/Layout/TreeBuilder.cppLibraries/LibWeb/Layout/VideoBox.hLibraries/LibWeb/RefCountedTreeNode.h
Comment on lines
+235
to
+241
| #[test] | ||
| fn node_data_slots_are_cache_line_aligned() { | ||
| assert_eq!(align_of::<Chunk>() % 64, 0); | ||
| let mut arena = LayoutNodeArena::new(); | ||
| let allocation = arena.allocate(); | ||
| assert_eq!(allocation.data as usize % 64, 0); | ||
| arena.free(allocation.slot, allocation.generation); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '200,280p' Libraries/LibWeb/Layout/Rust/src/layout_node_arena.rsRepository: LadybirdBrowser/ladybird
Length of output: 2130
Import align_of in the test module. align_of::<Chunk>() is unresolved here unless std::mem::align_of is imported or the call is fully qualified.
🤖 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/Layout/Rust/src/layout_node_arena.rs` around lines 235 -
241, Update the test module containing node_data_slots_are_cache_line_aligned to
import std::mem::align_of, or fully qualify the align_of::<Chunk>() call, so the
alignment assertion resolves without changing the test behavior.
Add stable Rust-owned storage for the layout node data that the future Rust layout rewrite will need, including topology, kinds, flags, generated-node state, style pointers, layout indices, and containing blocks. Existing C++ interfaces read from or synchronize with this representation so future Rust code can access the data without adding per-field FFI.
Tree construction currently asks C++ for topology and a large bundle of layout-node facts on every native walk. This keeps read-only traversal tied to callback traffic even though the same state is already mirrored in the shared node arena. Pass the arena into Rust tree-building and detach entry points, use slot IDs as the walker's node currency, and add a C++ shell back-pointer only for mutation callbacks. Mirror display and replaced-element state at their existing write points, then read topology, kinds, flags, and display facts directly from NodeData. Lazy text, DOM pseudo-style, child-capability, and fieldset legend queries remain targeted C++ calls. This removes the topology and aggregate facts callbacks, reducing the FFI surface that a future native layout rewrite must carry.
kalenikaliaksandr
force-pushed
the
layout-node-arena
branch
from
July 25, 2026 19:01
26f4aae to
183c10a
Compare
kalenikaliaksandr
enabled auto-merge (rebase)
July 25, 2026 19:42
kalenikaliaksandr
disabled auto-merge
July 25, 2026 20:23
kalenikaliaksandr
merged commit Jul 25, 2026
c529025
into
LadybirdBrowser:master
20 of 21 checks passed
This was referenced Jul 27, 2026
This was referenced Aug 9, 2026
This was referenced Aug 16, 2026
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.
Prep work before converting layout code in Rust but also removes
ffi_layout_node_facts()which was very heavy in profiles on https://www.nyan.cat/