Set restyle subtree restyle hint if the element animates display style from 'none' to other.

When display style is changed from 'none' to other in animation-only restyle
we need to resolve descendant elements' style that were in the display:none
subtree.

Three possible ways to resolve the descendant elements' style;

1) Traversing unstyled elements in animation-only restyle
   We can't simply traverse unstyled elements in the animation-only restyle
   since when we decided to traverse the unstyled elements we don't know yet
   the elements will be initially styled or are in display:none subtree. It
   will result that the new elements are styled in animation-only restyle,
   it's undesirable.

2) Creating a SequentialTask and resolve the descendants' style with
   ServoStyleSet::StyleNewSubtree()
   We can't resolve the descendants' styles with ServoStyleSet::StyleNewSubtree()
   in SequentialTask since at the moment we are still in servo traversal (i.e.
   sInServoTraversal is true). That means AutoSetInServoTraversal fails
   in PrepareAndTraverseSubtree().

3) Creating a SequentialTask and set restyle subtree hint and defer descendants'
   restyle in a subsequent normal traversal
   Note that, when we process throttled animations flush, we don't process
   normal traversal so the descendants will not be traversed until normal
   restyle happens but it will not be a big problem since it's really rare
   that user clicks display animation element just at the right moment when
   display property changes from none to other.  Also, if it will be really
   a problem, we should process *only* transform animations on the compositor,
   it's ideally right thing to do. Display property never runs on the
   compositor.

This patch takes the third approach.

components/style/context.rs

@@ -409,7 +409,7 @@ impl TraversalStatistics {
 #[cfg(feature = "gecko")]
 bitflags! {
     /// Represents which tasks are performed in a SequentialTask of
-    /// UpdateAnimations.
+    /// UpdateAnimations which is a result of normal restyle.
     pub flags UpdateAnimationsTasks: u8 {
         /// Update CSS Animations.
         const CSS_ANIMATIONS = structs::UpdateAnimationsTasks_CSSAnimations,
@@ -422,6 +422,18 @@ bitflags! {
     }
 }
 
+#[cfg(feature = "gecko")]
+bitflags! {
+    /// Represents which tasks are performed in a SequentialTask as a result of
+    /// animation-only restyle.
+    pub flags PostAnimationTasks: u8 {
+        /// Display property was changed from none in animation-only restyle so
+        /// that we need to resolve styles for descendants in a subsequent
+        /// normal restyle.
+        const DISPLAY_CHANGED_FROM_NONE_FOR_SMIL = 0x01,
+    }
+}
+
 
 /// A task to be run in sequential mode on the parent (non-worker) thread. This
 /// is used by the style system to queue up work which is not safe to do during
@@ -443,6 +455,17 @@ pub enum SequentialTask<E: TElement> {
         /// The tasks which are performed in this SequentialTask.
         tasks: UpdateAnimationsTasks
     },
+
+    /// Performs one of a number of possible tasks as a result of animation-only restyle.
+    /// Currently we do only process for resolving descendant elements that were display:none
+    /// subtree for SMIL animation.
+    #[cfg(feature = "gecko")]
+    PostAnimation {
+        /// The target element.
+        el: SendElement<E>,
+        /// The tasks which are performed in this SequentialTask.
+        tasks: PostAnimationTasks
+    },
 }
 
 impl<E: TElement> SequentialTask<E> {
@@ -456,6 +479,10 @@ impl<E: TElement> SequentialTask<E> {
             UpdateAnimations { el, before_change_style, tasks } => {
                 unsafe { el.update_animations(before_change_style, tasks) };
             }
+            #[cfg(feature = "gecko")]
+            PostAnimation { el, tasks } => {
+                unsafe { el.process_post_animation(tasks) };
+            }
         }
     }
 
@@ -472,6 +499,17 @@ impl<E: TElement> SequentialTask<E> {
             tasks: tasks,
         }
     }
+
+    /// Creates a task to do post-process for a given element as a result of
+    /// animation-only restyle.
+    #[cfg(feature = "gecko")]
+    pub fn process_post_animation(el: E, tasks: PostAnimationTasks) -> Self {
+        use self::SequentialTask::*;
+        PostAnimation {
+            el: unsafe { SendElement::new(el) },
+            tasks: tasks,
+        }
+    }
 }
 
 /// Map from Elements to ElementSelectorFlags. Used to defer applying selector

components/style/dom.rs

@@ -10,6 +10,7 @@
 use {Atom, Namespace, LocalName};
 use applicable_declarations::ApplicableDeclarationBlock;
 use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
+#[cfg(feature = "gecko")] use context::PostAnimationTasks;
 #[cfg(feature = "gecko")] use context::UpdateAnimationsTasks;
 use data::ElementData;
 use element_state::ElementState;
@@ -625,6 +626,10 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
                          before_change_style: Option<Arc<ComputedValues>>,
                          tasks: UpdateAnimationsTasks);
 
+    /// Creates a task to process post animation on a given element.
+    #[cfg(feature = "gecko")]
+    fn process_post_animation(&self, tasks: PostAnimationTasks);
+
     /// Returns true if the element has relevant animations. Relevant
     /// animations are those animations that are affecting the element's style
     /// or are scheduled to do so in the future.

components/style/gecko/wrapper.rs

