Bug 1581467 - Do not use synthetic display-inside values. r=TYLin

This matches the new servo layout engine too, and thus removes some #[cfg] gunk. Just use `flow` since it doesn't simplify the layout code as much. Differential Revision: https://phabricator.services.mozilla.com/D45973 --HG-- extra : moz-landing-system : lando
dothq · Dec 19, 2019 · cae1e55 · cae1e55
1 parent 02e8868
commit cae1e55
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 58 deletions.
diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
@@ -4300,8 +4300,15 @@ nsCSSFrameConstructor::FindDisplayData(const nsStyleDisplay& aDisplay,
   }
 
   switch (aDisplay.DisplayInside()) {
-    case StyleDisplayInside::Block:
+    case StyleDisplayInside::Flow:
     case StyleDisplayInside::FlowRoot: {
+      if (aDisplay.IsInlineFlow()) {
+        static const FrameConstructionData data =
+            FULL_CTOR_FCDATA(FCDATA_IS_INLINE | FCDATA_IS_LINE_PARTICIPANT,
+                             &nsCSSFrameConstructor::ConstructInline);
+        return &data;
+      }
+
       // If the frame is a block-level frame and is scrollable, then wrap it in
       // a scroll frame.  Except we don't want to do that for paginated contexts
       // for frames that are block-outside and aren't frames for native
@@ -4352,12 +4359,6 @@ nsCSSFrameConstructor::FindDisplayData(const nsStyleDisplay& aDisplay,
                &nsCSSFrameConstructor::ConstructNonScrollableBlock)}};
       return &sNonScrollableBlockData[suppressScrollFrame][caption];
     }
