Bug 1341083: Implement dynamic restyling for display: contents. r=heycam

MozReview-Commit-ID: KimTU2j4V4p
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

components/style/animation.rs

@@ -430,6 +430,7 @@ fn compute_style_for_animation_step(context: &SharedStyleContext,
                                                /* is_root = */ false,
                                                iter,
                                                previous_style,
+                                               previous_style,
                                                &context.default_computed_values,
                                                /* cascade_info = */ None,
                                                context.error_reporter.clone(),

components/style/gecko/media_queries.rs

@@ -492,6 +492,7 @@ impl Expression {
             is_root_element: false,
             viewport_size: device.au_viewport_size(),
             inherited_style: default_values,
+            layout_parent_style: default_values,
             // This cloning business is kind of dumb.... It's because Context
             // insists on having an actual ComputedValues inside itself.
             style: default_values.clone(),

components/style/matching.rs

@@ -443,6 +443,33 @@ struct CascadeBooleans {
 }
 
 trait PrivateMatchMethods: TElement {
+    /// Returns the closest parent element that doesn't have a display: contents
+    /// style (and thus generates a box).
+    ///
+    /// This is needed to correctly handle blockification of flex and grid
+    /// items.
+    ///
+    /// Returns itself if the element has no parent. In practice this doesn't
+    /// happen because the root element is blockified per spec, but it could
+    /// happen if we decide to not blockify for roots of disconnected subtrees,
+    /// which is a kind of dubious beahavior.
+    fn layout_parent(&self) -> Self {
+        let mut current = self.clone();
+        loop {
+            current = match current.parent_element() {
+                Some(el) => el,
+                None => return current,
+            };
+
+            let is_display_contents =
+                current.borrow_data().unwrap().styles().primary.values().is_display_contents();
+
+            if !is_display_contents {
+                return current;
+            }
+        }
+    }
+
     fn cascade_internal(&self,
                         context: &StyleContext<Self>,
                         primary_style: &ComputedStyle,
@@ -467,7 +494,7 @@ trait PrivateMatchMethods: TElement {
         let parent_data;
         let inherited_values_ = if pseudo_style.is_none() {
             parent_el = self.parent_element();
-            parent_data = parent_el.as_ref().and_then(|x| x.borrow_data());
+            parent_data = parent_el.as_ref().and_then(|e| e.borrow_data());
             let parent_values = parent_data.as_ref().map(|d| {
                 // Sometimes Gecko eagerly styles things without processing
                 // pending restyles first. In general we'd like to avoid this,
@@ -479,25 +506,44 @@ trait PrivateMatchMethods: TElement {
                 d.styles().primary.values()
             });
 
-            // Propagate the "can be fragmented" bit. It would be nice to
-            // encapsulate this better.
-            if let Some(ref p) = parent_values {
-                let can_be_fragmented =
-                    p.is_multicol() || parent_el.unwrap().as_node().can_be_fragmented();
-                unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); }
-            }
-
             parent_values
         } else {
+            parent_el = Some(self.clone());
             Some(primary_style.values())
         };
+
+        let mut layout_parent_el = parent_el.clone();
+        let layout_parent_data;
+        let mut layout_parent_style = inherited_values_;
+        if inherited_values_.map_or(false, |s| s.is_display_contents()) {
+            layout_parent_el = Some(layout_parent_el.unwrap().layout_parent());
+            layout_parent_data = layout_parent_el.as_ref().unwrap().borrow_data().unwrap();
+            layout_parent_style = Some(layout_parent_data.styles().primary.values())
+        }
+
         let inherited_values = inherited_values_.map(|x| &**x);
+        let layout_parent_style = layout_parent_style.map(|x| &**x);
+
+        // Propagate the "can be fragmented" bit. It would be nice to
+        // encapsulate this better.
+        //
+        // Note that this is not needed for pseudos since we already do that
+        // when we resolve the non-pseudo style.
+        if pseudo_style.is_none() {
+            if let Some(ref p) = layout_parent_style {
+                let can_be_fragmented =
+                    p.is_multicol() ||
+                    layout_parent_el.as_ref().unwrap().as_node().can_be_fragmented();
+                unsafe { self.as_node().set_can_be_fragmented(can_be_fragmented); }
+            }
+        }
 
         // Invoke the cascade algorithm.
         let values =
             Arc::new(cascade(shared_context.viewport_size,
                              rule_node,
                              inherited_values,
+                             layout_parent_style,
                              &shared_context.default_computed_values,
                              Some(&mut cascade_info),
                              shared_context.error_reporter.clone(),

components/style/properties/gecko.mako.rs

@@ -130,6 +130,11 @@ impl ComputedValues {
         })
     }
 
+    #[inline]
+    pub fn is_display_contents(&self) -> bool {
+        self.get_box().clone_display() == longhands::display::computed_value::T::contents
+    }
+
     % for style_struct in data.style_structs:
     #[inline]
     pub fn clone_${style_struct.name_lower}(&self) -> Arc<style_structs::${style_struct.name}> {

components/style/properties/properties.mako.rs

@@ -1424,6 +1424,11 @@ impl ComputedValues {
     /// Servo for obvious reasons.
     pub fn has_moz_binding(&self) -> bool { false }
 
+    /// Returns whether this style's display value is equal to contents.
+    ///
+    /// Since this isn't supported in Servo, this is always false for Servo.
+    pub fn is_display_contents(&self) -> bool { false }
+
     /// Get the root font size.
     fn root_font_size(&self) -> Au { self.root_font_size }
 
@@ -1761,14 +1766,16 @@ bitflags! {
 pub fn cascade(viewport_size: Size2D<Au>,
                rule_node: &StrongRuleNode,
                parent_style: Option<<&ComputedValues>,
+               layout_parent_style: Option<<&ComputedValues>,
                default_style: &Arc<ComputedValues>,
                cascade_info: Option<<&mut CascadeInfo>,
                error_reporter: StdBox<ParseErrorReporter + Send>,
                flags: CascadeFlags)
                -> ComputedValues {
-    let (is_root_element, inherited_style) = match parent_style {
+    debug_assert_eq!(parent_style.is_some(), layout_parent_style.is_some());
-        Some(parent_style) => (false, parent_style),
+    let (is_root_element, inherited_style, layout_parent_style) = match parent_style {
-        None => (true, &**default_style),
+        Some(parent_style) => (false, parent_style, layout_parent_style.unwrap()),
+        None => (true, &**default_style, &**default_style),
     };
     // Hold locks until after the apply_declarations() call returns.
     // Use filter_map because the root node has no style source.
@@ -1793,6 +1800,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
                        is_root_element,
                        iter_declarations,
                        inherited_style,
+                       layout_parent_style,
                        default_style,
                        cascade_info,
                        error_reporter,
@@ -1806,6 +1814,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
                                     is_root_element: bool,
                                     iter_declarations: F,
                                     inherited_style: &ComputedValues,
+                                    layout_parent_style: &ComputedValues,
                                     default_style: &Arc<ComputedValues>,
                                     mut cascade_info: Option<<&mut CascadeInfo>,
                                     mut error_reporter: StdBox<ParseErrorReporter + Send>,
@@ -1861,6 +1870,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
         is_root_element: is_root_element,
         viewport_size: viewport_size,
         inherited_style: inherited_style,
+        layout_parent_style: layout_parent_style,
         style: starting_style,
         font_metrics_provider: font_metrics_provider,
     };
@@ -1943,10 +1953,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
         longhands::position::SpecifiedValue::absolute |
         longhands::position::SpecifiedValue::fixed);
     let floated = style.get_box().clone_float() != longhands::float::computed_value::T::none;
-    // FIXME(heycam): We should look past any display:contents ancestors to
+    let is_item = matches!(context.layout_parent_style.get_box().clone_display(),
-    // determine if we are a flex or grid item, but we don't have access to
-    // grandparent or higher style here.
-    let is_item = matches!(context.inherited_style.get_box().clone_display(),
         % if product == "gecko":
         computed_values::display::T::grid |
         computed_values::display::T::inline_grid |
@@ -2023,7 +2030,7 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
         use computed_values::align_items::T as align_items;
         if style.get_position().clone_align_self() == computed_values::align_self::T::auto && !positioned {
             let self_align =
-                match context.inherited_style.get_position().clone_align_items() {
+                match context.layout_parent_style.get_position().clone_align_items() {
                     align_items::stretch => align_self::stretch,
                     align_items::baseline => align_self::baseline,
                     align_items::flex_start => align_self::flex_start,

components/style/servo/media_queries.rs

@@ -180,6 +180,7 @@ impl Range<specified::Length> {
             is_root_element: false,
             viewport_size: viewport_size,
             inherited_style: default_values,
+            layout_parent_style: default_values,
             // This cloning business is kind of dumb.... It's because Context
             // insists on having an actual ComputedValues inside itself.
             style: default_values.clone(),

components/style/stylist.rs

@@ -313,10 +313,25 @@ impl Stylist {
             flags.insert(INHERIT_ALL)
         }
 
+        // NOTE(emilio): We skip calculating the proper layout parent style
+        // here.
+        //
+        // It'd be fine to assert that this isn't called with a parent style
+        // where display contents is in effect, but in practice this is hard to
+        // do for stuff like :-moz-fieldset-content with a
+        // <fieldset style="display: contents">. That is, the computed value of
+        // display for the fieldset is "contents", even though it's not the used
+        // value, so we don't need to adjust in a different way anyway.
+        //
+        // In practice, I don't think any anonymous content can be a direct
+        // descendant of a display: contents element where display: contents is
+        // the actual used value, and the computed value of it would need
+        // blockification.
         let computed =
             properties::cascade(self.device.au_viewport_size(),
                                 &rule_node,
                                 parent.map(|p| &**p),
+                                parent.map(|p| &**p),
                                 default,
                                 None,
                                 Box::new(StdoutErrorReporter),
@@ -389,10 +404,15 @@ impl Stylist {
             self.rule_tree.insert_ordered_rules(
                 declarations.into_iter().map(|a| (a.source, a.level)));
 
+        // Read the comment on `precomputed_values_for_pseudo` to see why it's
+        // difficult to assert that display: contents nodes never arrive here
+        // (tl;dr: It doesn't apply for replaced elements and such, but the
+        // computed value is still "contents").
         let computed =
             properties::cascade(self.device.au_viewport_size(),
                                 &rule_node,
                                 Some(&**parent),
+                                Some(&**parent),
                                 default,
                                 None,
                                 Box::new(StdoutErrorReporter),

components/style/values/computed/mod.rs

@@ -43,6 +43,12 @@ pub struct Context<'a> {
     /// The style we're inheriting from.
     pub inherited_style: &'a ComputedValues,
 
+    /// The style of the layout parent node. This will almost always be
+    /// `inherited_style`, except when `display: contents` is at play, in which
+    /// case it's the style of the last ancestor with a `display` value that
+    /// isn't `contents`.
+    pub layout_parent_style: &'a ComputedValues,
+
     /// Values access through this need to be in the properties "computed
     /// early": color, text-decoration, font-size, display, position, float,
     /// border-*-style, outline-style, font-family, writing-mode...

components/style/viewport.rs

@@ -689,6 +689,7 @@ impl MaybeNew for ViewportConstraints {
             is_root_element: false,
             viewport_size: initial_viewport,
             inherited_style: device.default_values(),
+            layout_parent_style: device.default_values(),
             style: device.default_values().clone(),
             font_metrics_provider: None, // TODO: Should have!
         };

ports/geckolib/glue.rs

@@ -645,7 +645,10 @@ fn get_pseudo_style(element: GeckoElement, pseudo_tag: *mut nsIAtom,
         PseudoElementCascadeType::Lazy => {
             let d = doc_data.borrow_mut();
             let base = styles.primary.values();
-            d.stylist.lazily_compute_pseudo_element_style(&element, &pseudo, base, &d.default_computed_values())
+            d.stylist.lazily_compute_pseudo_element_style(&element,
+                                                          &pseudo,
+                                                          base,
+                                                          &d.default_computed_values())
                      .map(|s| s.values().clone())
         },
     }
@@ -1348,6 +1351,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeLis
         // FIXME (bug 1303229): Use the actual viewport size here
         viewport_size: Size2D::new(Au(0), Au(0)),
         inherited_style: parent_style.unwrap_or(&init),
+        layout_parent_style: parent_style.unwrap_or(&init),
         style: (**style).clone(),
         font_metrics_provider: None,
     };

tests/unit/style/parsing/image.rs

@@ -49,6 +49,7 @@ fn test_linear_gradient() {
         is_root_element: true,
         viewport_size: container,
         inherited_style: initial_style,
+        layout_parent_style: initial_style,
         style: initial_style.clone(),
         font_metrics_provider: None,
     };