servo: Merge #18893 - style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented (from emilio:bye-can-be-fragmented); r=SimonSapin
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 05 Jan 2018 05:11:00 -0600
changeset 449723 b1b7f193fae797aef0aa4a854ac963e245319904
parent 449722 e89a072ca221a8f44a38a65ebc6298191f5aeede
child 449724 63d8a697c1513c4ef23ee7a23b8f27bb2c9fd826
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersSimonSapin
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18893 - style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented (from emilio:bye-can-be-fragmented); r=SimonSapin Replace them instead by a computed value flag, the same way as the IS_IN_DISPLAY_NONE_SUBTREE flag works. Source-Repo: https://github.com/servo/servo Source-Revision: 83a8891bd4d04ccb3f2f7b292d53f2847380b94c
servo/components/layout/construct.rs
servo/components/layout/flow.rs
servo/components/layout/traversal.rs
servo/components/layout_thread/dom_wrapper.rs
servo/components/script/dom/node.rs
servo/components/script_layout_interface/wrapper_traits.rs
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/matching.rs
servo/components/style/properties/computed_value_flags.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/style_adjuster.rs
--- a/servo/components/layout/construct.rs
+++ b/servo/components/layout/construct.rs
@@ -1348,23 +1348,24 @@ impl<'a, ConcreteThreadSafeLayoutNode: T
         if need_to_reconstruct {
             return false
         }
 
         if node.restyle_damage().contains(ServoRestyleDamage::RECONSTRUCT_FLOW) {
             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();
 
             match *node.construction_result_mut(&mut *data) {
                 ConstructionResult::None => true,
                 ConstructionResult::Flow(ref mut flow, _) => {
                     // The node's flow is of the same type and has the same set of children and can
                     // therefore be repaired by simply propagating damage and style to the flow.
@@ -1652,26 +1653,19 @@ impl<ConcreteThreadSafeLayoutNode> NodeU
             PseudoElementType::After(_) => &mut data.after_flow_construction_result,
             PseudoElementType::DetailsSummary(_) => &mut data.details_summary_flow_construction_result,
             PseudoElementType::DetailsContent(_) => &mut data.details_content_flow_construction_result,
             PseudoElementType::Normal    => &mut data.flow_construction_result,
         }
     }
 
     #[inline(always)]
-    fn set_flow_construction_result(self, mut 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);
-            }
-        }
-
+    fn set_flow_construction_result(self, result: ConstructionResult) {
         let mut layout_data = self.mutate_layout_data().unwrap();
         let dst = self.construction_result_mut(&mut *layout_data);
-
         *dst = result;
     }
 
     #[inline(always)]
     fn get_construction_result(self) -> ConstructionResult {
         let mut layout_data = self.mutate_layout_data().unwrap();
         self.construction_result_mut(&mut *layout_data).get()
     }
--- a/servo/components/layout/flow.rs
+++ b/servo/components/layout/flow.rs
@@ -987,16 +987,20 @@ impl BaseFlow {
     #[inline]
     pub fn new(style: Option<&ComputedValues>,
                writing_mode: WritingMode,
                force_nonfloated: ForceNonfloatedFlag)
                -> 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);
 
                         let logical_position = style.logical_position();
                         if logical_position.inline_start == LengthOrPercentageOrAuto::Auto &&
                                 logical_position.inline_end == LengthOrPercentageOrAuto::Auto {
                             flags.insert(FlowFlags::INLINE_POSITION_IS_STATIC);
--- a/servo/components/layout/traversal.rs
+++ b/servo/components/layout/traversal.rs
@@ -275,17 +275,18 @@ impl<'a> PostorderFlowTraversal for Assi
 
         flow.assign_block_size(self.layout_context);
     }
 
     #[inline]
     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)
     }
 }
 
 pub struct ComputeStackingRelativePositions<'a> {
     pub layout_context: &'a LayoutContext<'a>,
 }
 