-    case StyleDisplayInside::Inline: {
-      static const FrameConstructionData data =
-          FULL_CTOR_FCDATA(FCDATA_IS_INLINE | FCDATA_IS_LINE_PARTICIPANT,
-                           &nsCSSFrameConstructor::ConstructInline);
-      return &data;
-    }
     case StyleDisplayInside::Table: {
       static const FrameConstructionData data =
           FULL_CTOR_FCDATA(0, &nsCSSFrameConstructor::ConstructTable);
@@ -7116,8 +7117,7 @@ void nsCSSFrameConstructor::ContentRangeInserted(
   // Examine the insertion.mParentFrame where the insertion is taking
   // place. If it's a certain kind of container then some special
   // processing is done.
-  if (StyleDisplayInside::Block == parentDisplayInside ||
-      StyleDisplayInside::Inline == parentDisplayInside) {
+  if (StyleDisplayInside::Flow == parentDisplayInside) {
     // Recover the special style flags for the containing block
     if (containingBlock) {
       haveFirstLetterStyle = HasFirstLetterStyle(containingBlock);
@@ -9322,12 +9322,13 @@ void nsCSSFrameConstructor::WrapItemsInPseudoParent(
     const FCItemIterator& aEndIter) {
   const PseudoParentData& pseudoData = sPseudoParentData[aWrapperType];
   PseudoStyleType pseudoType = pseudoData.mPseudoType;
-  auto parentDisplayInside = aParentStyle->StyleDisplay()->DisplayInside();
+  auto& parentDisplay = *aParentStyle->StyleDisplay();
+  auto parentDisplayInside = parentDisplay.DisplayInside();
 
   // XXXmats should we use IsInlineInsideStyle() here instead? seems odd to
   // exclude RubyBaseContainer/RubyTextContainer...
   if (pseudoType == PseudoStyleType::table &&
-      (parentDisplayInside == StyleDisplayInside::Inline ||
+      (parentDisplay.IsInlineFlow() ||
        parentDisplayInside == StyleDisplayInside::RubyBase ||
        parentDisplayInside == StyleDisplayInside::RubyText)) {
     pseudoType = PseudoStyleType::inlineTable;

diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp
@@ -5116,7 +5116,7 @@ void nsTextFrame::GetTextDecorations(
     // If we're on a ruby frame other than ruby text container, we
     // should continue.
     mozilla::StyleDisplay display = f->GetDisplay();
-    if (nsStyleDisplay::DisplayInside(display) != StyleDisplayInside::Inline &&
+    if (!nsStyleDisplay::IsInlineFlow(display) &&
         (!nsStyleDisplay::IsRubyDisplayType(display) ||
          display == mozilla::StyleDisplay::RubyTextContainer) &&
         nsStyleDisplay::IsDisplayTypeInlineOutside(display)) {

diff --git a/layout/style/nsStyleConsts.h b/layout/style/nsStyleConsts.h
@@ -39,9 +39,9 @@ enum class StyleDisplay : uint16_t {
   Contents =
       StyleDisplayFrom(StyleDisplayOutside::None, StyleDisplayInside::Contents),
   Inline =
-      StyleDisplayFrom(StyleDisplayOutside::Inline, StyleDisplayInside::Inline),
+      StyleDisplayFrom(StyleDisplayOutside::Inline, StyleDisplayInside::Flow),
   Block =
-      StyleDisplayFrom(StyleDisplayOutside::Block, StyleDisplayInside::Block),
+      StyleDisplayFrom(StyleDisplayOutside::Block, StyleDisplayInside::Flow),
   FlowRoot = StyleDisplayFrom(StyleDisplayOutside::Block,
                               StyleDisplayInside::FlowRoot),
   Flex = StyleDisplayFrom(StyleDisplayOutside::Block, StyleDisplayInside::Flex),
@@ -51,7 +51,7 @@ enum class StyleDisplay : uint16_t {
   InlineTable =
       StyleDisplayFrom(StyleDisplayOutside::Inline, StyleDisplayInside::Table),
   TableCaption = StyleDisplayFrom(StyleDisplayOutside::TableCaption,
-                                  StyleDisplayInside::Block),
+                                  StyleDisplayInside::Flow),
   Ruby =
       StyleDisplayFrom(StyleDisplayOutside::Inline, StyleDisplayInside::Ruby),
   WebkitBox = StyleDisplayFrom(StyleDisplayOutside::Block,

diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h
@@ -1645,15 +1645,15 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay {
 
   // Whether display is `inline` or `inline list-item`.
   static bool IsInlineFlow(mozilla::StyleDisplay aDisplay) {
-    return DisplayInside(aDisplay) == mozilla::StyleDisplayInside::Inline;
+    return DisplayOutside(aDisplay) == mozilla::StyleDisplayOutside::Inline &&
+           DisplayInside(aDisplay) == mozilla::StyleDisplayInside::Flow;
   }
 
   bool IsInlineFlow() const { return IsInlineFlow(mDisplay); }
 
   bool IsInlineInsideStyle() const {
     auto inside = DisplayInside();
-    return inside == mozilla::StyleDisplayInside::Inline ||
-           inside == mozilla::StyleDisplayInside::Ruby ||
+    return IsInlineFlow() || inside == mozilla::StyleDisplayInside::Ruby ||
            inside == mozilla::StyleDisplayInside::RubyBase ||
            inside == mozilla::StyleDisplayInside::RubyBaseContainer ||
            inside == mozilla::StyleDisplayInside::RubyText ||

diff --git a/servo/components/style/style_adjuster.rs b/servo/components/style/style_adjuster.rs
@@ -12,8 +12,6 @@ use crate::properties::longhands::float::computed_value::T as Float;
 use crate::properties::longhands::overflow_x::computed_value::T as Overflow;
 use crate::properties::longhands::position::computed_value::T as Position;
 use crate::properties::{self, ComputedValues, StyleBuilder};
-#[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-use crate::values::specified::box_::DisplayInside;
 use app_units::Au;
 
 /// A struct that implements all the adjustment methods.
@@ -207,7 +205,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.style.pseudo.map_or(false, |p| p.is_marker()) &&
                 self.style.get_parent_list().clone_list_style_position() ==
                     ListStylePosition::Outside &&
-                layout_parent_style.get_box().clone_display().inside() != DisplayInside::Inline
+                !layout_parent_style.get_box().clone_display().is_inline_flow()
         );
 
         if !blockify {

diff --git a/servo/components/style/values/specified/box.rs b/servo/components/style/values/specified/box.rs
@@ -61,12 +61,9 @@ pub enum DisplayInside {
     None = 0,
     #[cfg(any(feature = "servo-layout-2020", feature = "gecko"))]
     Contents,
-    #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-    Block,
+    Flow,
     FlowRoot,
     #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-    Inline,
-    #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
     Flex,
     #[cfg(feature = "gecko")]
     Grid,
@@ -114,7 +111,6 @@ pub enum DisplayInside {
     MozGroupbox,
     #[cfg(feature = "gecko")]
     MozPopup,
-    Flow, // only used for parsing, not computed value
 }
 
 #[allow(missing_docs)]
@@ -147,14 +143,8 @@ impl Display {
     pub const None: Self = Self::new(DisplayOutside::None, DisplayInside::None);
     #[cfg(any(feature = "servo-layout-2020", feature = "gecko"))]
     pub const Contents: Self = Self::new(DisplayOutside::None, DisplayInside::Contents);
-    #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-    pub const Inline: Self = Self::new(DisplayOutside::Inline, DisplayInside::Inline);
-    #[cfg(any(feature = "servo-layout-2020"))]
     pub const Inline: Self = Self::new(DisplayOutside::Inline, DisplayInside::Flow);
     pub const InlineBlock: Self = Self::new(DisplayOutside::Inline, DisplayInside::FlowRoot);
-    #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-    pub const Block: Self = Self::new(DisplayOutside::Block, DisplayInside::Block);
-    #[cfg(any(feature = "servo-layout-2020"))]
     pub const Block: Self = Self::new(DisplayOutside::Block, DisplayInside::Flow);
     #[cfg(feature = "gecko")]
     pub const FlowRoot: Self = Self::new(DisplayOutside::Block, DisplayInside::FlowRoot);
@@ -171,7 +161,7 @@ impl Display {
     #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
     pub const InlineTable: Self = Self::new(DisplayOutside::Inline, DisplayInside::Table);
     #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-    pub const TableCaption: Self = Self::new(DisplayOutside::TableCaption, DisplayInside::Block);
+    pub const TableCaption: Self = Self::new(DisplayOutside::TableCaption, DisplayInside::Flow);
     #[cfg(feature = "gecko")]
     pub const Ruby: Self = Self::new(DisplayOutside::Inline, DisplayInside::Ruby);
     #[cfg(feature = "gecko")]
@@ -256,18 +246,8 @@ impl Display {
     }
 
     /// Make a display enum value from <display-outside> and <display-inside> values.
-    /// We store `flow` as a synthetic `block` or `inline` inside-value to simplify
-    /// our layout code.
     #[inline]
     fn from3(outside: DisplayOutside, inside: DisplayInside, list_item: bool) -> Self {
-        let inside = match inside {
-            #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-            DisplayInside::Flow => match outside {
-                DisplayOutside::Inline => DisplayInside::Inline,
-                _ => DisplayInside::Block,
-            },
-            _ => inside,
-        };
         let v = Self::new(outside, inside);
         if !list_item {
             return v;
@@ -290,6 +270,13 @@ impl Display {
         .unwrap()
     }
 
+    /// Whether this is `display: inline` (or `inline list-item`).
+    #[inline]
+    pub fn is_inline_flow(&self) -> bool {
+        self.outside() == DisplayOutside::Inline &&
+            self.inside() == DisplayInside::Flow
+    }
+
     /// Returns whether this `display` value is some kind of list-item.
     #[inline]
     pub const fn is_list_item(&self) -> bool {
@@ -381,9 +368,8 @@ impl Display {
         match self.outside() {
             DisplayOutside::Inline => {
                 let inside = match self.inside() {
-                    #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-                    DisplayInside::Inline | DisplayInside::FlowRoot => DisplayInside::Block,
-                    #[cfg(feature = "servo-layout-2020")]
+                    // `inline-block` blockifies to `block` rather than
+                    // `flow-root`, for legacy reasons.
                     DisplayInside::FlowRoot => DisplayInside::Flow,
                     inside => inside,
                 };
@@ -407,7 +393,9 @@ impl Display {
         match self.outside() {
             DisplayOutside::Block => {
                 let inside = match self.inside() {
-                    DisplayInside::Block => DisplayInside::FlowRoot,
+                    // `display: block` inlinifies to `display: inline-block`,
+                    // rather than `inline`, for legacy reasons.
+                    DisplayInside::Flow => DisplayInside::FlowRoot,
                     inside => inside,
                 };
                 Display::from3(DisplayOutside::Inline, inside, self.is_list_item())
@@ -443,18 +431,8 @@ impl ToCss for Display {
     where
         W: fmt::Write,
     {
-        #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-        debug_assert_ne!(
-            self.inside(),
-            DisplayInside::Flow,
-            "`flow` fears in `display` computed value"
-        );
         let outside = self.outside();
-        let inside = match self.inside() {
-            #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))]
-            DisplayInside::Block | DisplayInside::Inline => DisplayInside::Flow,
-            inside => inside,
-        };
+        let inside = self.inside();
         match *self {
             Display::Block | Display::Inline => outside.to_css(dest),
             Display::InlineBlock => dest.write_str("inline-block"),