Distinguish cached inline_content_sizes() from uncached ones (#34595)

Several structs and enums had a `inline_content_sizes()` method, but it
wasn't clear which ones would try to cache the result, and which ones
would always compute it.

Therefore, this performs some clarifying renaming:
 - Cached ones stay as `inline_content_sizes()`
 - Uncached ones become `compute_inline_content_sizes()`

Also, to simplify calls to `LayoutBoxBase::inline_content_sizes()`,
`compute_inline_content_sizes()` is moved into a new trait.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>

components/layout_2020/flexbox/layout.rs

@@ -38,7 +38,9 @@ use crate::geom::{AuOrAuto, LogicalRect, LogicalSides, LogicalVec2, Size};
 use crate::positioned::{
     relative_adjustement, AbsolutelyPositionedBox, PositioningContext, PositioningContextLength,
 };
-use crate::sizing::{ContentSizes, InlineContentSizesResult, IntrinsicSizingMode};
+use crate::sizing::{
+    ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult, IntrinsicSizingMode,
+};
 use crate::style_ext::{
     AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBMDeprecated, PaddingBorderMargin,
 };
@@ -411,17 +413,17 @@ struct FlexItemBoxInlineContentSizesInfo {
     depends_on_block_constraints: bool,
 }
 
-impl FlexContainer {
+impl ComputeInlineContentSizes for FlexContainer {
     #[cfg_attr(
         feature = "tracing",
         tracing::instrument(
-            name = "FlexContainer::inline_content_sizes",
+            name = "FlexContainer::compute_inline_content_sizes",
             skip_all,
             fields(servo_profiling = true),
             level = "trace",
         )
     )]
-    pub fn inline_content_sizes(
+    fn compute_inline_content_sizes(
         &self,
         layout_context: &LayoutContext,
         constraint_space: &ConstraintSpace,
@@ -437,7 +439,9 @@ impl FlexContainer {
             FlexAxis::Column => self.cross_content_sizes(layout_context, &constraint_space.into()),
         }
     }
+}
 
+impl FlexContainer {
     fn cross_content_sizes(
         &self,
         layout_context: &LayoutContext,

components/layout_2020/flow/inline/mod.rs

@@ -126,7 +126,7 @@ use crate::fragment_tree::{
 };
 use crate::geom::{LogicalRect, LogicalVec2, ToLogical};
 use crate::positioned::{AbsolutelyPositionedBox, PositioningContext};
-use crate::sizing::{ContentSizes, InlineContentSizesResult};
+use crate::sizing::{ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::{ComputedValuesExt, PaddingBorderMargin};
 use crate::{ConstraintSpace, ContainingBlock};
 
@@ -1579,17 +1579,6 @@ impl InlineFormattingContext {
         }
     }
 
-    // This works on an already-constructed `InlineFormattingContext`,
-    // Which would have to change if/when
-    // `BlockContainer::construct` parallelize their construction.
-    pub(super) fn inline_content_sizes(
-        &self,
-        layout_context: &LayoutContext,
-        constraint_space: &ConstraintSpace,
-    ) -> InlineContentSizesResult {
-        ContentSizesComputation::compute(self, layout_context, constraint_space)
-    }
-
     pub(super) fn layout(
         &self,
         layout_context: &LayoutContext,
@@ -2194,6 +2183,19 @@ fn inline_container_needs_strut(
         .unwrap_or(false)
 }
 
+impl ComputeInlineContentSizes for InlineFormattingContext {
+    // This works on an already-constructed `InlineFormattingContext`,
+    // Which would have to change if/when
+    // `BlockContainer::construct` parallelize their construction.
+    fn compute_inline_content_sizes(
+        &self,
+        layout_context: &LayoutContext,
+        constraint_space: &ConstraintSpace,
+    ) -> InlineContentSizesResult {
+        ContentSizesComputation::compute(self, layout_context, constraint_space)
+    }
+}
+
 /// A struct which takes care of computing [`ContentSizes`] for an [`InlineFormattingContext`].
 struct ContentSizesComputation<'layout_data> {
     layout_context: &'layout_data LayoutContext<'layout_data>,

components/layout_2020/flow/mod.rs

@@ -40,7 +40,7 @@ use crate::geom::{
 use crate::layout_box_base::LayoutBoxBase;
 use crate::positioned::{AbsolutelyPositionedBox, PositioningContext, PositioningContextLength};
 use crate::replaced::ReplacedContents;
-use crate::sizing::{self, ContentSizes, InlineContentSizesResult};
+use crate::sizing::{self, ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::{
     Clamp, ComputedValuesExt, ContentBoxSizesAndPBMDeprecated, PaddingBorderMargin,
 };
@@ -246,10 +246,11 @@ impl OutsideMarker {
             &self.marker_style,
             None, /* TODO: support preferred aspect ratios on non-replaced boxes */
         );
-        let content_sizes = self.base.inline_content_sizes(&constraint_space, || {
+        let content_sizes = self.base.inline_content_sizes(
-            self.block_container
+            layout_context,
-                .inline_content_sizes(layout_context, &constraint_space)
+            &constraint_space,
-        });
+            &self.block_container,
+        );
         let containing_block_for_children = ContainingBlock {
             size: ContainingBlockSize {
                 inline: content_sizes.sizes.max_content,
@@ -379,7 +380,7 @@ impl BlockFormattingContext {
 /// But floats can flow horizontally depending on 'clear', so we may need to sum their sizes.
 /// CSS 2 does not define the exact algorithm, this logic is based on the behavior observed
 /// on Gecko and Blink.
-fn calculate_inline_content_size_for_block_level_boxes(
+fn compute_inline_content_sizes_for_block_level_boxes(
     boxes: &[ArcRefCell<BlockLevelBox>],
     layout_context: &LayoutContext,
     containing_block: &IndefiniteContainingBlock,
@@ -416,9 +417,7 @@ fn calculate_inline_content_size_for_block_level_boxes(
                     false,    /* auto_block_size_stretches_to_containing_block */
                     |_| None, /* TODO: support preferred aspect ratios on non-replaced boxes */
                     |constraint_space| {
-                        base.inline_content_sizes(constraint_space, || {
+                        base.inline_content_sizes(layout_context, constraint_space, contents)
-                            contents.inline_content_sizes(layout_context, constraint_space)
-                        })
                     },
                 );
                 // A block in the same BFC can overlap floats, it's not moved next to them,
@@ -552,20 +551,22 @@ impl BlockContainer {
             ),
         }
     }
+}
 
-    pub(super) fn inline_content_sizes(
+impl ComputeInlineContentSizes for BlockContainer {
+    fn compute_inline_content_sizes(
         &self,
         layout_context: &LayoutContext,
         constraint_space: &ConstraintSpace,
     ) -> InlineContentSizesResult {
         match &self {
-            Self::BlockLevelBoxes(boxes) => calculate_inline_content_size_for_block_level_boxes(
+            Self::BlockLevelBoxes(boxes) => compute_inline_content_sizes_for_block_level_boxes(
                 boxes,
                 layout_context,
                 &constraint_space.into(),
             ),
             Self::InlineFormattingContext(context) => {
-                context.inline_content_sizes(layout_context, constraint_space)
+                context.compute_inline_content_sizes(layout_context, constraint_space)
             },
         }
     }

components/layout_2020/formatting_contexts.rs

@@ -19,7 +19,7 @@ use crate::geom::LogicalSides;
 use crate::layout_box_base::LayoutBoxBase;
 use crate::positioned::PositioningContext;
 use crate::replaced::ReplacedContents;
-use crate::sizing::{self, InlineContentSizesResult};
+use crate::sizing::{self, ComputeInlineContentSizes, InlineContentSizesResult};
 use crate::style_ext::{AspectRatio, DisplayInside};
 use crate::table::Table;
 use crate::taffy::TaffyContainer;
@@ -187,15 +187,14 @@ impl IndependentFormattingContext {
         layout_context: &LayoutContext,
         constraint_space: &ConstraintSpace,
     ) -> InlineContentSizesResult {
-        self.base
+        match &self.contents {
-            .inline_content_sizes(constraint_space, || match &self.contents {
+            IndependentFormattingContextContents::NonReplaced(contents) => self
-                IndependentFormattingContextContents::NonReplaced(contents) => {
+                .base
-                    contents.inline_content_sizes(layout_context, constraint_space)
+                .inline_content_sizes(layout_context, constraint_space, contents),
-                },
+            IndependentFormattingContextContents::Replaced(contents) => self
-                IndependentFormattingContextContents::Replaced(contents) => {
+                .base
-                    contents.inline_content_sizes(layout_context, constraint_space)
+                .inline_content_sizes(layout_context, constraint_space, contents),
-                },
+        }
-            })
     }
 
     pub(crate) fn outer_inline_content_sizes(
@@ -270,8 +269,10 @@ impl IndependentNonReplacedContents {
         // TODO: support preferred aspect ratios on non-replaced boxes.
         None
     }
+}
 
-    pub(crate) fn inline_content_sizes(
+impl ComputeInlineContentSizes for IndependentNonReplacedContents {
+    fn compute_inline_content_sizes(
         &self,
         layout_context: &LayoutContext,
         constraint_space: &ConstraintSpace,
@@ -279,10 +280,16 @@ impl IndependentNonReplacedContents {
         match self {
             Self::Flow(inner) => inner
                 .contents
-                .inline_content_sizes(layout_context, constraint_space),
+                .compute_inline_content_sizes(layout_context, constraint_space),
-            Self::Flex(inner) => inner.inline_content_sizes(layout_context, constraint_space),
+            Self::Flex(inner) => {
-            Self::Grid(inner) => inner.inline_content_sizes(layout_context, constraint_space),
+                inner.compute_inline_content_sizes(layout_context, constraint_space)
-            Self::Table(table) => table.inline_content_sizes(layout_context, constraint_space),
+            },
+            Self::Grid(inner) => {
+                inner.compute_inline_content_sizes(layout_context, constraint_space)
+            },
+            Self::Table(inner) => {
+                inner.compute_inline_content_sizes(layout_context, constraint_space)
+            },
         }
     }
 }

components/layout_2020/layout_box_base.rs

@@ -7,9 +7,10 @@ use serde::Serialize;
 use servo_arc::Arc;
 use style::properties::ComputedValues;
 
+use crate::context::LayoutContext;
 use crate::fragment_tree::BaseFragmentInfo;
 use crate::geom::SizeConstraint;
-use crate::sizing::InlineContentSizesResult;
+use crate::sizing::{ComputeInlineContentSizes, InlineContentSizesResult};
 use crate::ConstraintSpace;
 
 /// A box tree node that handles containing information about style and the original DOM
@@ -41,8 +42,9 @@ impl LayoutBoxBase {
     /// the result from a cache when possible.
     pub(crate) fn inline_content_sizes(
         &self,
+        layout_context: &LayoutContext,
         constraint_space: &ConstraintSpace,
-        inline_content_sizes_fn: impl FnOnce() -> InlineContentSizesResult,
+        layout_box: &impl ComputeInlineContentSizes,
     ) -> InlineContentSizesResult {
         let mut cache = self.cached_inline_content_size.borrow_mut();
         if let Some((previous_cb_block_size, result)) = *cache {
@@ -54,7 +56,7 @@ impl LayoutBoxBase {
             // TODO: Should we keep multiple caches for various block sizes?
         }
 
-        let result = inline_content_sizes_fn();
+        let result = layout_box.compute_inline_content_sizes(layout_context, constraint_space);
         *cache = Some((constraint_space.block_size, result));
         result
     }

components/layout_2020/replaced.rs

@@ -29,7 +29,7 @@ use crate::context::LayoutContext;
 use crate::dom::NodeExt;
 use crate::fragment_tree::{BaseFragmentInfo, Fragment, IFrameFragment, ImageFragment};
 use crate::geom::{LogicalVec2, PhysicalPoint, PhysicalRect, PhysicalSize, Size};
-use crate::sizing::{ContentSizes, InlineContentSizesResult};
+use crate::sizing::{ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::{AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBM};
 use crate::{ConstraintSpace, ContainingBlock, SizeConstraint};
 
@@ -278,29 +278,6 @@ impl ReplacedContents {
         }
     }
 
-    pub fn inline_content_sizes(
-        &self,
-        _: &LayoutContext,
-        constraint_space: &ConstraintSpace,
-    ) -> InlineContentSizesResult {
-        let get_inline_fallback_size = || {
-            let writing_mode = constraint_space.writing_mode;
-            self.flow_relative_natural_size(writing_mode)
-                .inline
-                .unwrap_or_else(|| Self::flow_relative_default_object_size(writing_mode).inline)
-        };
-        let inline_content_size = self.content_size(
-            Direction::Inline,
-            constraint_space.preferred_aspect_ratio,
-            &|| constraint_space.block_size,
-            &get_inline_fallback_size,
-        );
-        InlineContentSizesResult {
-            sizes: inline_content_size.into(),
-            depends_on_block_constraints: constraint_space.preferred_aspect_ratio.is_some(),
-        }
-    }
-
     pub fn make_fragments(
         &self,
         style: &ServoArc<ComputedValues>,
@@ -606,6 +583,31 @@ impl ReplacedContents {
     }
 }
 
+impl ComputeInlineContentSizes for ReplacedContents {
+    fn compute_inline_content_sizes(
+        &self,
+        _: &LayoutContext,
+        constraint_space: &ConstraintSpace,
+    ) -> InlineContentSizesResult {
+        let get_inline_fallback_size = || {
+            let writing_mode = constraint_space.writing_mode;
+            self.flow_relative_natural_size(writing_mode)
+                .inline
+                .unwrap_or_else(|| Self::flow_relative_default_object_size(writing_mode).inline)
+        };
+        let inline_content_size = self.content_size(
+            Direction::Inline,
+            constraint_space.preferred_aspect_ratio,
+            &|| constraint_space.block_size,
+            &get_inline_fallback_size,
+        );
+        InlineContentSizesResult {
+            sizes: inline_content_size.into(),
+            depends_on_block_constraints: constraint_space.preferred_aspect_ratio.is_some(),
+        }
+    }
+}
+
 fn try_to_parse_image_data_url(string: &str) -> Option<Url> {
     if !string.starts_with("data:") {
         return None;

components/layout_2020/sizing.rs

@@ -12,6 +12,7 @@ use serde::Serialize;
 use style::properties::ComputedValues;
 use style::Zero;
 
+use crate::context::LayoutContext;
 use crate::geom::Size;
 use crate::style_ext::{AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBM};
 use crate::{ConstraintSpace, IndefiniteContainingBlock, LogicalVec2, SizeConstraint};
@@ -216,3 +217,11 @@ pub(crate) struct InlineContentSizesResult {
     pub sizes: ContentSizes,
     pub depends_on_block_constraints: bool,
 }
+
+pub(crate) trait ComputeInlineContentSizes {
+    fn compute_inline_content_sizes(
+        &self,
+        layout_context: &LayoutContext,
+        constraint_space: &ConstraintSpace,
+    ) -> InlineContentSizesResult;
+}

components/layout_2020/table/layout.rs

@@ -36,7 +36,7 @@ use crate::geom::{
     Size, ToLogical, ToLogicalWithContainingBlock,
 };
 use crate::positioned::{relative_adjustement, PositioningContext, PositioningContextLength};
-use crate::sizing::{ContentSizes, InlineContentSizesResult};
+use crate::sizing::{ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::{Clamp, ComputedValuesExt, PaddingBorderMargin};
 use crate::table::TableSlotCoordinates;
 use crate::{
@@ -2621,52 +2621,6 @@ impl Table {
         }
     }
 
-    #[cfg_attr(
-        feature = "tracing",
-        tracing::instrument(
-            name = "Table::inline_content_sizes",
-            skip_all,
-            fields(servo_profiling = true),
-            level = "trace",
-        )
-    )]
-    pub(crate) fn inline_content_sizes(
-        &self,
-        layout_context: &LayoutContext,
-        constraint_space: &ConstraintSpace,
-    ) -> InlineContentSizesResult {
-        let writing_mode = constraint_space.writing_mode;
-        let mut layout = TableLayout::new(self);
-        let mut table_content_sizes = layout.compute_grid_min_max(layout_context, writing_mode);
-
-        let mut caption_minimum_inline_size =
-            layout.compute_caption_minimum_inline_size(layout_context);
-        if caption_minimum_inline_size > table_content_sizes.min_content ||
-            caption_minimum_inline_size > table_content_sizes.max_content
-        {
-            // Padding and border should apply to the table grid, but they will be taken into
-            // account when computing the inline content sizes of the table wrapper (our parent), so
-            // this code removes their contribution from the inline content size of the caption.
-            let padding = self
-                .style
-                .padding(writing_mode)
-                .percentages_relative_to(Au::zero());
-            let border = self.style.border_width(writing_mode);
-            caption_minimum_inline_size -= padding.inline_sum() + border.inline_sum();
-            table_content_sizes
-                .min_content
-                .max_assign(caption_minimum_inline_size);
-            table_content_sizes
-                .max_content
-                .max_assign(caption_minimum_inline_size);
-        }
-
-        InlineContentSizesResult {
-            sizes: table_content_sizes,
-            depends_on_block_constraints: false,
-        }
-    }
-
     fn get_column_measure_for_column_at_index(
         &self,
         writing_mode: WritingMode,
@@ -2740,6 +2694,54 @@ impl Table {
     }
 }
 
+impl ComputeInlineContentSizes for Table {
+    #[cfg_attr(
+        feature = "tracing",
+        tracing::instrument(
+            name = "Table::compute_inline_content_sizes",
+            skip_all,
+            fields(servo_profiling = true),
+            level = "trace",
+        )
+    )]
+    fn compute_inline_content_sizes(
+        &self,
+        layout_context: &LayoutContext,
+        constraint_space: &ConstraintSpace,
+    ) -> InlineContentSizesResult {
+        let writing_mode = constraint_space.writing_mode;
+        let mut layout = TableLayout::new(self);
+        let mut table_content_sizes = layout.compute_grid_min_max(layout_context, writing_mode);
+
+        let mut caption_minimum_inline_size =
+            layout.compute_caption_minimum_inline_size(layout_context);
+        if caption_minimum_inline_size > table_content_sizes.min_content ||
+            caption_minimum_inline_size > table_content_sizes.max_content
+        {
+            // Padding and border should apply to the table grid, but they will be taken into
+            // account when computing the inline content sizes of the table wrapper (our parent), so
+            // this code removes their contribution from the inline content size of the caption.
+            let padding = self
+                .style
+                .padding(writing_mode)
+                .percentages_relative_to(Au::zero());
+            let border = self.style.border_width(writing_mode);
+            caption_minimum_inline_size -= padding.inline_sum() + border.inline_sum();
+            table_content_sizes
+                .min_content
+                .max_assign(caption_minimum_inline_size);
+            table_content_sizes
+                .max_content
+                .max_assign(caption_minimum_inline_size);
+        }
+
+        InlineContentSizesResult {
+            sizes: table_content_sizes,
+            depends_on_block_constraints: false,
+        }
+    }
+}
+
 impl TableSlotCell {
     fn effective_vertical_align(&self) -> VerticalAlignKeyword {
         match self.base.style.clone_vertical_align() {
@@ -2764,11 +2766,7 @@ impl TableSlotCell {
             None, /* TODO: support preferred aspect ratios on non-replaced boxes */
         );
         self.base
-            .inline_content_sizes(&constraint_space, || {
+            .inline_content_sizes(layout_context, &constraint_space, &self.contents.contents)
-                self.contents
-                    .contents
-                    .inline_content_sizes(layout_context, &constraint_space)
-            })
             .sizes
     }
 

components/layout_2020/taffy/layout.rs

@@ -25,7 +25,7 @@ use crate::geom::{
     SizeConstraint,
 };
 use crate::positioned::{AbsolutelyPositionedBox, PositioningContext, PositioningContextLength};
-use crate::sizing::{ContentSizes, InlineContentSizesResult};
+use crate::sizing::{ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::ComputedValuesExt;
 use crate::{ConstraintSpace, ContainingBlock, ContainingBlockSize};
 
@@ -307,8 +307,8 @@ impl taffy::LayoutGridContainer for TaffyContainerContext<'_> {
     }
 }
 
-impl TaffyContainer {
+impl ComputeInlineContentSizes for TaffyContainer {
-    pub fn inline_content_sizes(
+    fn compute_inline_content_sizes(
         &self,
         layout_context: &LayoutContext,
         _constraint_space: &ConstraintSpace,
@@ -382,7 +382,9 @@ impl TaffyContainer {
             depends_on_block_constraints: true,
         }
     }
+}
 
+impl TaffyContainer {
     /// <https://drafts.csswg.org/css-grid/#layout-algorithm>
     pub(crate) fn layout(
         &self,