Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-09-27 23:30:08 +01:00
Code Issues Projects Releases Packages Wiki Activity

layout: Support storing layout data for two-level nested pseudo-elements (#38678)

Browse source 
Add basic support for storing layout data for pseudo-elements nested to
up to two levels. This removes the last unstored layout result and fixes
a double-borrow issue. This change does not add properly parsing nor
styling of these element types, but does prepare for those changes which
must come from stylo.

Testing: This fixes a intermittent panic in
`tests/wpt/tests/css/css-lists/nested-marker-styling.html`
Fixes: #38177. 
Closes: #38183.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Martin Robinson 2025-08-14 15:41:34 +02:00 • committed by GitHub
parent 43da933247
commit 99ce81cf64
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 273 additions and 177 deletions
Download patch file Download diff file Expand all files Collapse all files

171
components/layout/dom.rs
View file

@ -34,7 +34,7 @@ use crate::taffy::TaffyItemBox;
#[derive(MallocSizeOf)]
pub struct PseudoLayoutData {
pseudo: PseudoElement,
box_slot: ArcRefCell<Option<LayoutBox>>,
data: ArcRefCell<InnerDOMLayoutData>,
}
/// The data that is stored in each DOM node that is used by layout.
@ -45,22 +45,65 @@ pub struct InnerDOMLayoutData {
}
impl InnerDOMLayoutData {
pub(crate) fn for_pseudo(
fn pseudo_layout_data(
&self,
pseudo_element: Option<PseudoElement>,
) -> Option<AtomicRef<Option<LayoutBox>>> {
let Some(pseudo_element) = pseudo_element else {
return Some(self.self_box.borrow());
};
pseudo_element: PseudoElement,
) -> Option<ArcRefCell<InnerDOMLayoutData>> {
for pseudo_layout_data in self.pseudo_boxes.iter() {
if pseudo_element == pseudo_layout_data.pseudo {
return Some(pseudo_layout_data.box_slot.borrow());
return Some(pseudo_layout_data.data.clone());
}
}
None
}
fn create_pseudo_layout_data(
&mut self,
pseudo_element: PseudoElement,
) -> ArcRefCell<InnerDOMLayoutData> {
let data: ArcRefCell<InnerDOMLayoutData> = Default::default();
self.pseudo_boxes.push(PseudoLayoutData {
pseudo: pseudo_element,
data: data.clone(),
});
data
}
fn fragments(&self) -> Vec<Fragment> {
self.self_box
.borrow()
.as_ref()
.map(LayoutBox::fragments)
.unwrap_or_default()
}
fn repair_style(&self, node: &ServoThreadSafeLayoutNode, context: &SharedStyleContext) {
if let Some(layout_object) = &*self.self_box.borrow() {
layout_object.repair_style(context, node, &node.style(context));
}
for pseudo_layout_data in self.pseudo_boxes.iter() {
let Some(node_with_pseudo) = node.with_pseudo(pseudo_layout_data.pseudo) else {
continue;
};
pseudo_layout_data
.data
.borrow()
.repair_style(&node_with_pseudo, context);
}
}
fn clear_fragment_layout_cache(&self) {
if let Some(data) = self.self_box.borrow().as_ref() {
data.clear_fragment_layout_cache();
}
for pseudo_layout_data in self.pseudo_boxes.iter() {
pseudo_layout_data
.data
.borrow()
.clear_fragment_layout_cache();
}
}
}
/// A box that is stored in one of the `DOMLayoutData` slots.
@ -191,14 +234,6 @@ impl From<ArcRefCell<Option<LayoutBox>>> for BoxSlot<'_> {
/// A mutable reference to a `LayoutBox` stored in a DOM element.
impl BoxSlot<'_> {
pub(crate) fn dummy() -> Self {
let slot = None;
Self {
slot,
marker: PhantomData,
}
}
pub(crate) fn set(mut self, box_: LayoutBox) {
if let Some(slot) = &mut self.slot {
*slot.borrow_mut() = Some(box_);
@ -233,10 +268,9 @@ pub(crate) trait NodeExt<'dom> {
fn as_svg(&self) -> Option<SVGElementData>;
fn as_typeless_object_with_data_attribute(&self) -> Option<String>;
fn layout_data_mut(&self) -> AtomicRefMut<'dom, InnerDOMLayoutData>;
fn layout_data(&self) -> Option<AtomicRef<'dom, InnerDOMLayoutData>>;
fn element_box_slot(&self) -> BoxSlot<'dom>;
fn pseudo_element_box_slot(&self, pseudo_element: PseudoElement) -> BoxSlot<'dom>;
fn ensure_inner_layout_data(&self) -> AtomicRefMut<'dom, InnerDOMLayoutData>;
fn inner_layout_data(&self) -> Option<AtomicRef<'dom, InnerDOMLayoutData>>;
fn box_slot(&self) -> BoxSlot<'dom>;
/// Remove boxes for the element itself, and all of its pseudo-element boxes.
fn unset_all_boxes(&self);
@ -327,11 +361,11 @@ impl<'dom> NodeExt<'dom> for ServoThreadSafeLayoutNode<'dom> {
.map(|string| string.to_owned())
}
fn layout_data_mut(&self) -> AtomicRefMut<'dom, InnerDOMLayoutData> {
if ThreadSafeLayoutNode::layout_data(self).is_none() {
fn ensure_inner_layout_data(&self) -> AtomicRefMut<'dom, InnerDOMLayoutData> {
if self.layout_data().is_none() {
self.initialize_layout_data::<DOMLayoutData>();
}
ThreadSafeLayoutNode::layout_data(self)
self.layout_data()
.unwrap()
.as_any()
.downcast_ref::<DOMLayoutData>()
@ -340,8 +374,8 @@ impl<'dom> NodeExt<'dom> for ServoThreadSafeLayoutNode<'dom> {
.borrow_mut()
}
fn layout_data(&self) -> Option<AtomicRef<'dom, InnerDOMLayoutData>> {
ThreadSafeLayoutNode::layout_data(self).map(|data| {
fn inner_layout_data(&self) -> Option<AtomicRef<'dom, InnerDOMLayoutData>> {
self.layout_data().map(|data| {
data.as_any()
.downcast_ref::<DOMLayoutData>()
.unwrap()
@ -350,22 +384,38 @@ impl<'dom> NodeExt<'dom> for ServoThreadSafeLayoutNode<'dom> {
})
}
fn element_box_slot(&self) -> BoxSlot<'dom> {
self.layout_data_mut().self_box.clone().into()
}
fn box_slot(&self) -> BoxSlot<'dom> {
let pseudo_element_chain = self.pseudo_element_chain();
let Some(primary) = pseudo_element_chain.primary else {
return self.ensure_inner_layout_data().self_box.clone().into();
};
fn pseudo_element_box_slot(&self, pseudo_element: PseudoElement) -> BoxSlot<'dom> {
let mut layout_data = self.layout_data_mut();
let box_slot = ArcRefCell::new(None);
layout_data.pseudo_boxes.push(PseudoLayoutData {
pseudo: pseudo_element,
box_slot: box_slot.clone(),
});
box_slot.into()
let Some(secondary) = pseudo_element_chain.secondary else {
let primary_layout_data = self
.ensure_inner_layout_data()
.create_pseudo_layout_data(primary);
return primary_layout_data.borrow().self_box.clone().into();
};
// It's *very* important that this not borrow the element's main
// `InnerLayoutData`. Primary pseudo-elements are processed at the same recursion
// level as the main data, so the `BoxSlot` is created sequentially with other
// primary pseudo-elements and the element itself. The secondary pseudo-element is
// one level deep, so could be happening in parallel with the primary
// pseudo-elements or main element layout.
let primary_layout_data = self
.inner_layout_data()
.expect("Should already have element InnerLayoutData here.")
.pseudo_layout_data(primary)
.expect("Should already have primary pseudo-element InnerLayoutData here");
let secondary_layout_data = primary_layout_data
.borrow_mut()
.create_pseudo_layout_data(secondary);
secondary_layout_data.borrow().self_box.clone().into()
}
fn unset_all_boxes(&self) {
let mut layout_data = self.layout_data_mut();
let mut layout_data = self.ensure_inner_layout_data();
*layout_data.self_box.borrow_mut() = None;
layout_data.pseudo_boxes.clear();
@ -374,48 +424,31 @@ impl<'dom> NodeExt<'dom> for ServoThreadSafeLayoutNode<'dom> {
}
fn unset_all_pseudo_boxes(&self) {
self.layout_data_mut().pseudo_boxes.clear();
self.ensure_inner_layout_data().pseudo_boxes.clear();
}
fn clear_fragment_layout_cache(&self) {
let data = self.layout_data_mut();
if let Some(data) = data.self_box.borrow_mut().as_ref() {
data.clear_fragment_layout_cache();
}
for pseudo_layout_data in data.pseudo_boxes.iter() {
if let Some(layout_box) = pseudo_layout_data.box_slot.borrow().as_ref() {
layout_box.clear_fragment_layout_cache();
}
if let Some(inner_layout_data) = self.inner_layout_data() {
inner_layout_data.clear_fragment_layout_cache();
}
}
fn fragments_for_pseudo(&self, pseudo_element: Option<PseudoElement>) -> Vec<Fragment> {
let Some(layout_data) = NodeExt::layout_data(self) else {
let Some(layout_data) = self.inner_layout_data() else {
return vec![];
};
let Some(layout_data) = layout_data.for_pseudo(pseudo_element) else {
return vec![];
};
layout_data
.as_ref()
.map(LayoutBox::fragments)
.unwrap_or_default()
match pseudo_element {
Some(pseudo_element) => layout_data
.pseudo_layout_data(pseudo_element)
.map(|pseudo_layout_data| pseudo_layout_data.borrow().fragments())
.unwrap_or_default(),
None => layout_data.fragments(),
}
}
fn repair_style(&self, context: &SharedStyleContext) {
let data = self.layout_data_mut();
if let Some(layout_object) = &*data.self_box.borrow() {
let style = self.style(context);
layout_object.repair_style(context, self, &style);
}
for pseudo_layout_data in data.pseudo_boxes.iter() {
if let Some(layout_box) = pseudo_layout_data.box_slot.borrow().as_ref() {
if let Some(node) = self.with_pseudo(pseudo_layout_data.pseudo) {
layout_box.repair_style(context, self, &node.style(context));
}
}
if let Some(layout_data) = self.inner_layout_data() {
layout_data.repair_style(self, context);
}
}

32
components/layout/dom_traversal.rs
View file

@ -6,7 +6,9 @@ use std::borrow::Cow;
use fonts::ByteIndex;
use html5ever::{LocalName, local_name};
use layout_api::wrapper_traits::{ThreadSafeLayoutElement, ThreadSafeLayoutNode};
use layout_api::wrapper_traits::{
PseudoElementChain, ThreadSafeLayoutElement, ThreadSafeLayoutNode,
};
use layout_api::{LayoutDamage, LayoutElementType, LayoutNodeType};
use range::Range;
use script::layout_dom::ServoThreadSafeLayoutNode;
@ -48,8 +50,8 @@ impl<'dom> NodeAndStyleInfo<'dom> {
}
}
pub(crate) fn pseudo_element(&self) -> Option<PseudoElement> {
self.node.pseudo_element()
pub(crate) fn pseudo_element_chain(&self) -> PseudoElementChain {
self.node.pseudo_element_chain()
}
pub(crate) fn with_pseudo_element(
@ -74,7 +76,7 @@ impl<'dom> NodeAndStyleInfo<'dom> {
impl<'dom> From<&NodeAndStyleInfo<'dom>> for BaseFragmentInfo {
fn from(info: &NodeAndStyleInfo<'dom>) -> Self {
let threadsafe_node = info.node;
let pseudo = info.node.pseudo_element();
let pseudo_element_chain = info.node.pseudo_element_chain();
let mut flags = FragmentFlags::empty();
// Anonymous boxes should not have a tag, because they should not take part in hit testing.
@ -83,7 +85,7 @@ impl<'dom> From<&NodeAndStyleInfo<'dom>> for BaseFragmentInfo {
// cases, but currently this means that the order of hit test results isn't as expected for
// some WPT tests. This needs more investigation.
if matches!(
pseudo,
pseudo_element_chain.innermost(),
Some(PseudoElement::ServoAnonymousBox) |
Some(PseudoElement::ServoAnonymousTable) |
Some(PseudoElement::ServoAnonymousTableCell) |
@ -122,7 +124,10 @@ impl<'dom> From<&NodeAndStyleInfo<'dom>> for BaseFragmentInfo {
};
Self {
tag: Some(Tag::new_pseudo(threadsafe_node.opaque(), pseudo)),
tag: Some(Tag::new_pseudo(
threadsafe_node.opaque(),
pseudo_element_chain,
)),
flags,
}
}
@ -232,7 +237,7 @@ fn traverse_element<'dom>(
} else {
let shared_inline_styles: SharedInlineStyles = (&info).into();
element
.element_box_slot()
.box_slot()
.set(LayoutBox::DisplayContents(shared_inline_styles.clone()));
handler.enter_display_contents(shared_inline_styles);
@ -254,7 +259,7 @@ fn traverse_element<'dom>(
NonReplacedContents::OfElement.into()
};
let display = display.used_value_for_contents(&contents);
let box_slot = element.element_box_slot();
let box_slot = element.box_slot();
handler.handle_element(&info, display, contents, box_slot);
},
}
@ -282,9 +287,7 @@ fn traverse_eager_pseudo_element<'dom>(
Display::None => {},
Display::Contents => {
let items = generate_pseudo_element_content(&pseudo_element_info, context);
let box_slot = pseudo_element_info
.node
.pseudo_element_box_slot(pseudo_element_type);
let box_slot = pseudo_element_info.node.box_slot();
let shared_inline_styles: SharedInlineStyles = (&pseudo_element_info).into();
box_slot.set(LayoutBox::DisplayContents(shared_inline_styles.clone()));
@ -294,9 +297,7 @@ fn traverse_eager_pseudo_element<'dom>(
},
Display::GeneratingBox(display) => {
let items = generate_pseudo_element_content(&pseudo_element_info, context);
let box_slot = pseudo_element_info
.node
.pseudo_element_box_slot(pseudo_element_type);
let box_slot = pseudo_element_info.node.box_slot();
let contents = NonReplacedContents::OfPseudoElement(items).into();
handler.handle_element(&pseudo_element_info, display, contents, box_slot);
},
@ -333,8 +334,7 @@ fn traverse_pseudo_element_contents<'dom>(
anonymous_info,
display_inline,
Contents::Replaced(contents),
info.node
.pseudo_element_box_slot(PseudoElement::ServoAnonymousBox),
anonymous_info.node.box_slot(),
)
},
}

42
components/layout/flow/construct.rs
View file

@ -171,11 +171,10 @@ impl BlockContainer {
if let Some((marker_info, marker_contents)) = crate::lists::make_marker(context, info) {
match marker_info.style.clone_list_style_position() {
ListStylePosition::Inside => {
builder.handle_list_item_marker_inside(&marker_info, info, marker_contents)
builder.handle_list_item_marker_inside(&marker_info, marker_contents)
},
ListStylePosition::Outside => builder.handle_list_item_marker_outside(
&marker_info,
info,
marker_contents,
info.style.clone(),
),
@ -299,9 +298,7 @@ impl<'dom, 'style> BlockContainerBuilder<'dom, 'style> {
self.push_block_level_job_for_inline_formatting_context(inline_formatting_context);
}
let box_slot = table_info
.node
.pseudo_element_box_slot(PseudoElement::ServoAnonymousTable);
let box_slot = table_info.node.box_slot();
self.block_level_boxes.push(BlockLevelJob {
info: table_info,
box_slot,
@ -408,19 +405,9 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
fn handle_list_item_marker_inside(
&mut self,
marker_info: &NodeAndStyleInfo<'dom>,
container_info: &NodeAndStyleInfo<'dom>,
contents: Vec<crate::dom_traversal::PseudoElementContentItem>,
) {
// TODO: We do not currently support saving box slots for ::marker pseudo-elements
// that are part nested in ::before and ::after pseudo elements. For now, just
// forget about them once they are built.
let box_slot = match container_info.pseudo_element() {
Some(_) => BoxSlot::dummy(),
None => marker_info
.node
.pseudo_element_box_slot(PseudoElement::Marker),
};
let box_slot = marker_info.node.box_slot();
self.handle_inline_level_element(
marker_info,
DisplayInside::Flow {
@ -434,20 +421,10 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
fn handle_list_item_marker_outside(
&mut self,
marker_info: &NodeAndStyleInfo<'dom>,
container_info: &NodeAndStyleInfo<'dom>,
contents: Vec<crate::dom_traversal::PseudoElementContentItem>,
list_item_style: Arc<ComputedValues>,
) {
// TODO: We do not currently support saving box slots for ::marker pseudo-elements
// that are part nested in ::before and ::after pseudo elements. For now, just
// forget about them once they are built.
let box_slot = match container_info.pseudo_element() {
Some(_) => BoxSlot::dummy(),
None => marker_info
.node
.pseudo_element_box_slot(PseudoElement::Marker),
};
let box_slot = marker_info.node.box_slot();
self.block_level_boxes.push(BlockLevelJob {
info: marker_info.clone(),
box_slot,
@ -511,7 +488,7 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
// Ignore `list-style-position` here:
// “If the list item is an inline box: this value is equivalent to `inside`.”
// https://drafts.csswg.org/css-lists/#list-style-position-outside
self.handle_list_item_marker_inside(&marker_info, info, marker_contents)
self.handle_list_item_marker_inside(&marker_info, marker_contents)
}
}
@ -682,7 +659,7 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
inline_formatting_context: InlineFormattingContext,
) {
let layout_context = self.context;
let info = self
let anonymous_info = self
.anonymous_box_info
.get_or_insert_with(|| {
self.info
@ -691,12 +668,9 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
})
.clone();
let box_slot = self
.info
.node
.pseudo_element_box_slot(PseudoElement::ServoAnonymousBox);
let box_slot = anonymous_info.node.box_slot();
self.block_level_boxes.push(BlockLevelJob {
info,
info: anonymous_info,
box_slot,
kind: BlockLevelCreator::SameFormattingContextBlock(
IntermediateBlockContainer::InlineFormattingContext(

4
components/layout/flow/root.rs
View file

@ -190,7 +190,7 @@ fn construct_for_root_element(
let root_box = ArcRefCell::new(root_box);
root_element
.element_box_slot()
.box_slot()
.set(LayoutBox::BlockLevel(root_box.clone()));
vec![root_box]
}
@ -301,7 +301,7 @@ impl<'dom> IncrementalBoxTreeUpdate<'dom> {
return None;
}
let layout_data = NodeExt::layout_data(&potential_thread_safe_dirty_root_node)?;
let layout_data = NodeExt::inner_layout_data(&potential_thread_safe_dirty_root_node)?;
if !layout_data.pseudo_boxes.is_empty() {
return None;
}

18
components/layout/fragment_tree/base_fragment.rs
View file

@ -4,10 +4,10 @@
use bitflags::bitflags;
use layout_api::combine_id_with_fragment_type;
use layout_api::wrapper_traits::PseudoElementChain;
use malloc_size_of::malloc_size_of_is_0;
use malloc_size_of_derive::MallocSizeOf;
use style::dom::OpaqueNode;
use style::selector_parser::PseudoElement;
/// This data structure stores fields that are common to all non-base
/// Fragment types and should generally be the first member of all
@ -114,23 +114,29 @@ malloc_size_of_is_0!(FragmentFlags);
#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)]
pub(crate) struct Tag {
pub(crate) node: OpaqueNode,
pub(crate) pseudo: Option<PseudoElement>,
pub(crate) pseudo_element_chain: PseudoElementChain,
}
impl Tag {
/// Create a new Tag for a non-pseudo element. This is mainly used for
/// matching existing tags, since it does not accept an `info` argument.
pub(crate) fn new(node: OpaqueNode) -> Self {
Tag { node, pseudo: None }
Tag {
node,
pseudo_element_chain: Default::default(),
}
}
/// Create a new Tag for a pseudo element. This is mainly used for
/// matching existing tags, since it does not accept an `info` argument.
pub(crate) fn new_pseudo(node: OpaqueNode, pseudo: Option<PseudoElement>) -> Self {
Tag { node, pseudo }
pub(crate) fn new_pseudo(node: OpaqueNode, pseudo_element_chain: PseudoElementChain) -> Self {
Tag {
node,
pseudo_element_chain,
}
}
pub(crate) fn to_display_list_fragment_id(self) -> u64 {
combine_id_with_fragment_type(self.node.id(), self.pseudo.into())
combine_id_with_fragment_type(self.node.id(), self.pseudo_element_chain.primary.into())
}
}

11
components/layout/fragment_tree/fragment_tree.rs
View file

@ -71,8 +71,15 @@ impl FragmentTree {
fragment_tree.find(|fragment, _level, containing_block| {
if let Some(tag) = fragment.tag() {
invalid_animating_nodes.remove(&AnimationSetKey::new(tag.node, tag.pseudo));
invalid_image_animating_nodes.remove(&AnimationSetKey::new(tag.node, tag.pseudo));
// TODO: Support animations on nested pseudo-elements.
invalid_animating_nodes.remove(&AnimationSetKey::new(
tag.node,
tag.pseudo_element_chain.primary,
));
invalid_image_animating_nodes.remove(&AnimationSetKey::new(
tag.node,
tag.pseudo_element_chain.primary,
));
}
fragment.set_containing_block(containing_block);

12
components/layout/table/construct.rs
View file

@ -720,9 +720,9 @@ impl<'style, 'dom> TableBuilderTraversal<'style, 'dom> {
});
self.push_table_row(table_row.clone());
self.info
anonymous_info
.node
.pseudo_element_box_slot(PseudoElement::ServoAnonymousTableRow)
.box_slot()
.set(LayoutBox::TableLevelBox(TableLevelBox::Track(table_row)))
}
@ -989,9 +989,9 @@ impl<'style, 'builder, 'dom, 'a> TableRowBuilder<'style, 'builder, 'dom, 'a> {
.builder
.add_cell(new_table_cell.clone());
self.info
anonymous_info
.node
.pseudo_element_box_slot(PseudoElement::ServoAnonymousTableCell)
.box_slot()
.set(LayoutBox::TableLevelBox(TableLevelBox::Cell(
new_table_cell,
)));
@ -1027,7 +1027,7 @@ impl<'dom> TraversalHandler<'dom> for TableRowBuilder<'_, '_, 'dom, '_> {
let cell = old_cell.unwrap_or_else(|| {
// This value will already have filtered out rowspan=0
// in quirks mode, so we don't have to worry about that.
let (rowspan, colspan) = if info.pseudo_element().is_none() {
let (rowspan, colspan) = if info.pseudo_element_chain().is_empty() {
let rowspan = info.node.get_rowspan().unwrap_or(1) as usize;
let colspan = info.node.get_colspan().unwrap_or(1) as usize;
@ -1148,7 +1148,7 @@ fn add_column(
is_anonymous: bool,
old_column: Option<ArcRefCell<TableTrack>>,
) -> ArcRefCell<TableTrack> {
let span = if column_info.pseudo_element().is_none() {
let span = if column_info.pseudo_element_chain().is_empty() {
column_info.node.get_span().unwrap_or(1)
} else {
1