stylo: Rewrite getComputedStyle/getDefaultComputedStyle using StyleResolverForElement. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 09 Jul 2017 22:12:59 +0200
changeset 606875 7c1e3a43c05be81a599f7732210f5094c43862d3
parent 606874 2c453d62c33b8c41b0ac117f7801d3cb44f1240a
child 606876 5ad55b20512a7c601b06f5da755e5586decbc2e8
push id67817
push userbmo:emilio+bugs@crisal.io
push dateTue, 11 Jul 2017 14:32:38 +0000
milestone56.0a1
stylo: Rewrite getComputedStyle/getDefaultComputedStyle using StyleResolverForElement. Removing the ugly. MozReview-Commit-ID: BvahbMKS7QU
servo/components/style/style_resolver.rs
servo/components/style/traversal.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -16,24 +16,26 @@ use properties::{IS_ROOT_ELEMENT, PROHIB
 use properties::{VISITED_DEPENDENT_ONLY, cascade};
 use rule_tree::StrongRuleNode;
 use selector_parser::{PseudoElement, SelectorImpl};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, VisitedHandlingMode};
 use stylearc::Arc;
 use stylist::RuleInclusion;
 
 /// A struct that takes care of resolving the style of a given element.
-pub struct StyleResolverForElement<'a, 'b, E>
+pub struct StyleResolverForElement<'a, 'ctx, 'le, E>
 where
-    'b: 'a,
-    E: TElement + MatchMethods + 'static,
+    'ctx: 'a,
+    'le: 'ctx,
+    E: TElement + MatchMethods + 'le,
 {
     element: E,
-    context: &'a mut StyleContext<'b, E>,
+    context: &'a mut StyleContext<'ctx, E>,
     rule_inclusion: RuleInclusion,
+    _marker: ::std::marker::PhantomData<&'le E>,
 }
 
 struct MatchingResults {
     rule_node: StrongRuleNode,
     relevant_link_found: bool,
 }
 
 /// The primary style of an element or an element-backed pseudo-element.
@@ -42,28 +44,34 @@ pub struct PrimaryStyle {
     pub style: Arc<ComputedValues>,
 
     /// Whether a relevant link was found while computing this style.
     ///
     /// FIXME(emilio): Slightly out of place?
     pub relevant_link_found: bool,
 }
 
-impl<'a, 'b, E> StyleResolverForElement<'a, 'b, E>
+impl<'a, 'ctx, 'le, E> StyleResolverForElement<'a, 'ctx, 'le, E>
 where
