Bug 1351535 - Part 6: Handle TraversalRestyleBehavior::ForReconstruct in the Servo restyle. r?bholley draft
authorCameron McCormack <cam@mcc.id.au>
Tue, 04 Apr 2017 19:32:47 +0800
changeset 555570 ef16495b630d27a41371c671d9bb2db9790bf322
parent 555569 4d050465fee8a64e4b3443ea512d8698c2450a46
child 555571 daf15541bbd22a1e46431cf2da89a703271007a9
push id52264
push userbmo:cam@mcc.id.au
push dateTue, 04 Apr 2017 13:50:52 +0000
reviewersbholley
bugs1351535
milestone55.0a1
Bug 1351535 - Part 6: Handle TraversalRestyleBehavior::ForReconstruct in the Servo restyle. r?bholley MozReview-Commit-ID: 6vz9gHgzK87
servo/components/style/context.rs
servo/components/style/data.rs
servo/components/style/matching.rs
servo/components/style/traversal.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/context.rs
+++ b/servo/components/style/context.rs
@@ -86,16 +86,21 @@ pub struct SharedStyleContext<'a> {
     /// them.
     pub timer: Timer,
 
     /// The QuirksMode state which the document needs to be rendered with
     pub quirks_mode: QuirksMode,
 
     /// True if the traversal is processing only animation restyles.
     pub animation_only_restyle: bool,
+
+    /// True if we are restyling a subtree in preparation for reconstructing its
+    /// frames. The result is new styles will be computed for the subtree, and
+    /// any existing styles are discarded without generating change hints.
+    pub restyle_for_reconstruct: bool,
 }
 
 impl<'a> SharedStyleContext<'a> {
     /// Return a suitable viewport size in order to be used for viewport units.
     pub fn viewport_size(&self) -> Size2D<Au> {
         self.stylist.device.au_viewport_size()
     }
 }
--- a/servo/components/style/data.rs
+++ b/servo/components/style/data.rs
@@ -161,16 +161,23 @@ impl StoredRestyleHint {
     }
 
     /// Creates a restyle hint that forces the whole subtree to be restyled,
     /// including the element.
     pub fn subtree() -> Self {
         StoredRestyleHint(RESTYLE_SELF | RESTYLE_DESCENDANTS)
     }
 
