Distinguish cached inline_content_sizes() from uncached ones (servo…

…#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 <[email protected]>

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, || {
-            self.block_container
-                .inline_content_sizes(layout_context, &constraint_space)
-        });
+        let content_sizes = self.base.inline_content_sizes(
+            layout_context,
+            &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, || {
-                            contents.inline_content_sizes(layout_context, constraint_space)
-                        })
+                        base.inline_content_sizes(layout_context, constraint_space, contents)
                     },
                 );
                 // 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
-            .inline_content_sizes(constraint_space, || match &self.contents {
-                IndependentFormattingContextContents::NonReplaced(contents) => {
-                    contents.inline_content_sizes(layout_context, constraint_space)
-                },
-                IndependentFormattingContextContents::Replaced(contents) => {
-                    contents.inline_content_sizes(layout_context, constraint_space)
-                },
-            })
+        match &self.contents {
+            IndependentFormattingContextContents::NonReplaced(contents) => self
+                .base
+                .inline_content_sizes(layout_context, constraint_space, contents),
+            IndependentFormattingContextContents::Replaced(contents) => self
+                .base
+                .inline_content_sizes(layout_context, constraint_space, contents),
+        }
     }
 
     pub(crate) fn outer_inline_content_sizes(
@@ -270,19 +269,27 @@ 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,
     ) -> InlineContentSizesResult {
         match self {
             Self::Flow(inner) => inner
                 .contents
-                .inline_content_sizes(layout_context, constraint_space),
-            Self::Flex(inner) => inner.inline_content_sizes(layout_context, constraint_space),
-            Self::Grid(inner) => inner.inline_content_sizes(layout_context, constraint_space),
-            Self::Table(table) => table.inline_content_sizes(layout_context, constraint_space),
+                .compute_inline_content_sizes(layout_context, constraint_space),
+            Self::Flex(inner) => {
+                inner.compute_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;
+}