From d33d0577632ec352eb01889d0b662691af7a3d76 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Thu, 3 Jul 2025 14:06:31 +0200 Subject: [PATCH] layout: Add incremental box tree construction for table cell (#37850) This change extends incremental box tree updates to table cells. In addition, for simplicity this refactors `BoxSlot::take_layout_box()` into `BoxSlot::take_layout_box_if_undamaged()`. Testing: This should not change observable behavior and is thus covered by existing WPT tests. Signed-off-by: Oriol Brufau Co-authored-by: Martin Robinson --- components/layout/dom.rs | 5 +- components/layout/table/construct.rs | 94 +++++++++++++++------------- 2 files changed, 54 insertions(+), 45 deletions(-) diff --git a/components/layout/dom.rs b/components/layout/dom.rs index 6a1aab4e7a6..40023439982 100644 --- a/components/layout/dom.rs +++ b/components/layout/dom.rs @@ -181,7 +181,10 @@ impl BoxSlot<'_> { } } - pub(crate) fn take_layout_box(&self) -> Option { + pub(crate) fn take_layout_box_if_undamaged(&self, damage: LayoutDamage) -> Option { + if damage.has_box_damage() { + return None; + } self.slot.as_ref().and_then(|slot| slot.borrow_mut().take()) } } diff --git a/components/layout/table/construct.rs b/components/layout/table/construct.rs index aa5c97c8107..a0b0711f0ef 100644 --- a/components/layout/table/construct.rs +++ b/components/layout/table/construct.rs @@ -855,14 +855,11 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> { unreachable!("Replaced should not have a LayoutInternal display type."); }; - let old_caption = (!info.damage.has_box_damage()) - .then(|| match box_slot.take_layout_box() { - Some(LayoutBox::TableLevelBox(TableLevelBox::Caption(caption))) => { - Some(caption) - }, - _ => None, - }) - .flatten(); + let old_box = box_slot.take_layout_box_if_undamaged(info.damage); + let old_caption = old_box.and_then(|layout_box| match layout_box { + LayoutBox::TableLevelBox(TableLevelBox::Caption(caption)) => Some(caption), + _ => None, + }); let caption = old_caption.unwrap_or_else(|| { let contents = IndependentNonReplacedContents::Flow( @@ -1006,45 +1003,54 @@ impl<'dom> TraversalHandler<'dom> for TableRowBuilder<'_, '_, 'dom, '_> { match display { DisplayGeneratingBox::LayoutInternal(internal) => match internal { DisplayLayoutInternal::TableCell => { - // 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_type.is_none() { - let node = info.node.to_threadsafe(); - let rowspan = node.get_rowspan().unwrap_or(1) as usize; - let colspan = node.get_colspan().unwrap_or(1) as usize; - - // The HTML specification clamps value of `rowspan` to [0, 65534] and - // `colspan` to [1, 1000]. - assert!((1..=1000).contains(&colspan)); - assert!((0..=65534).contains(&rowspan)); - - (rowspan, colspan) - } else { - (1, 1) - }; - - let propagated_data = - self.propagated_data.disallowing_percentage_table_columns(); - let Contents::NonReplaced(non_replaced_contents) = contents else { - unreachable!("Replaced should not have a LayoutInternal display type."); - }; - - let contents = BlockFormattingContext::construct( - self.table_traversal.context, - info, - non_replaced_contents, - propagated_data, - false, /* is_list_item */ - ); - self.finish_current_anonymous_cell_if_needed(); - let cell = ArcRefCell::new(TableSlotCell { - base: LayoutBoxBase::new(info.into(), info.style.clone()), - contents, - colspan, - rowspan, + let old_box = box_slot.take_layout_box_if_undamaged(info.damage); + let old_cell = old_box.and_then(|layout_box| match layout_box { + LayoutBox::TableLevelBox(TableLevelBox::Cell(cell)) => Some(cell), + _ => None, }); + + 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_type.is_none() { + let node = info.node.to_threadsafe(); + let rowspan = node.get_rowspan().unwrap_or(1) as usize; + let colspan = node.get_colspan().unwrap_or(1) as usize; + + // The HTML specification clamps value of `rowspan` to [0, 65534] and + // `colspan` to [1, 1000]. + assert!((1..=1000).contains(&colspan)); + assert!((0..=65534).contains(&rowspan)); + + (rowspan, colspan) + } else { + (1, 1) + }; + + let propagated_data = + self.propagated_data.disallowing_percentage_table_columns(); + let Contents::NonReplaced(non_replaced_contents) = contents else { + unreachable!("Replaced should not have a LayoutInternal display type."); + }; + + let contents = BlockFormattingContext::construct( + self.table_traversal.context, + info, + non_replaced_contents, + propagated_data, + false, /* is_list_item */ + ); + + ArcRefCell::new(TableSlotCell { + base: LayoutBoxBase::new(info.into(), info.style.clone()), + contents, + colspan, + rowspan, + }) + }); + self.table_traversal.builder.add_cell(cell.clone()); box_slot.set(LayoutBox::TableLevelBox(TableLevelBox::Cell(cell))); },