servo: Merge #14130 - layout: Mark flex items properly during construction (from stshine:construct-flexbox); r=pcwalton

<!-- Please describe your changes on the following line: -->

Set the flag of the fragment of children in a flex container according
to the direction of the container. The mark is done on the fragment
because flex item enstablish a stacking context when its z-index is
non-zero ,despite its `position' property.

Part of #14123.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because refactoring

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

r? @pcwalton

Source-Repo: https://github.com/servo/servo
Source-Revision: 333c397f03bc2d143e0d8c112138d5758d37f6f5

servo/components/layout/block.rs

@@ -41,6 +41,7 @@ use flow::{ImmutableFlowUtils, LateAbsolutePositionInfo, MutableFlowUtils, Opaqu
 use flow::IS_ABSOLUTELY_POSITIONED;
 use flow_list::FlowList;
 use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, Overflow};
+use fragment::{IS_INLINE_FLEX_ITEM, IS_BLOCK_FLEX_ITEM};
 use fragment::SpecificFragmentInfo;
 use gfx::display_list::{ClippingRegion, StackingContext};
 use gfx_traits::ScrollRootId;
@@ -484,7 +485,7 @@ pub enum BlockType {
     FloatNonReplaced,
     InlineBlockReplaced,
     InlineBlockNonReplaced,
-    FlexItem,
+    InlineFlexItem,
 }
 
 #[derive(Clone, PartialEq)]
@@ -520,8 +521,6 @@ bitflags! {
     flags BlockFlowFlags: u8 {
         #[doc = "If this is set, then this block flow is the root flow."]
         const IS_ROOT = 0b0000_0001,
-        #[doc = "Whether this block flow is a child of a flex container."]
-        const IS_FLEX = 0b0001_0000,
     }
 }
 
@@ -561,8 +560,8 @@ impl BlockFlow {
             } else {
                 BlockType::AbsoluteNonReplaced
             }
-        } else if self.is_flex() {
-            BlockType::FlexItem
+        } else if self.is_inline_flex_item() {
+            BlockType::InlineFlexItem
         } else if self.base.flags.is_float() {
             if self.is_replaced_content() {
                 BlockType::FloatReplaced
@@ -638,8 +637,8 @@ impl BlockFlow {
                                                               shared_context,
                                                               containing_block_inline_size);
             }
-            BlockType::FlexItem => {
-                let inline_size_computer = FlexItem;
+            BlockType::InlineFlexItem => {
+                let inline_size_computer = InlineFlexItem;
                 inline_size_computer.compute_used_inline_size(self,
                                                               shared_context,
                                                               containing_block_inline_size);
@@ -1506,7 +1505,7 @@ impl BlockFlow {
         }
 
         // If you remove the might_have_floats_in conditional, this will go off.
-        debug_assert!(!self.is_flex());
+        debug_assert!(!self.is_inline_flex_item());
 
         // Compute the available space for us, based on the actual floats.
         let rect = self.base.floats.available_rect(Au(0),
@@ -1778,12 +1777,12 @@ impl BlockFlow {
         padding.block_start.is_definitely_zero() && padding.block_end.is_definitely_zero()
     }
 
-    pub fn mark_as_flex(&mut self) {
-        self.flags.insert(IS_FLEX)
+    pub fn is_inline_flex_item(&self) -> bool {
+        self.fragment.flags.contains(IS_INLINE_FLEX_ITEM)
     }
 
-    pub fn is_flex(&self) -> bool {
-        self.flags.contains(IS_FLEX)
+    pub fn is_block_flex_item(&self) -> bool {
+        self.fragment.flags.contains(IS_BLOCK_FLEX_ITEM)
     }
 }
 
@@ -2589,7 +2588,7 @@ pub struct FloatNonReplaced;
 pub struct FloatReplaced;
 pub struct InlineBlockNonReplaced;
 pub struct InlineBlockReplaced;
-pub struct FlexItem;
+pub struct InlineFlexItem;
 
 impl ISizeAndMarginsComputer for AbsoluteNonReplaced {
     /// Solve the horizontal constraint equation for absolute non-replaced elements.
@@ -3080,7 +3079,7 @@ impl ISizeAndMarginsComputer for InlineBlockReplaced {
     }
 }
 
-impl ISizeAndMarginsComputer for FlexItem {
+impl ISizeAndMarginsComputer for InlineFlexItem {
     // Replace the default method directly to prevent recalculating and setting margins again
     // which has already been set by its parent.
     fn compute_used_inline_size(&self,

servo/components/layout/construct.rs

@@ -25,6 +25,7 @@ use flow::{MutableFlowUtils, MutableOwnedFlowUtils};
 use flow_ref::FlowRef;
 use fragment::{CanvasFragmentInfo, ImageFragmentInfo, InlineAbsoluteFragmentInfo, SvgFragmentInfo};
 use fragment::{Fragment, GeneratedContentInfo, IframeFragmentInfo};
+use fragment::{IS_INLINE_FLEX_ITEM, IS_BLOCK_FLEX_ITEM};
 use fragment::{InlineAbsoluteHypotheticalFragmentInfo, TableColumnFragmentInfo};
 use fragment::{InlineBlockFragmentInfo, SpecificFragmentInfo, UnscannedTextFragmentInfo};
 use fragment::WhitespaceStrippingResult;
@@ -33,6 +34,7 @@ use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFlow, InlineFragmentNodeFlags};
 use inline::{InlineFragmentNodeInfo, LAST_FRAGMENT_OF_ELEMENT};
 use linked_list::prepend_from;
 use list_item::{ListItemFlow, ListStyleTypeContent};
+use model::Direction;
 use multicol::{MulticolColumnFlow, MulticolFlow};
 use parallel;
 use script_layout_interface::{LayoutElementType, LayoutNodeType, is_image_data};
@@ -1928,9 +1930,16 @@ impl Legalizer {
                                                      &[PseudoElement::ServoAnonymousBlock],
                                                      SpecificFragmentInfo::Generic,
                                                      BlockFlow::from_fragment);
-                flow::mut_base(FlowRef::deref_mut(&mut
-                                                   block_wrapper)).flags
-                                                                  .insert(MARGINS_CANNOT_COLLAPSE);
+                {
+                    let flag = if parent.as_flex().main_mode() == Direction::Inline {
+                        IS_INLINE_FLEX_ITEM
+                    } else {
+                        IS_BLOCK_FLEX_ITEM
+                    };
+                    let mut block = FlowRef::deref_mut(&mut block_wrapper).as_mut_block();
+                    block.base.flags.insert(MARGINS_CANNOT_COLLAPSE);
+                    block.fragment.flags.insert(flag);
+                }
                 block_wrapper.add_new_child((*child).clone());
                 block_wrapper.finish();
                 parent.add_new_child(block_wrapper);
@@ -1938,7 +1947,16 @@ impl Legalizer {
             }
 
             (FlowClass::Flex, _) => {
-                flow::mut_base(FlowRef::deref_mut(child)).flags.insert(MARGINS_CANNOT_COLLAPSE);
+                {
+                    let flag = if parent.as_flex().main_mode() == Direction::Inline {
+                        IS_INLINE_FLEX_ITEM
+                    } else {
+                        IS_BLOCK_FLEX_ITEM
+                    };
+                    let mut block = FlowRef::deref_mut(child).as_mut_block();
+                    block.base.flags.insert(MARGINS_CANNOT_COLLAPSE);
+                    block.fragment.flags.insert(flag);
+                }
                 parent.add_new_child((*child).clone());
                 true
             }

servo/components/layout/flex.rs

@@ -395,6 +395,10 @@ impl FlexFlow {
         }
     }
 
+    pub fn main_mode(&self) -> Direction {
+        self.main_mode
+    }
+
     /// Returns a line start after the last item that is already in a line.
     /// Note that when the container main size is infinite(i.e. A column flexbox with auto height),
     /// we do not need to do flex resolving and this can be considered as a fast-path, so the
@@ -613,8 +617,6 @@ impl FlexFlow {
                 //
                 // TODO(#2265, pcwalton): Do this in the cascade instead.
                 block.base.flags.set_text_align(containing_block_text_align);
-                // FIXME(stshine): should this be done during construction?
-                block.mark_as_flex();
 
                 let margin = block.fragment.style().logical_margin();
                 let auto_len =
@@ -810,6 +812,14 @@ impl Flow for FlexFlow {
         FlowClass::Flex
     }
 
+    fn as_mut_flex(&mut self) -> &mut FlexFlow {
+        self
+    }
+
+    fn as_flex(&self) -> &FlexFlow {
+        self
+    }
+
     fn as_block(&self) -> &BlockFlow {
         &self.block_flow
     }

servo/components/layout/flow.rs

@@ -30,6 +30,7 @@ use block::{BlockFlow, FormattingContextType};
 use context::{LayoutContext, SharedLayoutContext};
 use display_list_builder::DisplayListBuildState;
 use euclid::{Point2D, Size2D};
+use flex::FlexFlow;
 use floats::{Floats, SpeculatedFloatPlacement};
 use flow_list::{FlowList, MutFlowListIterator};
 use flow_ref::{FlowRef, WeakFlowRef};
@@ -86,6 +87,16 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static {
         panic!("called as_mut_block() on a non-block flow")
     }
 
+    /// If this is a flex flow, returns the underlying object. Fails otherwise.
+    fn as_flex(&self) -> &FlexFlow {
+        panic!("called as_flex() on a non-flex flow")
+    }
+
+    /// If this is a flex flow, returns the underlying object, borrowed mutably. Fails otherwise.
+    fn as_mut_flex(&mut self) -> &mut FlexFlow {
+        panic!("called as_mut_flex() on a non-flex flow")
+    }
+
     /// If this is an inline flow, returns the underlying object. Fails otherwise.
     fn as_inline(&self) -> &InlineFlow {
         panic!("called as_inline() on a non-inline flow")

servo/components/layout/fragment.rs

@@ -123,6 +123,9 @@ pub struct Fragment {
     /// The pseudo-element that this fragment represents.
     pub pseudo: PseudoElementType<()>,
 
+    /// Various flags for this fragment.
+    pub flags: FragmentFlags,
+
     /// A debug ID that is consistent for the life of this fragment (via transform etc).
     /// This ID should not be considered stable across multiple layouts or fragment
     /// manipulations.
@@ -917,6 +920,7 @@ impl Fragment {
             specific: specific,
             inline_context: None,
             pseudo: node.get_pseudo_element_type().strip(),
+            flags: FragmentFlags::empty(),
             debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::new(0),
         }
@@ -945,6 +949,7 @@ impl Fragment {
             specific: specific,
             inline_context: None,
             pseudo: pseudo,
+            flags: FragmentFlags::empty(),
             debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::new(0),
         }
@@ -969,6 +974,7 @@ impl Fragment {
             specific: specific,
             inline_context: None,
             pseudo: self.pseudo,
+            flags: FragmentFlags::empty(),
             debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::new(0),
         }
@@ -996,6 +1002,7 @@ impl Fragment {
             specific: info,
             inline_context: self.inline_context.clone(),
             pseudo: self.pseudo.clone(),
+            flags: FragmentFlags::empty(),
             debug_id: self.debug_id.clone(),
             stacking_context_id: StackingContextId::new(0),
         }
@@ -3111,6 +3118,16 @@ impl Overflow {
     }
 }
 
+bitflags! {
+    pub flags FragmentFlags: u8 {
+        // TODO(stshine): find a better name since these flags can also be used for grid item.
+        /// Whether this fragment represents a child in a row flex container.
+        const IS_INLINE_FLEX_ITEM = 0b0000_0001,
+        /// Whether this fragment represents a child in a column flex container.
+        const IS_BLOCK_FLEX_ITEM = 0b0000_0010,
+    }
+}
+
 /// Specified distances from the margin edge of a block to its content in the inline direction.
 /// These are returned by `guess_inline_content_edge_offsets()` and are used in the float placement
 /// speculation logic.

servo/components/layout/model.rs

@@ -505,7 +505,7 @@ impl ToGfxMatrix for ComputedMatrix {
 }
 
 // Used to specify the logical direction.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq)]
 pub enum Direction {
     Inline,
     Block