@@ -18,7 +18,7 @@ use CaseSensitivityExt;
 use app_units::Au;
 use applicable_declarations::ApplicableDeclarationBlock;
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
-use context::{QuirksMode, SharedStyleContext, UpdateAnimationsTasks};
+use context::{QuirksMode, SharedStyleContext, PostAnimationTasks, UpdateAnimationsTasks};
 use data::{ElementData, RestyleData};
 use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthesizer};
@@ -1167,6 +1167,31 @@ impl<'le> TElement for GeckoElement<'le> {
         self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations)
     }
 
+    /// Process various tasks that are a result of animation-only restyle.
+    fn process_post_animation(&self,
+                              tasks: PostAnimationTasks) {
+        use context::DISPLAY_CHANGED_FROM_NONE_FOR_SMIL;
+        use gecko_bindings::structs::nsChangeHint_nsChangeHint_Empty;
+        use gecko_bindings::structs::nsRestyleHint_eRestyle_Subtree;
+
+        debug_assert!(!tasks.is_empty(), "Should be involved a task");
+
+        // If display style was changed from none to other, we need to resolve
+        // the descendants in the display:none subtree. Instead of resolving
+        // those styles in animation-only restyle, we defer it to a subsequent
+        // normal restyle.
+        if tasks.intersects(DISPLAY_CHANGED_FROM_NONE_FOR_SMIL) {
+            debug_assert!(self.implemented_pseudo_element()
+                              .map_or(true, |p| !p.is_before_or_after()),
+                          "display property animation shouldn't run on pseudo elements \
+                           since it's only for SMIL");
+            self.note_explicit_hints(nsRestyleHint_eRestyle_Subtree,
+                                     nsChangeHint_nsChangeHint_Empty);
+        }
+    }
+
+    /// Update various animation-related state on a given (pseudo-)element as
+    /// results of normal restyle.
     fn update_animations(&self,
                          before_change_style: Option<Arc<ComputedValues>>,
                          tasks: UpdateAnimationsTasks) {

components/style/matching.rs

@@ -185,15 +185,56 @@ trait PrivateMatchMethods: TElement {
         })
     }
 
+    /// Create a SequentialTask for resolving descendants in a SMIL display property
+    /// animation if the display property changed from none.
+    #[cfg(feature = "gecko")]
+    fn handle_display_change_for_smil_if_needed(&self,
+                                                context: &mut StyleContext<Self>,
+                                                old_values: Option<&ComputedValues>,
+                                                new_values: &ComputedValues,
+                                                restyle_hints: RestyleHint) {
+        use context::DISPLAY_CHANGED_FROM_NONE_FOR_SMIL;
+
+        let display_changed_from_none = old_values.as_ref().map_or(false, |old| {
+            let old_display_style = old.get_box().clone_display();
+            let new_display_style = new_values.get_box().clone_display();
+            old_display_style == display::T::none &&
+            new_display_style != display::T::none
+        });
+
+        if display_changed_from_none {
+          // When display value is changed from none to other, we need
+          // to traverse descendant elements in a subsequent normal
+          // traversal (we can't traverse them in this animation-only
+          // restyle since we have no way to know whether the decendants
+          // need to be traversed at the beginning of the animation-only
+          // restyle)
+          debug_assert!(restyle_hints.intersects(RESTYLE_SMIL),
+                        "Display animation should only happen for SMIL");
+          let task = ::context::SequentialTask::process_post_animation(*self,
+                                                                       DISPLAY_CHANGED_FROM_NONE_FOR_SMIL);
+          context.thread_local.tasks.push(task);
+        }
+    }
+
     #[cfg(feature = "gecko")]
     fn process_animations(&self,
                           context: &mut StyleContext<Self>,
                           old_values: &mut Option<Arc<ComputedValues>>,
                           new_values: &mut Arc<ComputedValues>,
+                          restyle_hint: RestyleHint,
                           important_rules_changed: bool) {
         use context::{CASCADE_RESULTS, CSS_ANIMATIONS, CSS_TRANSITIONS, EFFECT_PROPERTIES};
         use context::UpdateAnimationsTasks;
 
+        if context.shared.traversal_flags.for_animation_only() {
+            self.handle_display_change_for_smil_if_needed(context,
+                                                          old_values.as_ref().map(|v| &**v),
+                                                          new_values,
+                                                          restyle_hint);
+            return;
+        }
+
         // Bug 868975: These steps should examine and update the visited styles
         // in addition to the unvisited styles.
 
@@ -258,6 +299,7 @@ trait PrivateMatchMethods: TElement {
                           context: &mut StyleContext<Self>,
                           old_values: &mut Option<Arc<ComputedValues>>,
                           new_values: &mut Arc<ComputedValues>,
+                          _restyle_hint: RestyleHint,
                           _important_rules_changed: bool) {
         use animation;
         use dom::TNode;
@@ -450,14 +492,13 @@ pub trait MatchMethods : TElement {
 
         debug_assert!(new_styles.primary.is_some(), "How did that happen?");
 
-        if !context.shared.traversal_flags.for_animation_only() {
-            self.process_animations(
-                context,
-                &mut data.styles.primary,
-                &mut new_styles.primary.as_mut().unwrap(),
-                important_rules_changed,
-            );
-        }
+        self.process_animations(
+            context,
+            &mut data.styles.primary,
+            &mut new_styles.primary.as_mut().unwrap(),
+            data.restyle.hint,
+            important_rules_changed,
+        );
 
         // First of all, update the styles.
         let old_styles = mem::replace(&mut data.styles, new_styles);