Bug 1317016 - Basic infrastructure for RestyleHint-driven traversal.

MozReview-Commit-ID: 7wH5XcILVmX

components/layout/construct.rs

@@ -1354,8 +1354,8 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
         let mut set_has_newly_constructed_flow_flag = false;
         let result = {
             let mut style = node.style(self.style_context());
+            let damage = node.restyle_damage();
             let mut data = node.mutate_layout_data().unwrap();
-            let damage = data.base.restyle_damage;
 
             match *node.construction_result_mut(&mut *data) {
                 ConstructionResult::None => true,

components/layout/query.rs

@@ -29,6 +29,7 @@ use std::ops::Deref;
 use std::sync::{Arc, Mutex};
 use style::computed_values;
 use style::context::StyleContext;
+use style::dom::TElement;
 use style::logical_geometry::{WritingMode, BlockFlowDirection, InlineBaseDirection};
 use style::properties::longhands::{display, position};
 use style::properties::style_structs;
@@ -607,20 +608,6 @@ pub fn process_node_scroll_area_request< N: LayoutNode>(requested_node: N, layou
     }
 }
 
-/// Ensures that a node's data, and all its parents' is initialized. This is
-/// needed to resolve style lazily.
-fn ensure_node_data_initialized<N: LayoutNode>(node: &N) {
-    let mut cur = Some(node.clone());
-    while let Some(current) = cur {
-        if current.borrow_layout_data().is_some() {
-            break;
-        }
-
-        current.initialize_data();
-        cur = current.parent_node();
-    }
-}
-
 /// Return the resolved value of property for a given (pseudo)element.
 /// https://drafts.csswg.org/cssom/#resolved-value
 pub fn process_resolved_style_request<'a, N, C>(requested_node: N,
@@ -631,14 +618,24 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N,
     where N: LayoutNode,
           C: StyleContext<'a>
 {
-    use style::traversal::ensure_element_styled;
+    use style::traversal::{clear_descendant_data, style_element_in_display_none_subtree};
+    let element = requested_node.as_element().unwrap();
 
-    // This node might have display: none, or it's style might be not up to
-    // date, so we might need to do style recalc.
-    //
-    // FIXME(emilio): Is a bit shame we have to do this instead of in style.
-    ensure_node_data_initialized(&requested_node);
-    ensure_element_styled(requested_node.as_element().unwrap(), style_context);
+    // We call process_resolved_style_request after performing a whole-document
+    // traversal, so the only reason we wouldn't have an up-to-date style here
+    // is that the requested node is in a display:none subtree. We currently
+    // maintain the invariant that elements in display:none subtrees always have
+    // no ElementData, so we need to temporarily bend those invariants here, and
+    // then throw them the style data away again before returning to preserve them.
+    // We could optimize this later to keep the style data cached somehow, but
+    // we'd need a mechanism to prevent detect when it's stale (since we don't
+    // traverse display:none subtrees during restyle).
+    let display_none_root = if element.get_data().is_none() {
+        Some(style_element_in_display_none_subtree(element, &|e| e.as_node().initialize_data(),
+                                                   style_context))
+    } else {
+        None
+    };
 
     let layout_el = requested_node.to_threadsafe().as_element().unwrap();
     let layout_el = match *pseudo {
@@ -662,6 +659,10 @@ pub fn process_resolved_style_request<'a, N, C>(requested_node: N,
 
     let style = &*layout_el.resolved_style();
 
+    // Clear any temporarily-resolved data to maintain our invariants. See the comment
+    // at the top of this function.
+    display_none_root.map(|r| clear_descendant_data(r, &|e| e.as_node().clear_data()));
+
     let positioned = match style.get_box().position {
         position::computed_value::T::relative |
         /*position::computed_value::T::sticky |*/

components/layout/traversal.rs

@@ -112,15 +112,11 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
         construct_flows_at(&self.context, self.root, node);
     }
 
-    fn should_traverse_child(parent: N::ConcreteElement, child: N) -> bool {
-        // If the parent is display:none, we don't need to do anything.
-        if parent.is_display_none() {
-            return false;
-        }
-
+    fn should_traverse_child(child: N, restyled_previous_sibling_element: bool) -> bool {
         match child.as_element() {
             // Elements should be traversed if they need styling or flow construction.
-            Some(el) => el.styling_mode() != StylingMode::Stop ||
+            Some(el) => restyled_previous_sibling_element ||
+                        el.styling_mode() != StylingMode::Stop ||
                         el.as_node().to_threadsafe().restyle_damage() != RestyleDamage::empty(),
 
             // Text nodes never need styling. However, there are two cases they may need
@@ -128,7 +124,7 @@ impl<'lc, N> DomTraversalContext<N> for RecalcStyleAndConstructFlows<'lc>
             // (1) They child doesn't yet have layout data (preorder traversal initializes it).
             // (2) The parent element has restyle damage (so the text flow also needs fixup).
             None => child.get_raw_data().is_none() ||
-                    parent.as_node().to_threadsafe().restyle_damage() != RestyleDamage::empty(),
+                    child.parent_node().unwrap().to_threadsafe().restyle_damage() != RestyleDamage::empty(),
         }
     }
 
@@ -156,6 +152,8 @@ pub trait PostorderNodeMutTraversal<ConcreteThreadSafeLayoutNode: ThreadSafeLayo
 #[inline]
 #[allow(unsafe_code)]
 fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: OpaqueNode, node: N) {
+    debug!("construct_flows_at: {:?}", node);
+
     // Construct flows for this node.
     {
         let tnode = node.to_threadsafe();
@@ -167,16 +165,18 @@ fn construct_flows_at<'a, N: LayoutNode>(context: &'a LayoutContext<'a>, root: O
             let mut flow_constructor = FlowConstructor::new(context);
             if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) {
                 flow_constructor.process(&tnode);
-                debug!("Constructed flow for {:x}: {:x}",
-                       tnode.debug_id(),
+                debug!("Constructed flow for {:?}: {:x}",
+                       tnode,
                        tnode.flow_debug_id());
             }
         }
-
-        tnode.clear_restyle_damage();
     }
 
-    unsafe { node.clear_dirty_bits(); }
+    if let Some(el) = node.as_element() {
+        el.mutate_data().unwrap().persist();
+        unsafe { el.unset_dirty_descendants(); }
+    }
+
     remove_from_bloom_filter(context, root, node);
 }
 

components/layout/wrapper.rs

@@ -37,8 +37,6 @@ use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutElemen
 use script_layout_interface::wrapper_traits::GetLayoutData;
 use style::atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
 use style::computed_values::content::{self, ContentItem};
-use style::dom::TElement;
-use style::traversal::prepare_for_styling;
 
 pub type NonOpaqueStyleAndLayoutData = AtomicRefCell<PersistentLayoutData>;
 
@@ -97,9 +95,6 @@ impl<T: LayoutNode> LayoutNodeHelpers for T {
                 ptr: unsafe { NonZero::new(ptr as *mut AtomicRefCell<PartialPersistentLayoutData>) }
             };
             unsafe { self.init_style_and_layout_data(opaque) };
-            if let Some(el) = self.as_element() {
-                let _ = prepare_for_styling(el, el.get_data().unwrap());
-            }
         };
     }