--- a/servo/components/layout_thread/dom_wrapper.rs
+++ b/servo/components/layout_thread/dom_wrapper.rs
@@ -204,27 +204,19 @@ impl<'ln> TNode for ServoLayoutNode<'ln>
     fn as_element(&self) -> Option<ServoLayoutElement<'ln>> {
         as_element(self.node)
     }
 
     fn as_document(&self) -> Option<ServoLayoutDocument<'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> {
     type ConcreteThreadSafeLayoutNode = ServoThreadSafeLayoutNode<'ln>;
 
     fn to_threadsafe(&self) -> Self::ConcreteThreadSafeLayoutNode {
         ServoThreadSafeLayoutNode::new(self)
     }
@@ -925,20 +917,16 @@ impl<'ln> ThreadSafeLayoutNode for Servo
             !self.style(context).get_inheritedtext().white_space.preserve_newlines()
         }
     }
 
     unsafe fn unsafe_get(self) -> Self::ConcreteNode {
         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();
     }
 
     fn selection(&self) -> Option<Range<ByteIndex>> {
         let this = unsafe { self.get_jsmanaged() };
 
--- a/servo/components/script/dom/node.rs
+++ b/servo/components/script/dom/node.rs
@@ -159,20 +159,17 @@ bitflags! {
         // Perhaps using a Set in Document?
         #[doc = "Specifies whether or not there is an authentic click in progress on \
                  this element."]
         const CLICK_IN_PROGRESS = 1 << 2;
         #[doc = "Specifies whether this node is focusable and whether it is supposed \
                  to be reachable with using sequential focus navigation."]
         const SEQUENTIALLY_FOCUSABLE = 1 << 3;
 
-        /// Whether any ancestor is a fragmentation container
-        const CAN_BE_FRAGMENTED = 1 << 4;
-
-        // There's a free bit here.
+        // There are two free bits here.
 
         #[doc = "Specifies whether the parser has set an associated form owner for \
                  this element. Only applicable for form-associatable elements."]
         const PARSER_ASSOCIATED_FORM_OWNER = 1 << 6;
 
         /// Whether this element has a snapshot stored due to a style or
         /// attribute change.
         ///
--- a/servo/components/script_layout_interface/wrapper_traits.rs
+++ b/servo/components/script_layout_interface/wrapper_traits.rs
@@ -235,18 +235,16 @@ pub trait ThreadSafeLayoutNode: Clone + 
     /// Returns access to the underlying LayoutNode. This is breaks the abstraction
     /// barrier of ThreadSafeLayout wrapper layer, and can lead to races if not used
     /// carefully.
     ///
     /// We need this because the implementation of some methods need to access the layout
     /// 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`.
     fn selection(&self) -> Option<Range<ByteIndex>>;
 
     /// If this is an image element, returns its URL. If this is not an image element, fails.
     fn image_url(&self) -> Option<ServoUrl>;
 
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -234,23 +234,16 @@ pub trait TNode : Sized + Copy + Clone +
     /// A debug id, only useful, mm... for debugging.
     fn debug_id(self) -> usize;
 
     /// Get this node as an element, if it's one.
     fn as_element(&self) -> Option<Self::ConcreteElement>;
 
     /// 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
 /// for Debug.
 pub struct ShowSubtree<N: TNode>(pub N);
 impl<N: TNode> Debug for ShowSubtree<N> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         writeln!(f, "DOM Subtree:")?;
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -340,28 +340,16 @@ impl<'ln> TNode for GeckoNode<'ln> {
     #[inline]
     fn as_document(&self) -> Option<Self::ConcreteDocument> {
         if self.is_document() {
             Some(self.owner_doc())
         } else {
             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
 /// iterated.
 ///
 /// We generally iterate children by traversing the light-tree siblings of the
 /// first child like Servo does.
 ///
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -408,16 +408,25 @@ trait PrivateMatchMethods: TElement {
             }
 
             if was_legacy_justify_items &&
                 old_justify_items.computed != new_justify_items.computed {
                 return ChildCascadeRequirement::MustCascadeChildren;
             }
         }
 
+        #[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
     }
 
     #[cfg(feature = "servo")]
     fn update_animations_for_cascade(&self,
                                      context: &SharedStyleContext,
@@ -499,48 +508,29 @@ pub trait MatchMethods : TElement {
     /// damage.
     fn finish_restyle(
         &self,
         context: &mut StyleContext<Self>,
         data: &mut ElementData,
         mut new_styles: ResolvedElementStyles,
         important_rules_changed: bool,
     ) -> ChildCascadeRequirement {
-        use dom::TNode;
         use std::cmp;
 
         self.process_animations(
             context,
             &mut data.styles.primary,
             &mut new_styles.primary.style.0,
             data.hint,
             important_rules_changed,
         );
 
         // 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;
         if self.is_root() && !self.is_native_anonymous() {
             let device = context.shared.stylist.device();
             let new_font_size = new_primary_style.get_font().clone_font_size();
 
             if old_styles.primary.as_ref().map_or(true, |s| s.get_font().clone_font_size() != new_font_size) {
--- a/servo/components/style/properties/computed_value_flags.rs
+++ b/servo/components/style/properties/computed_value_flags.rs
@@ -57,16 +57,21 @@ bitflags! {
         /// Important because of the same reason.
         const INHERITS_CONTENT = 1 << 7;
 
         /// Whether the child explicitly inherits any reset property.
         const INHERITS_RESET_STYLE = 1 << 8;
 
         /// 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;
     }
 }
 
 impl ComputedValueFlags {
     /// Returns the flags that are inherited.
     #[inline]
     pub fn inherited(self) -> Self {
         self & !(ComputedValueFlags::INHERITS_DISPLAY |
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -295,20 +295,16 @@ impl ComputedValuesInner {
         &self.visited_style
     }
 
     #[allow(non_snake_case)]
     pub fn has_moz_binding(&self) -> bool {
         !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:
                 % if prop.animatable:
                     PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}) => {
                         PropertyDeclaration::${prop.camel_case}(
                             % if prop.boxed:
                                 Box::new(
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2053,16 +2053,28 @@ pub mod style_structs {
 
             /// Returns whether there are any transitions specified.
             #[cfg(feature = "servo")]
             pub fn specifies_transitions(&self) -> bool {
                 self.transition_duration_iter()
                     .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
     }
 
     % for longhand in style_struct.longhands:
         % if longhand.need_index:
             /// An iterator over the values of the ${longhand.name} properties.
             pub struct ${longhand.camel_case}Iter<'a> {
                 style_struct: &'a style_structs::${style_struct.name},
@@ -2271,27 +2283,26 @@ impl ComputedValuesInner {
     pub fn ineffective_content_property(&self) -> bool {
         use properties::longhands::content::computed_value::T;
         match self.get_counters().content {
             T::Normal | T::None => true,
             T::Items(ref items) => items.is_empty(),
         }
     }
 
+    /// 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();
-        match style.column_width {
-            Either::First(_width) => true,
-            Either::Second(_auto) => match style.column_count {
-                Either::First(_n) => true,
-                Either::Second(_auto) => false,
-            }
-        }
+        self.get_column().is_multicol()
     }
 
     /// Resolves the currentColor keyword.
     ///
     /// Any color value from computed values (except for the 'color' property
     /// itself) should go through this method.
     ///
     /// Usage example:
@@ -2756,26 +2767,26 @@ impl<'a> StyleBuilder<'a> {
         % if property.style_struct.inherited:
             self.inherited_style.get_${property.style_struct.name_lower}();
         % else:
             self.inherited_style_ignoring_first_line
                 .get_${property.style_struct.name_lower}();
         % 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()
             .copy_${property.ident}_from(
                 inherited_struct,
                 % if property.logical:
                 self.writing_mode,
                 % endif
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -99,16 +99,25 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.style.get_box().clone_display() == Display::None {
             self.style.flags.insert(ComputedValueFlags::IS_IN_DISPLAY_NONE_SUBTREE);
         }
 
         if self.style.inherited_flags().contains(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE) ||
             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.
     ///
     /// The adjustments here are a subset of the adjustments generally, because
     /// text only inherits properties.
     ///
     /// Note that this, for Gecko, comes through Servo_ComputedValues_Inherit.