style: Skip parent display-based style fixups when restyling anon boxes. r=emilio draft
authorCameron McCormack <cam@mcc.id.au>
Mon, 24 Apr 2017 16:31:00 +0800
changeset 567516 6dd8a680b92b885bac43d48b55d5f347556a3282
parent 567515 a19d334d84b14969d85a4b9d12f11d8e61235f80
child 567517 8eedad34cdbd070f931844fb8c41d39fffdb9e5c
child 568336 fa8f87a95251c7f28319306a2e6293bc9345d1d1
push id55600
push userbmo:cam@mcc.id.au
push dateTue, 25 Apr 2017 04:55:45 +0000
reviewersemilio
milestone55.0a1
style: Skip parent display-based style fixups when restyling anon boxes. r=emilio MozReview-Commit-ID: KPY1K3hUTrQ
servo/components/script/layout_wrapper.rs
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/matching.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/script/layout_wrapper.rs
+++ b/servo/components/script/layout_wrapper.rs
@@ -444,17 +444,17 @@ impl<'le> TElement for ServoLayoutElemen
             self.get_style_and_layout_data().map(|d| {
                 let ppld: &AtomicRefCell<PartialPersistentLayoutData> = &**d.ptr;
                 let psd: &AtomicRefCell<ElementData> = transmute(ppld);
                 psd
             })
         }
     }
 
-    fn skip_root_and_item_based_display_fixup(&self) -> bool {
+    fn skip_root_and_item_based_display_fixup(&self, _pseudo: Option<&PseudoElement>) -> bool {
         false
     }
 
     unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) {
         self.element.insert_selector_flags(flags);
     }
 
     fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool {
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -442,17 +442,17 @@ pub trait TElement : Eq + PartialEq + De
     /// Mutably borrows the ElementData.
     fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
         self.get_data().map(|x| x.borrow_mut())
     }
 
     /// Whether we should skip any root- or item-based display property
     /// blockification on this element.  (This function exists so that Gecko
     /// native anonymous content can opt out of this style fixup.)
-    fn skip_root_and_item_based_display_fixup(&self) -> bool;
+    fn skip_root_and_item_based_display_fixup(&self, pseudo: Option<&PseudoElement>) -> bool;
 
     /// Sets selector flags, which indicate what kinds of selectors may have
     /// matched on this element and therefore what kind of work may need to
     /// be performed when DOM state changes.
     ///
     /// This is unsafe, like all the flag-setting methods, because it's only safe
     /// to call with exclusive access to the element. When setting flags on the
     /// parent during parallel traversal, we use SequentialTask to queue up the
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -642,23 +642,33 @@ impl<'le> TElement for GeckoElement<'le>
     fn did_process_child(&self) -> isize {
         panic!("Atomic child count not implemented in Gecko");
     }
 
     fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
         unsafe { self.0.mServoData.get().as_ref() }
     }
 