+    /// Creates a restyle hint that forces the element and all its later
+    /// siblings to have their whole subtrees restyled, including the elements
+    /// themselves.
+    pub fn subtree_and_later_siblings() -> Self {
+        StoredRestyleHint(RESTYLE_SELF | RESTYLE_DESCENDANTS | RESTYLE_LATER_SIBLINGS)
+    }
+
     /// Returns true if the hint indicates that our style may be invalidated.
     pub fn has_self_invalidations(&self) -> bool {
         self.0.intersects(RestyleHint::for_self())
     }
 
     /// Returns true if the hint indicates that our sibling's style may be
     /// invalidated.
     pub fn has_sibling_invalidations(&self) -> bool {
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -586,19 +586,21 @@ trait PrivateMatchMethods: TElement {
         if booleans.animate {
             self.process_animations(context,
                                     &mut old_values,
                                     &mut new_values,
                                     pseudo,
                                     possibly_expired_animations);
         }
 
-        // Accumulate restyle damage.
-        if let Some(old) = old_values {
-            self.accumulate_damage(restyle.unwrap(), &old, &new_values, pseudo);
+        // Accumulate restyle damage, unless we are in a restyle for reconstruction.
+        if !context.shared.restyle_for_reconstruct {
+            if let Some(old) = old_values {
+                self.accumulate_damage(restyle.unwrap(), &old, &new_values, pseudo);
+            }
         }
 
         // Set the new computed values.
         if let Some((_, ref mut style)) = pseudo_style {
             style.values = Some(new_values);
         } else {
             primary_style.values = Some(new_values);
         }
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -30,31 +30,39 @@ pub struct PerLevelTraversalData {
     pub current_dom_depth: Option<usize>,
 }
 
 bitflags! {
     /// Represents that target elements of the traversal.
     pub flags TraversalFlags: u8 {
         /// Traverse only unstyled children.
         const UNSTYLED_CHILDREN_ONLY = 0x01,
-        /// Traverse only elements for animation restyles
+        /// Traverse only elements for animation restyles.
         const ANIMATION_ONLY = 0x02,
+        /// Traverse the entire subtree, ignoring any existing styles, and
+        /// therefore not generating any change hints.
+        const FOR_RECONSTRUCT = 0x04,
     }
 }
 
 impl TraversalFlags {
     /// Returns true if the traversal is for animation-only restyles.
     pub fn for_animation_only(&self) -> bool {
         self.contains(ANIMATION_ONLY)
     }
 
     /// Returns true if the traversal is for unstyled children.
     pub fn for_unstyled_children_only(&self) -> bool {
         self.contains(UNSTYLED_CHILDREN_ONLY)
     }
+
+    /// Returns true if the traversal is for a frame reconstruction.
+    pub fn for_reconstruct(&self) -> bool {
+        self.contains(FOR_RECONSTRUCT)
+    }
 }
 
 /// This structure exists to enforce that callers invoke pre_traverse, and also
 /// to pass information from the pre-traversal into the primary traversal.
 pub struct PreTraverseToken {
     traverse: bool,
     unstyled_children_only: bool,
 }
@@ -133,16 +141,20 @@ pub trait DomTraversal<E: TElement> : Sy
     /// The traversal_flag is used in Gecko.
     /// If traversal_flag::UNSTYLED_CHILDREN_ONLY is specified, style newly-
     /// appended children without restyling the parent.
     /// If traversal_flag::ANIMATION_ONLY is specified, style only elements for
     /// animations.
     fn pre_traverse(root: E, stylist: &Stylist, traversal_flags: TraversalFlags)
                     -> PreTraverseToken
     {
+        debug_assert!(!(traversal_flags.for_reconstruct() &&
+                        traversal_flags.for_unstyled_children_only()),
+                      "must not specify FOR_RECONSTRUCT in combination with UNSTYLED_CHILDREN_ONLY");
+
         if traversal_flags.for_unstyled_children_only() {
             if root.borrow_data().map_or(true, |d| d.has_styles() && d.styles().is_display_none()) {
                 return PreTraverseToken {
                     traverse: false,
                     unstyled_children_only: false,
                 };
             }
             return PreTraverseToken {
@@ -154,23 +166,24 @@ pub trait DomTraversal<E: TElement> : Sy
         // Expand the snapshot, if any. This is normally handled by the parent, so
         // we need a special case for the root.
         //
         // Expanding snapshots here may create a LATER_SIBLINGS restyle hint, which
         // we will drop on the floor. To prevent missed restyles, we assert against
         // restyling a root with later siblings.
         if let Some(mut data) = root.mutate_data() {
             if let Some(r) = data.get_restyle_mut() {
-                debug_assert!(root.next_sibling_element().is_none());
-                let _later_siblings = r.compute_final_hint(root, stylist);
+                let later_siblings = r.compute_final_hint(root, stylist);
+                debug_assert!(!later_siblings || traversal_flags.for_reconstruct());
             }
         }
 
         PreTraverseToken {
-            traverse: Self::node_needs_traversal(root.as_node(), traversal_flags.for_animation_only()),
+            traverse: traversal_flags.for_reconstruct() ||
+                      Self::node_needs_traversal(root.as_node(), traversal_flags.for_animation_only()),
             unstyled_children_only: false,
         }
     }
 
     /// Returns true if traversal should visit a text node. The style system never
     /// processes text nodes, but Servo overrides this to visit them for flow
     /// construction when necessary.
     fn text_node_needs_traversal(node: E::ConcreteNode) -> bool {
@@ -327,21 +340,25 @@ pub trait DomTraversal<E: TElement> : Sy
                                           &parent.borrow_data().unwrap(), MayLog);
         thread_local.borrow_mut().end_element(parent);
         if !should_traverse {
             return;
         }
 
         for kid in parent.as_node().children() {
             if Self::node_needs_traversal(kid, self.shared_context().animation_only_restyle) {
-                let el = kid.as_element();
-                if el.as_ref().and_then(|el| el.borrow_data())
-                              .map_or(false, |d| d.has_styles())
-                {
-                    unsafe { parent.set_dirty_descendants(); }
+                // If we are in a restyle for reconstruction, there is no need to
+                // perform a post-traversal, so we don't need to set the dirty
+                // descendants bit on the parent.
+                if !self.shared_context().restyle_for_reconstruct {
+                    let el = kid.as_element();
+                    if el.as_ref().and_then(|el| el.borrow_data())
+                                  .map_or(false, |d| d.has_styles()) {
+                        unsafe { parent.set_dirty_descendants(); }
+                    }
                 }
                 f(thread_local, kid);
             }
         }
     }
 
     /// Ensures the existence of the ElementData, and returns it. This can't live
     /// on TNode because of the trait-based separation between Servo's script
@@ -541,30 +558,44 @@ pub fn recalc_style_at<E, D>(traversal: 
 
         preprocess_children(traversal,
                             element,
                             propagated_hint,
                             damage_handled,
                             inherited_style_changed);
     }
 
+    // If we are in a restyle for reconstruction, drop the existing restyle
+    // data here, since we won't need to perform a post-traversal to pick up
+    // any change hints.
+    if context.shared.restyle_for_reconstruct {
+        data.clear_restyle();
+    }
+
     if context.shared.animation_only_restyle {
         unsafe { element.unset_animation_only_dirty_descendants(); }
     }
 
-    // Make sure the dirty descendants bit is not set for the root of a
-    // display:none subtree, even if the style didn't change (since, if
-    // the style did change, we'd have already cleared it above).
+    // There are two cases when we want to clear the dity descendants bit
+    // here after styling this element.
+    //
+    // The first case is when this element is the root of a display:none
+    // subtree, even if the style didn't change (since, if the style did
+    // change, we'd have already cleared it above).
     //
     // This keeps the tree in a valid state without requiring the DOM to
     // check display:none on the parent when inserting new children (which
     // can be moderately expensive). Instead, DOM implementations can
     // unconditionally set the dirty descendants bit on any styled parent,
     // and let the traversal sort it out.
-    if data.styles().is_display_none() {
+    //
+    // The second case is when we are in a restyle for reconstruction,
+    // where we won't need to perform a post-traversal to pick up any
+    // change hints.
+    if data.styles().is_display_none() || context.shared.restyle_for_reconstruct {
         unsafe { element.unset_dirty_descendants(); }
     }
 }
 
 fn compute_style<E, D>(_traversal: &D,
                        traversal_data: &mut PerLevelTraversalData,
                        context: &mut StyleContext<E>,
                        element: E,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -11,17 +11,17 @@ use selectors::Element;
 use std::borrow::Cow;
 use std::env;
 use std::fmt::Write;
 use std::ptr;
 use std::sync::{Arc, Mutex};
 use style::arc_ptr_eq;
 use style::context::{QuirksMode, SharedStyleContext, StyleContext};
 use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInfo};
-use style::data::{ElementData, ElementStyles, RestyleData};
+use style::data::{ElementData, ElementStyles, RestyleData, StoredRestyleHint};
 use style::dom::{AnimationOnlyDirtyDescendants, DirtyDescendants};
 use style::dom::{ShowSubtreeData, TElement, TNode};
 use style::error_reporting::StdoutErrorReporter;
 use style::gecko::data::{PerDocumentStyleData, PerDocumentStyleDataImpl};
 use style::gecko::global_style_data::GLOBAL_STYLE_DATA;
 use style::gecko::restyle_damage::GeckoRestyleDamage;
 use style::gecko::selector_parser::{SelectorImpl, PseudoElement};
 use style::gecko::traversal::RecalcStyleOnly;
@@ -75,20 +75,21 @@ use style::properties::parse_one_declara
 use style::restyle_hints::{self, RestyleHint};
 use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
 use style::stylesheets::{CssRule, CssRules, ImportRule, MediaRule, NamespaceRule};
 use style::stylesheets::{Origin, Stylesheet, StyleRule};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
+use style::stylist::Stylist;
 use style::supports::parse_condition_or_declaration;
 use style::thread_state;
 use style::timer::Timer;
-use style::traversal::{ANIMATION_ONLY, UNSTYLED_CHILDREN_ONLY};
+use style::traversal::{ANIMATION_ONLY, FOR_RECONSTRUCT, UNSTYLED_CHILDREN_ONLY};
 use style::traversal::{resolve_style, DomTraversal, TraversalDriver, TraversalFlags};
 use style_traits::ToCss;
 use super::stylesheet_loader::StylesheetLoader;
 
 /*
  * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in
  * the C header in Gecko. In order to catch accidental mismatches, we run rust-bindgen against
  * those signatures as well, giving us a second declaration of all the Servo_* functions in this
@@ -135,60 +136,82 @@ pub extern "C" fn Servo_Shutdown() {
 }
 
 unsafe fn dummy_url_data() -> &'static RefPtr<URLExtraData> {
     RefPtr::from_ptr_ref(DUMMY_URL_DATA.as_ref().unwrap())
 }
 
 fn create_shared_context<'a>(guard: &'a SharedRwLockReadGuard,
                              per_doc_data: &PerDocumentStyleDataImpl,
-                             animation_only: bool) -> SharedStyleContext<'a> {
+                             traversal_flags: TraversalFlags) -> SharedStyleContext<'a> {
     let local_context_data =
         ThreadLocalStyleContextCreationInfo::new(per_doc_data.new_animations_sender.clone());
 
     SharedStyleContext {
         stylist: per_doc_data.stylist.clone(),
         guards: StylesheetGuards::same(guard),
         running_animations: per_doc_data.running_animations.clone(),
         expired_animations: per_doc_data.expired_animations.clone(),
         // FIXME(emilio): Stop boxing here.
         error_reporter: Box::new(StdoutErrorReporter),
         local_context_creation_data: Mutex::new(local_context_data),
         timer: Timer::new(),
         // FIXME Find the real QuirksMode information for this document
         quirks_mode: QuirksMode::NoQuirks,
-        animation_only_restyle: animation_only,
+        animation_only_restyle: traversal_flags.for_animation_only(),
+        restyle_for_reconstruct: traversal_flags.for_reconstruct(),
+    }
+}
+
+fn compute_final_hint_and_propagate_later_siblings(element: GeckoElement, stylist: &Stylist) {
+    if let Some(data) = element.get_data() {
+        let later_siblings = data.borrow_mut().get_restyle_mut()
+                                 .map_or(false, |r| r.compute_final_hint(element, stylist));
+        if later_siblings {
+            if let Some(next) = element.next_sibling_element() {
+                let mut next_data = unsafe { next.ensure_data() }.borrow_mut();
+                next_data.ensure_restyle().hint.insert(&StoredRestyleHint::subtree_and_later_siblings());
+            }
+        }
     }
 }
 
 fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
                     traversal_flags: TraversalFlags) {
     // When new content is inserted in a display:none subtree, we will call into
     // servo to try to style it. Detect that here and bail out.
     if let Some(parent) = element.parent_element() {
         if parent.borrow_data().map_or(true, |d| d.styles().is_display_none()) {
             debug!("{:?} has unstyled parent {:?} - ignoring call to traverse_subtree", element, parent);
             return;
         }
     }
 
     let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow();
 
+    // If we are in a restyle for reconstruction, then we must ensure that any
+    // later sibling restyle hint is propagated onto element's next sibling
+    // element, since we are not restyling the entire document. We must expand
+    // an element snapshot into restyle hints here too, in case that expansion
+    // results in a later siblings hint.
+    if traversal_flags.for_reconstruct() {
+        compute_final_hint_and_propagate_later_siblings(element, &per_doc_data.stylist);
+    }
+
     let token = RecalcStyleOnly::pre_traverse(element, &per_doc_data.stylist, traversal_flags);
     if !token.should_traverse() {
         return;
     }
 
     debug!("Traversing subtree:");
     debug!("{:?}", ShowSubtreeData(element.as_node()));
 
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
-    let shared_style_context = create_shared_context(&guard, &per_doc_data,
-                                                     traversal_flags.for_animation_only());
+    let shared_style_context = create_shared_context(&guard, &per_doc_data, traversal_flags);
 
     let traversal_driver = if global_style_data.style_thread_pool.is_none() {
         TraversalDriver::Sequential
     } else {
         TraversalDriver::Parallel
     };
 
     let traversal = RecalcStyleOnly::new(shared_style_context, traversal_driver);
@@ -202,23 +225,30 @@ fn traverse_subtree(element: GeckoElemen
 }
 
 /// Traverses the subtree rooted at `root` for restyling.  Returns whether a
 /// Gecko post-traversal (to perform lazy frame construction, or consume any
 /// RestyleData, or drop any ElementData) is required.
 #[no_mangle]
 pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
                                         raw_data: RawServoStyleSetBorrowed,
-                                        behavior: structs::TraversalRootBehavior) -> bool {
+                                        root_behavior: structs::TraversalRootBehavior,
+                                        restyle_behavior: structs::TraversalRestyleBehavior)
+                                        -> bool {
+    use self::structs::TraversalRootBehavior as Root;
+    use self::structs::TraversalRestyleBehavior as Restyle;
+
     let element = GeckoElement(root);
     debug!("Servo_TraverseSubtree: {:?}", element);
 
-    let traversal_flags = match behavior {
-        structs::TraversalRootBehavior::UnstyledChildrenOnly => UNSTYLED_CHILDREN_ONLY,
-        _ => TraversalFlags::empty(),
+    let traversal_flags = match (root_behavior, restyle_behavior) {
+        (Root::Normal, Restyle::Normal) => TraversalFlags::empty(),
+        (Root::UnstyledChildrenOnly, Restyle::Normal) => UNSTYLED_CHILDREN_ONLY,
+        (Root::Normal, Restyle::ForReconstruct) => FOR_RECONSTRUCT,
+        _ => panic!("invalid combination of TraversalRootBehavior and TraversalRestyleBehavior"),
     };
 
     if element.has_animation_only_dirty_descendants() ||
        element.has_animation_restyle_hints() {
         traverse_subtree(element, raw_data, traversal_flags | ANIMATION_ONLY);
     }
 
     traverse_subtree(element, raw_data, traversal_flags);
@@ -1511,17 +1541,17 @@ pub extern "C" fn Servo_ResolveStyleLazi
     // up all the computation machinery.
     let mut result = element.mutate_data()
                             .and_then(|d| d.get_styles().map(&finish));
     if result.is_some() {
         return result.unwrap().into_strong();
     }
 
     // We don't have the style ready. Go ahead and compute it as necessary.
-    let shared = create_shared_context(&guard, &mut doc_data.borrow_mut(), false);
+    let shared = create_shared_context(&guard, &mut doc_data.borrow_mut(), TraversalFlags::empty());
     let mut tlc = ThreadLocalStyleContext::new(&shared);
     let mut context = StyleContext {
         shared: &shared,
         thread_local: &mut tlc,
     };
     let ensure = |el: GeckoElement| { unsafe { el.ensure_data(); } };
     let clear = |el: GeckoElement| el.clear_data();
     resolve_style(&mut context, element, &ensure, &clear,