From c7c33f5f47fb9a062811c2ae20baa61b08ab5d26 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Wed, 30 Jul 2025 14:55:32 +0200 Subject: [PATCH] layout: Make `IndependentFormattingContext::contents` private (again) (#38350) This was done in #24871, but after some refactorings it became public. This makes it private again. As said in b2b3ea992c2f045a49d7264df18f5f85b2c913fb: > Privacy forces the rest of the code to go through methods > rather than matching on the enum, > reducing accidental layout-mode-specific behavior. It also avoids the risk of accidentally calling `layout()` on the inner layout-mode-specific struct, bypassing caching. Testing: Not needed (no behavior change) Signed-off-by: Oriol Brufau --- components/layout/construct_modern.rs | 8 +++---- components/layout/flow/inline/mod.rs | 11 +++------- components/layout/flow/mod.rs | 4 +--- components/layout/formatting_contexts.rs | 28 +++++++++++++++++------- components/layout/table/construct.rs | 15 +++++-------- 5 files changed, 34 insertions(+), 32 deletions(-) diff --git a/components/layout/construct_modern.rs b/components/layout/construct_modern.rs index bda7fed6174..bc0c78beefa 100644 --- a/components/layout/construct_modern.rs +++ b/components/layout/construct_modern.rs @@ -69,10 +69,10 @@ impl<'dom> ModernContainerJob<'dom> { BlockContainer::InlineFormattingContext(inline_formatting_context), ); let info: &NodeAndStyleInfo = anonymous_info; - let formatting_context = IndependentFormattingContext { - base: LayoutBoxBase::new(info.into(), info.style.clone()), - contents: IndependentFormattingContextContents::Flow(block_formatting_context), - }; + let formatting_context = IndependentFormattingContext::new( + LayoutBoxBase::new(info.into(), info.style.clone()), + IndependentFormattingContextContents::Flow(block_formatting_context), + ); Some(ModernItem { kind: ModernItemKind::InFlow(formatting_context), diff --git a/components/layout/flow/inline/mod.rs b/components/layout/flow/inline/mod.rs index 14405e4474c..94319fa3d9a 100644 --- a/components/layout/flow/inline/mod.rs +++ b/components/layout/flow/inline/mod.rs @@ -114,10 +114,7 @@ use webrender_api::FontInstanceKey; use xi_unicode::linebreak_property; use super::float::{Clear, PlacementAmongFloats}; -use super::{ - CacheableLayoutResult, IndependentFloatOrAtomicLayoutResult, - IndependentFormattingContextContents, -}; +use super::{CacheableLayoutResult, IndependentFloatOrAtomicLayoutResult}; use crate::cell::ArcRefCell; use crate::context::LayoutContext; use crate::dom_traversal::NodeAndStyleInfo; @@ -2151,10 +2148,8 @@ impl IndependentFormattingContext { match self.style().clone_baseline_source() { BaselineSource::First => baselines.first, BaselineSource::Last => baselines.last, - BaselineSource::Auto => match &self.contents { - IndependentFormattingContextContents::Flow(_) => baselines.last, - _ => baselines.first, - }, + BaselineSource::Auto if self.is_block_container() => baselines.last, + BaselineSource::Auto => baselines.first, } } diff --git a/components/layout/flow/mod.rs b/components/layout/flow/mod.rs index e7a0ae7f908..481247039c9 100644 --- a/components/layout/flow/mod.rs +++ b/components/layout/flow/mod.rs @@ -28,9 +28,7 @@ use crate::flow::float::{ Clear, ContainingBlockPositionInfo, FloatBox, FloatSide, PlacementAmongFloats, SequentialLayoutState, }; -use crate::formatting_contexts::{ - Baselines, IndependentFormattingContext, IndependentFormattingContextContents, -}; +use crate::formatting_contexts::{Baselines, IndependentFormattingContext}; use crate::fragment_tree::{ BaseFragmentInfo, BlockLevelLayoutInfo, BoxFragment, CollapsedBlockMargins, CollapsedMargin, Fragment, FragmentFlags, diff --git a/components/layout/formatting_contexts.rs b/components/layout/formatting_contexts.rs index 16cef35bbbe..8e97787c001 100644 --- a/components/layout/formatting_contexts.rs +++ b/components/layout/formatting_contexts.rs @@ -34,7 +34,9 @@ use crate::{ #[derive(Debug, MallocSizeOf)] pub(crate) struct IndependentFormattingContext { pub base: LayoutBoxBase, - pub contents: IndependentFormattingContextContents, + // Private so that code outside of this module cannot match variants. + // It should go through methods instead. + contents: IndependentFormattingContextContents, } #[derive(Debug, MallocSizeOf)] @@ -65,6 +67,10 @@ impl Baselines { } impl IndependentFormattingContext { + pub(crate) fn new(base: LayoutBoxBase, contents: IndependentFormattingContextContents) -> Self { + Self { base, contents } + } + pub fn construct( context: &LayoutContext, node_and_style_info: &NodeAndStyleInfo, @@ -139,13 +145,6 @@ impl IndependentFormattingContext { } } - pub fn is_replaced(&self) -> bool { - matches!( - self.contents, - IndependentFormattingContextContents::Replaced(_) - ) - } - #[inline] pub fn style(&self) -> &Arc { &self.base.style @@ -238,6 +237,19 @@ impl IndependentFormattingContext { } } + #[inline] + pub(crate) fn is_block_container(&self) -> bool { + matches!(self.contents, IndependentFormattingContextContents::Flow(_)) + } + + #[inline] + pub(crate) fn is_replaced(&self) -> bool { + matches!( + self.contents, + IndependentFormattingContextContents::Replaced(_) + ) + } + #[inline] pub(crate) fn is_table(&self) -> bool { matches!( diff --git a/components/layout/table/construct.rs b/components/layout/table/construct.rs index 72c1730f022..31f0689771c 100644 --- a/components/layout/table/construct.rs +++ b/components/layout/table/construct.rs @@ -118,10 +118,10 @@ impl Table { let mut table = table_builder.finish(); table.anonymous = true; - let ifc = IndependentFormattingContext { - base: LayoutBoxBase::new((&table_info).into(), table_style), - contents: IndependentFormattingContextContents::Table(table), - }; + let ifc = IndependentFormattingContext::new( + LayoutBoxBase::new((&table_info).into(), table_style), + IndependentFormattingContextContents::Table(table), + ); (table_info, ifc) } @@ -881,12 +881,9 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> { false, /* is_list_item */ ), ); - + let base = LayoutBoxBase::new(info.into(), info.style.clone()); ArcRefCell::new(TableCaption { - context: IndependentFormattingContext { - base: LayoutBoxBase::new(info.into(), info.style.clone()), - contents, - }, + context: IndependentFormattingContext::new(base, contents), }) });