-    'b: 'a,
-    E: TElement + MatchMethods + 'static,
+    'ctx: 'a,
+    'le: 'ctx,
+    E: TElement + MatchMethods + 'le,
 {
     /// Trivially construct a new StyleResolverForElement.
     pub fn new(
         element: E,
-        context: &'a mut StyleContext<'b, E>,
+        context: &'a mut StyleContext<'ctx, E>,
         rule_inclusion: RuleInclusion,
     ) -> Self {
-        Self { element, context, rule_inclusion, }
+        Self {
+            element,
+            context,
+            rule_inclusion,
+            _marker: ::std::marker::PhantomData,
+        }
     }
 
     /// Resolve just the style of a given element.
     pub fn resolve_primary_style(
         &mut self,
         parent_style: Option<&ComputedValues>,
         layout_parent_style: Option<&ComputedValues>,
     ) -> PrimaryStyle {
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -2,22 +2,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Traversing the DOM tree; the bloom filter.
 
 use atomic_refcell::AtomicRefCell;
 use context::{ElementCascadeInputs, StyleContext, SharedStyleContext};
 use data::{ElementData, ElementStyles};
-use dom::{DirtyDescendants, NodeInfo, OpaqueNode, TElement, TNode};
+use dom::{NodeInfo, OpaqueNode, TElement, TNode};
 use invalidation::element::restyle_hints::{RECASCADE_SELF, RECASCADE_DESCENDANTS, RestyleHint};
 use matching::{ChildCascadeRequirement, MatchMethods};
 use sharing::{StyleSharingBehavior, StyleSharingTarget};
-#[cfg(feature = "servo")] use servo_config::opts;
 use smallvec::SmallVec;
+use stylist::RuleInclusion;
 
 /// A per-traversal-level chunk of data. This is sent down by the traversal, and
 /// currently only holds the dom depth for the bloom filter.
 ///
 /// NB: Keep this as small as possible, please!
 #[derive(Clone, Debug)]
 pub struct PerLevelTraversalData {
     /// The current dom depth.
@@ -121,16 +121,18 @@ impl TraversalDriver {
     #[inline]
     pub fn is_parallel(&self) -> bool {
         matches!(*self, TraversalDriver::Parallel)
     }
 }
 
 #[cfg(feature = "servo")]
 fn is_servo_nonincremental_layout() -> bool {
+    use servo_config::opts;
+
     opts::get().nonincremental_layout
 }
 
 #[cfg(not(feature = "servo"))]
 fn is_servo_nonincremental_layout() -> bool {
     false
 }
 
@@ -515,161 +517,97 @@ pub trait DomTraversal<E: TElement> : Sy
     ///
     /// NB: We do this check on runtime. We could guarantee correctness in this
     /// regard via the type system via a `TraversalDriver` trait for this trait,
     /// that could be one of two concrete types. It's not clear whether the
     /// potential code size impact of that is worth it.
     fn is_parallel(&self) -> bool;
 }
 
-/// Helper for the function below.
-fn resolve_style_internal<E, F>(
-    context: &mut StyleContext<E>,
-    element: E, ensure_data: &F
-) -> Option<E>
-    where E: TElement,
-          F: Fn(E),
-{
-    ensure_data(element);
-    let mut data = element.mutate_data().unwrap();
-    let mut display_none_root = None;
-
-    // If the Element isn't styled, we need to compute its style.
-    if !data.has_styles() {
-        // Compute the parent style if necessary.
-        let parent = element.traversal_parent();
-        if let Some(p) = parent {
-            display_none_root = resolve_style_internal(context, p, ensure_data);
-        }
-
-        // Maintain the bloom filter. If it doesn't exist, we need to build it
-        // from scratch. Otherwise we just need to push the parent.
-        if context.thread_local.bloom_filter.is_empty() {
-            context.thread_local.bloom_filter.rebuild(element);
-        } else {
-            context.thread_local.bloom_filter.push(parent.unwrap());
-            context.thread_local.bloom_filter.assert_complete(element);
-        }
-
-        // Compute our style.
-        context.thread_local.begin_element(element, &data);
-        element.match_and_cascade(context,
-                                  &mut data,
-                                  StyleSharingBehavior::Disallow);
-        context.thread_local.end_element(element);
-
-        if !context.shared.traversal_flags.for_default_styles() {
-            // Conservatively mark us as having dirty descendants, since there
-            // might be other unstyled siblings we miss when walking straight up
-            // the parent chain.
-            //
-            // No need to do this if we're computing default styles, since
-            // resolve_default_style will want the tree to be left as it is.
-            unsafe { element.note_descendants::<DirtyDescendants>() };
-        }
-    }
-
-    // If we're display:none and none of our ancestors are, we're the root of a
-    // display:none subtree.
-    if display_none_root.is_none() && data.styles.is_display_none() {
-        display_none_root = Some(element);
-    }
-
-    return display_none_root
-}
-
 /// Manually resolve style by sequentially walking up the parent chain to the
 /// first styled Element, ignoring pending restyles. The resolved style is made
 /// available via a callback, and can be dropped by the time this function
 /// returns in the display:none subtree case.
-pub fn resolve_style<E, F, G, H>(context: &mut StyleContext<E>, element: E,
-                                 ensure_data: &F, clear_data: &G, callback: H)
-    where E: TElement,
-          F: Fn(E),
-          G: Fn(E),
-          H: FnOnce(&ElementStyles)
+pub fn resolve_style<E>(
+    context: &mut StyleContext<E>,
+    element: E,
+    rule_inclusion: RuleInclusion,
+) -> ElementStyles
+where
+    E: TElement,
 {
+    use style_resolver::StyleResolverForElement;
+
+    debug_assert!(rule_inclusion == RuleInclusion::DefaultOnly ||
+                  element.borrow_data().map_or(true, |d| !d.has_styles()),
+                  "Why are we here?");
+    let mut ancestors_requiring_style_resolution = SmallVec::<[E; 16]>::new();
+
     // Clear the bloom filter, just in case the caller is reusing TLS.
     context.thread_local.bloom_filter.clear();
 
-    // Resolve styles up the tree.
-    let display_none_root = resolve_style_internal(context, element, ensure_data);
-
-    // Make them available for the scope of the callback. The callee may use the
-    // argument, or perform any other processing that requires the styles to
-    // exist on the Element.
-    callback(&element.borrow_data().unwrap().styles);
-
-    // Clear any styles in display:none subtrees or subtrees not in the
-    // document, to leave the tree in a valid state.  For display:none subtrees,
-    // we leave the styles on the display:none root, but for subtrees not in the
-    // document, we clear styles all the way up to the root of the disconnected
-    // subtree.
-    let in_doc = element.as_node().is_in_doc();
-    if !in_doc || display_none_root.is_some() {
-        let mut curr = element;
-        loop {
-            unsafe {
-                curr.unset_dirty_descendants();
-                curr.unset_animation_only_dirty_descendants();
-            }
-            if in_doc && curr == display_none_root.unwrap() {
-                break;
-            }
-            clear_data(curr);
-            curr = match curr.traversal_parent() {
-                Some(parent) => parent,
-                None => break,
-            };
-        }
-    }
-}
-
-/// Manually resolve default styles for the given Element, which are the styles
-/// only taking into account user agent and user cascade levels.  The resolved
-/// style is made available via a callback, and will be dropped by the time this
-/// function returns.
-pub fn resolve_default_style<E, F, G, H>(
-    context: &mut StyleContext<E>,
-    element: E,
-    ensure_data: &F,
-    set_data: &G,
-    callback: H
-)
-where
-    E: TElement,
-    F: Fn(E),
-    G: Fn(E, Option<ElementData>) -> Option<ElementData>,
-    H: FnOnce(&ElementStyles),
-{
-    // Save and clear out element data from the element and its ancestors.
-    let mut old_data: SmallVec<[(E, Option<ElementData>); 8]> = SmallVec::new();
-    {
-        let mut e = element;
-        loop {
-            old_data.push((e, set_data(e, None)));
-            match e.traversal_parent() {
-                Some(parent) => e = parent,
-                None => break,
+    let mut style = None;
+    let mut ancestor = element.traversal_parent();
+    while let Some(current) = ancestor {
+        if rule_inclusion == RuleInclusion::All {
+            if let Some(data) = element.borrow_data() {
+                if let Some(ancestor_style) = data.styles.get_primary() {
+                    style = Some(ancestor_style.clone());
+                    break;
+                }
             }
         }
+        ancestors_requiring_style_resolution.push(current);
+        ancestor = current.traversal_parent();
+    }
+
+    if let Some(ancestor) = ancestor {
+        context.thread_local.bloom_filter.rebuild(ancestor);
+        context.thread_local.bloom_filter.push(ancestor);
+    }
+
+    let mut layout_parent_style = style.clone();
+    while let Some(style) = layout_parent_style.take() {
+        if !style.is_display_contents() {
+            layout_parent_style = Some(style);
+            break;
+        }
+
+        ancestor = ancestor.unwrap().traversal_parent();
+        layout_parent_style = ancestor.map(|a| {
+            a.borrow_data().unwrap().styles.primary().clone()
+        });
     }
 
-    // Resolve styles up the tree.
-    resolve_style_internal(context, element, ensure_data);
+    for ancestor in ancestors_requiring_style_resolution.iter().rev() {
+        context.thread_local.bloom_filter.assert_complete(*ancestor);
+
+        let primary_style =
+            StyleResolverForElement::new(*ancestor, context, rule_inclusion)
+                .resolve_primary_style(
+                    style.as_ref().map(|s| &**s),
+                    layout_parent_style.as_ref().map(|s| &**s)
+                );
+
+        let is_display_contents = primary_style.style.is_display_contents();
 
-    // Make them available for the scope of the callback. The callee may use the
-    // argument, or perform any other processing that requires the styles to
-    // exist on the Element.
-    callback(&element.borrow_data().unwrap().styles);
+        style = Some(primary_style.style);
+        if !is_display_contents {
+            layout_parent_style = style.clone();
+        }
 
-    // Swap the old element data back into the element and its ancestors.
-    for entry in old_data {
-        set_data(entry.0, entry.1);
+        context.thread_local.bloom_filter.push(*ancestor);
     }
+
+    context.thread_local.bloom_filter.assert_complete(element);
+    StyleResolverForElement::new(element, context, rule_inclusion)
+        .resolve_style(
+            style.as_ref().map(|s| &**s),
+            layout_parent_style.as_ref().map(|s| &**s)
+        )
 }
 
 /// Calculates the style for a single node.
 #[inline]
 #[allow(unsafe_code)]
 pub fn recalc_style_at<E, D>(
     traversal: &D,
     traversal_data: &PerLevelTraversalData,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -109,17 +109,17 @@ use style::stylesheets::StylesheetConten
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue};
 use style::stylesheets::supports_rule::parse_condition_or_declaration;
 use style::stylist::RuleInclusion;
 use style::thread_state;
 use style::timer::Timer;
 use style::traversal::{ANIMATION_ONLY, DomTraversal, FOR_CSS_RULE_CHANGES, FOR_RECONSTRUCT};
 use style::traversal::{FOR_DEFAULT_STYLES, TraversalDriver, TraversalFlags, UNSTYLED_CHILDREN_ONLY};
-use style::traversal::{resolve_style, resolve_default_style};
+use style::traversal::resolve_style;
 use style::values::{CustomIdent, KeyframesName};
 use style::values::computed::Context;
 use style_traits::{PARSING_MODE_DEFAULT, ToCss};
 use super::error_reporter::ErrorReporter;
 use super::stylesheet_loader::StylesheetLoader;
 
 /*
  * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in
@@ -2770,43 +2770,29 @@ pub extern "C" fn Servo_ResolveStyleLazi
     }
 
     let traversal_flags = match rule_inclusion {
         RuleInclusion::All => TraversalFlags::empty(),
         RuleInclusion::DefaultOnly => FOR_DEFAULT_STYLES,
     };
 
     // We don't have the style ready. Go ahead and compute it as necessary.
-    let mut result = None;
     let shared = create_shared_context(&global_style_data,
                                        &guard,
                                        &data,
                                        traversal_flags,
                                        unsafe { &*snapshots });
     let mut tlc = ThreadLocalStyleContext::new(&shared);
     let mut context = StyleContext {
         shared: &shared,
         thread_local: &mut tlc,
     };
-    let ensure = |el: GeckoElement| { unsafe { el.ensure_data(); } };
-
-    match rule_inclusion {
-        RuleInclusion::All => {
-            let clear = |el: GeckoElement| el.clear_data();
-            resolve_style(&mut context, element, &ensure, &clear,
-                          |styles| result = Some(finish(styles)));
-        }
-        RuleInclusion::DefaultOnly => {
-            let set_data = |el: GeckoElement, data| { unsafe { el.set_data(data) } };
-            resolve_default_style(&mut context, element, &ensure, &set_data,
-                                  |styles| result = Some(finish(styles)));
-        }
-    }
-
-    result.unwrap().into_strong()
+
+    let styles = resolve_style(&mut context, element, rule_inclusion);
+    finish(&styles).into_strong()
 }
 
 #[cfg(feature = "gecko_debug")]
 fn simulate_compute_values_failure(property: &PropertyValuePair) -> bool {
     let p = property.mProperty;
     let id = get_property_id_from_nscsspropertyid!(p, false);
     id.as_shorthand().is_ok() && property.mSimulateComputeValuesFailure
 }