-    fn skip_root_and_item_based_display_fixup(&self) -> bool {
+    fn skip_root_and_item_based_display_fixup(&self, pseudo: Option<&PseudoElement>) -> bool {
         // We don't want to fix up display values of native anonymous content.
         // Additionally, we want to skip root-based display fixup for document
         // level native anonymous content subtree roots, since they're not
         // really roots from the style fixup perspective.  Checking that we
         // are NAC handles both cases.
-        self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) != 0
+        if self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) != 0 {
+            return true;
+        }
+
+        // Also, check whether this is a pseudo-element that explicitly opts out
+        // of parent display-based style fixups.
+        if pseudo.map_or(false, |p| p.skips_display_fixup()) {
+            return true;
+        }
+
+        false
     }
 
     unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) {
         debug_assert!(!flags.is_empty());
         self.set_flags(selector_flags_to_node_flags(flags));
     }
 
     fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool {
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -381,28 +381,28 @@ trait PrivateMatchMethods: TElement {
         }
     }
 
     fn cascade_with_rules(&self,
                           shared_context: &SharedStyleContext,
                           font_metrics_provider: &FontMetricsProvider,
                           rule_node: &StrongRuleNode,
                           primary_style: &ComputedStyle,
-                          is_pseudo: bool)
+                          pseudo: Option<&PseudoElement>)
                           -> Arc<ComputedValues> {
         let mut cascade_info = CascadeInfo::new();
         let mut cascade_flags = CascadeFlags::empty();
-        if self.skip_root_and_item_based_display_fixup() {
+        if self.skip_root_and_item_based_display_fixup(pseudo) {
             cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP)
         }
 
         // Grab the inherited values.
         let parent_el;
         let parent_data;
-        let style_to_inherit_from = if !is_pseudo {
+        let style_to_inherit_from = if pseudo.is_none() {
             parent_el = self.parent_element();
             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,
                 // but there can be good reasons (for example, needing to
                 // construct a frame for some small piece of newly-added
                 // content in order to do something specific with that frame,
@@ -429,17 +429,17 @@ trait PrivateMatchMethods: TElement {
         let style_to_inherit_from = style_to_inherit_from.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 !is_pseudo {
+        if pseudo.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); }
             }
         }
 
@@ -457,26 +457,29 @@ trait PrivateMatchMethods: TElement {
 
         cascade_info.finish(&self.as_node());
         values
     }
 
     fn cascade_internal(&self,
                         context: &StyleContext<Self>,
                         primary_style: &ComputedStyle,
+                        pseudo: Option<&PseudoElement>,
                         pseudo_style: Option<&ComputedStyle>)
                         -> Arc<ComputedValues> {
+        debug_assert_eq!(pseudo.is_some(), pseudo_style.is_some());
+
         // Grab the rule node.
         let rule_node = &pseudo_style.unwrap_or(primary_style).rules;
 
         self.cascade_with_rules(context.shared,
                                 &context.thread_local.font_metrics_provider,
                                 rule_node,
                                 primary_style,
-                                pseudo_style.is_some())
+                                pseudo)
     }
 
     /// Computes values and damage for the primary or pseudo style of an element,
     /// setting them on the ElementData.
     fn cascade_primary_or_pseudo(&self,
                                  context: &mut StyleContext<Self>,
                                  data: &mut ElementData,
                                  pseudo: Option<&PseudoElement>,
@@ -497,16 +500,17 @@ trait PrivateMatchMethods: TElement {
         let mut old_values = match pseudo_style {
             Some(ref mut s) => s.values.take(),
             None => primary_style.values.take(),
         };
 
         // Compute the new values.
         let mut new_values = self.cascade_internal(context,
                                                    primary_style,
+                                                   pseudo,
                                                    pseudo_style.as_ref().map(|s| &**s));
 
         // Handle animations.
         if animate && !context.shared.traversal_flags.for_animation_only() {
             self.process_animations(context,
                                     &mut old_values,
                                     &mut new_values,
                                     primary_style,
@@ -529,33 +533,36 @@ trait PrivateMatchMethods: TElement {
     }
 
     /// get_after_change_style removes the transition rules from the ComputedValues.
     /// If there is no transition rule in the ComputedValues, it returns None.
     #[cfg(feature = "gecko")]
     fn get_after_change_style(&self,
                               context: &mut StyleContext<Self>,
                               primary_style: &ComputedStyle,
+                              pseudo: Option<&PseudoElement>,
                               pseudo_style: Option<&ComputedStyle>)
                               -> Option<Arc<ComputedValues>> {
+        debug_assert_eq!(pseudo.is_some(), pseudo_style.is_some());
+
         let relevant_style = pseudo_style.unwrap_or(primary_style);
         let rule_node = &relevant_style.rules;
         let without_transition_rules =
             context.shared.stylist.rule_tree.remove_transition_rule_if_applicable(rule_node);
         if without_transition_rules == *rule_node {
             // We don't have transition rule in this case, so return None to let the caller
             // use the original ComputedValues.
             return None;
         }
 
         Some(self.cascade_with_rules(context.shared,
                                      &context.thread_local.font_metrics_provider,
                                      &without_transition_rules,
                                      primary_style,
-                                     pseudo_style.is_some()))
+                                     pseudo))
     }
 
     #[cfg(feature = "gecko")]
     fn needs_animations_update(&self,
                                old_values: &Option<Arc<ComputedValues>>,
                                new_values: &Arc<ComputedValues>,
                                pseudo: Option<&PseudoElement>) -> bool {
         let ref new_box_style = new_values.get_box();
@@ -595,17 +602,17 @@ trait PrivateMatchMethods: TElement {
         if self.needs_animations_update(old_values, new_values, pseudo) {
             tasks.insert(CSS_ANIMATIONS);
         }
 
         let before_change_style = if self.might_need_transitions_update(&old_values.as_ref(),
                                                                         new_values,
                                                                         pseudo) {
             let after_change_style = if self.has_css_transitions(pseudo) {
-                self.get_after_change_style(context, primary_style, pseudo_style)
+                self.get_after_change_style(context, primary_style, pseudo, pseudo_style)
             } else {
                 None
             };
 
             // In order to avoid creating a SequentialTask for transitions which may not be updated,
             // we check it per property to make sure Gecko side will really update transition.
             let needs_transitions_update = {
                 // We borrow new_values here, so need to add a scope to make sure we release it
@@ -1320,30 +1327,33 @@ pub trait MatchMethods : TElement {
         }
     }
 
     /// Returns computed values without animation and transition rules.
     fn get_base_style(&self,
                       shared_context: &SharedStyleContext,
                       font_metrics_provider: &FontMetricsProvider,
                       primary_style: &ComputedStyle,
+                      pseudo: Option<&PseudoElement>,
                       pseudo_style: Option<&ComputedStyle>)
                       -> Arc<ComputedValues> {
+        debug_assert_eq!(pseudo.is_some(), pseudo_style.is_some());
+
         let relevant_style = pseudo_style.unwrap_or(primary_style);
         let rule_node = &relevant_style.rules;
         let without_animation_rules =
             shared_context.stylist.rule_tree.remove_animation_and_transition_rules(rule_node);
         if without_animation_rules == *rule_node {
             // Note that unwrapping here is fine, because the style is
             // only incomplete during the styling process.
             return relevant_style.values.as_ref().unwrap().clone();
         }
 
         self.cascade_with_rules(shared_context,
                                 font_metrics_provider,
                                 &without_animation_rules,
                                 primary_style,
-                                pseudo_style.is_some())
+                                pseudo)
     }
 
 }
 
 impl<E: TElement> MatchMethods for E {}
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -461,17 +461,17 @@ pub extern "C" fn Servo_StyleSet_GetBase
             let style = pseudos.get(p);
             debug_assert!(style.is_some());
             style
         }
         None => None,
     };
 
     let provider = get_metrics_provider_for_product();
-    element.get_base_style(shared_context, &provider, &styles.primary, pseudo_style)
+    element.get_base_style(shared_context, &provider, &styles.primary, pseudo.as_ref(), pseudo_style)
            .into_strong()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue(computed_values: ServoComputedValuesBorrowed,
                                                              property_id: nsCSSPropertyID)
                                                              -> RawServoAnimationValueStrong
 {