Auto merge of #18893 - emilio:bye-can-be-fragmented, r=SimonSapin

style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18893)
<!-- Reviewable:end -->

components/layout/construct.rs

@@ -1353,13 +1353,14 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
             return false
         }
 
-        if node.can_be_fragmented() || node.style(self.style_context()).is_multicol() {
-            return false
-        }
-
         let mut set_has_newly_constructed_flow_flag = false;
         let result = {
             let style = node.style(self.style_context());
+
+            if style.can_be_fragmented() || style.is_multicol() {
+                return false
+            }
+
             let damage = node.restyle_damage();
             let mut data = node.mutate_layout_data().unwrap();
 
@@ -1657,16 +1658,9 @@ impl<ConcreteThreadSafeLayoutNode> NodeUtils for ConcreteThreadSafeLayoutNode
     }
 
     #[inline(always)]
-    fn set_flow_construction_result(self, mut result: ConstructionResult) {
+    fn set_flow_construction_result(self, result: ConstructionResult) {
-        if self.can_be_fragmented() {
-            if let ConstructionResult::Flow(ref mut flow, _) = result {
-                FlowRef::deref_mut(flow).mut_base().flags.insert(FlowFlags::CAN_BE_FRAGMENTED);
-            }
-        }
-
         let mut layout_data = self.mutate_layout_data().unwrap();
         let dst = self.construction_result_mut(&mut *layout_data);
-
         *dst = result;
     }
 

components/layout/flow.rs

@@ -992,6 +992,10 @@ impl BaseFlow {
         let mut flags = FlowFlags::empty();
         match style {
             Some(style) => {
+                if style.can_be_fragmented() {
+                    flags.insert(FlowFlags::CAN_BE_FRAGMENTED);
+                }
+
                 match style.get_box().position {
                     Position::Absolute | Position::Fixed => {
                         flags.insert(FlowFlags::IS_ABSOLUTELY_POSITIONED);

components/layout/traversal.rs

@@ -280,7 +280,8 @@ impl<'a> PostorderFlowTraversal for AssignBSizes<'a> {
     fn should_process(&self, flow: &mut Flow) -> bool {
         let base = flow.base();
         base.restyle_damage.intersects(ServoRestyleDamage::REFLOW_OUT_OF_FLOW | ServoRestyleDamage::REFLOW) &&
-        // The fragmentation countainer is responsible for calling Flow::fragment recursively
+        // The fragmentation countainer is responsible for calling
+        // Flow::fragment recursively
         !base.flags.contains(FlowFlags::CAN_BE_FRAGMENTED)
     }
 }

components/layout_thread/dom_wrapper.rs

@@ -209,17 +209,9 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
         self.node.downcast().map(ServoLayoutDocument::from_layout_js)
     }
 
-    fn can_be_fragmented(&self) -> bool {
-        unsafe { self.node.get_flag(NodeFlags::CAN_BE_FRAGMENTED) }
-    }
-
     fn is_in_document(&self) -> bool {
         unsafe { self.node.get_flag(NodeFlags::IS_IN_DOC) }
     }
-
-    unsafe fn set_can_be_fragmented(&self, value: bool) {
-        self.node.set_flag(NodeFlags::CAN_BE_FRAGMENTED, value)
-    }
 }
 
 impl<'ln> LayoutNode for ServoLayoutNode<'ln> {
@@ -930,10 +922,6 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
         self.node
     }
 
-    fn can_be_fragmented(&self) -> bool {
-        self.node.can_be_fragmented()
-    }
-
     fn node_text_content(&self) -> String {
         let this = unsafe { self.get_jsmanaged() };
         return this.text_content();

components/script/dom/node.rs

@@ -164,10 +164,7 @@ bitflags! {
                  to be reachable with using sequential focus navigation."]
         const SEQUENTIALLY_FOCUSABLE = 1 << 3;
 
-        /// Whether any ancestor is a fragmentation container
+        // There are two free bits here.
-        const CAN_BE_FRAGMENTED = 1 << 4;
-
-        // There's a free bit here.
 
         #[doc = "Specifies whether the parser has set an associated form owner for \
                  this element. Only applicable for form-associatable elements."]

components/script_layout_interface/wrapper_traits.rs

@@ -240,8 +240,6 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
     /// data flags, and we have this annoying trait separation between script and layout :-(
     unsafe fn unsafe_get(self) -> Self::ConcreteNode;
 
-    fn can_be_fragmented(&self) -> bool;
-
     fn node_text_content(&self) -> String;
 
     /// If the insertion point is within this node, returns it. Otherwise, returns `None`.

components/style/dom.rs

@@ -239,13 +239,6 @@ pub trait TNode : Sized + Copy + Clone + Debug + NodeInfo + PartialEq {
 
     /// Get this node as a document, if it's one.
     fn as_document(&self) -> Option<Self::ConcreteDocument>;
-
-    /// Whether this node can be fragmented. This is used for multicol, and only
-    /// for Servo.
-    fn can_be_fragmented(&self) -> bool;
-
-    /// Set whether this node can be fragmented.
-    unsafe fn set_can_be_fragmented(&self, value: bool);
 }
 
 /// Wrapper to output the subtree rather than the single node when formatting

components/style/gecko/wrapper.rs

@@ -345,18 +345,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
             None
         }
     }
-
-    #[inline]
-    fn can_be_fragmented(&self) -> bool {
-        // FIXME(SimonSapin): Servo uses this to implement CSS multicol / fragmentation
-        // Maybe this isn’t useful for Gecko?
-        false
-    }
-
-    unsafe fn set_can_be_fragmented(&self, _value: bool) {
-        // FIXME(SimonSapin): Servo uses this to implement CSS multicol / fragmentation
-        // Maybe this isn’t useful for Gecko?
-    }
 }
 
 /// A wrapper on top of two kind of iterators, depending on the parent being

components/style/matching.rs

@@ -413,6 +413,15 @@ trait PrivateMatchMethods: TElement {
             }
         }
 
+        #[cfg(feature = "servo")]
+        {
+            // We may need to set or propagate the CAN_BE_FRAGMENTED bit
+            // on our children.
+            if old_values.is_multicol() != new_values.is_multicol() {
+                return ChildCascadeRequirement::MustCascadeChildren;
+            }
+        }
+
         // We could prove that, if our children don't inherit reset
         // properties, we can stop the cascade.
         ChildCascadeRequirement::MustCascadeChildrenIfInheritResetStyle
@@ -504,7 +513,6 @@ pub trait MatchMethods : TElement {
         mut new_styles: ResolvedElementStyles,
         important_rules_changed: bool,
     ) -> ChildCascadeRequirement {
-        use dom::TNode;
         use std::cmp;
 
         self.process_animations(
@@ -518,24 +526,6 @@ pub trait MatchMethods : TElement {
         // First of all, update the styles.
         let old_styles = data.set_styles(new_styles);
 
-        // Propagate the "can be fragmented" bit. It would be nice to
-        // encapsulate this better.
-        if cfg!(feature = "servo") {
-            let layout_parent =
-                self.inheritance_parent().map(|e| e.layout_parent());
-            let layout_parent_data =
-                layout_parent.as_ref().and_then(|e| e.borrow_data());
-            let layout_parent_style =
-                layout_parent_data.as_ref().map(|d| d.styles.primary());
-
-            if let Some(ref p) = layout_parent_style {
-                let can_be_fragmented =
-                    p.is_multicol() ||
-                    layout_parent.as_ref().unwrap().as_node().can_be_fragmented();
-                unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); }
-            }
-        }
-
         let new_primary_style = data.styles.primary.as_ref().unwrap();
 
         let mut cascade_requirement = ChildCascadeRequirement::CanSkipCascade;

components/style/properties/computed_value_flags.rs

@@ -62,6 +62,11 @@ bitflags! {
 
         /// A flag to mark a style which is a visited style.
         const IS_STYLE_IF_VISITED = 1 << 9;
+
+        /// Whether the style or any of the ancestors has a multicol style.
+        ///
+        /// Only used in Servo.
+        const CAN_BE_FRAGMENTED = 1 << 10;
     }
 }
 

components/style/properties/gecko.mako.rs

@@ -300,10 +300,6 @@ impl ComputedValuesInner {
         !self.get_box().gecko.mBinding.mRawPtr.is_null()
     }
 
-    // FIXME(bholley): Implement this properly.
-    #[inline]
-    pub fn is_multicol(&self) -> bool { false }
-
     pub fn to_declaration_block(&self, property: PropertyDeclarationId) -> PropertyDeclarationBlock {
         let value = match property {
             % for prop in data.longhands:

components/style/properties/properties.mako.rs

@@ -2058,6 +2058,18 @@ pub mod style_structs {
                     .take(self.transition_property_count())
                     .any(|t| t.seconds() > 0.)
             }
+        % elif style_struct.name == "Column":
+            /// Whether this is a multicol style.
+            #[cfg(feature = "servo")]
+            pub fn is_multicol(&self) -> bool {
+                match self.column_width {
+                    Either::First(_width) => true,
+                    Either::Second(_auto) => match self.column_count {
+                        Either::First(_n) => true,
+                        Either::Second(_auto) => false,
+                    }
+                }
+            }
         % endif
     }
 
@@ -2276,17 +2288,16 @@ impl ComputedValuesInner {
         }
     }
 
+    /// Whether the current style or any of its ancestors is multicolumn.
+    #[inline]
+    pub fn can_be_fragmented(&self) -> bool {
+        self.flags.contains(ComputedValueFlags::CAN_BE_FRAGMENTED)
+    }
+
     /// Whether the current style is multicolumn.
     #[inline]
     pub fn is_multicol(&self) -> bool {
-        let style = self.get_column();
+        self.get_column().is_multicol()
-        match style.column_width {
-            Either::First(_width) => true,
-            Either::Second(_auto) => match style.column_count {
-                Either::First(_n) => true,
-                Either::Second(_auto) => false,
-            }
-        }
     }
 
     /// Resolves the currentColor keyword.
@@ -2761,16 +2772,16 @@ impl<'a> StyleBuilder<'a> {
         % endif
 
         % if not property.style_struct.inherited:
-        self.flags.insert(::properties::computed_value_flags::ComputedValueFlags::INHERITS_RESET_STYLE);
+        self.flags.insert(ComputedValueFlags::INHERITS_RESET_STYLE);
         self.modified_reset = true;
         % endif
 
         % if property.ident == "content":
-        self.flags.insert(::properties::computed_value_flags::ComputedValueFlags::INHERITS_CONTENT);
+        self.flags.insert(ComputedValueFlags::INHERITS_CONTENT);
         % endif
 
         % if property.ident == "display":
-        self.flags.insert(::properties::computed_value_flags::ComputedValueFlags::INHERITS_DISPLAY);
+        self.flags.insert(ComputedValueFlags::INHERITS_DISPLAY);
         % endif
 
         self.${property.style_struct.ident}.mutate()

components/style/style_adjuster.rs

@@ -104,6 +104,15 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.style.is_pseudo_element() {
             self.style.flags.insert(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE);
         }
+
+        #[cfg(feature = "servo")]
+        {
+            if self.style.inherited_flags().contains(ComputedValueFlags::CAN_BE_FRAGMENTED) ||
+                self.style.get_parent_column().is_multicol()
+            {
+                self.style.flags.insert(ComputedValueFlags::CAN_BE_FRAGMENTED);
+            }
+        }
     }
 
     /// Adjust the style for text style.

tests/wpt/metadata/css/css-flexbox/flexbox_columns.html.ini

@@ -0,0 +1,2 @@
+[flexbox_columns.html]
+  expected: FAIL