Auto merge of #17944 - hiikezoe:display-property-animation, r=emilio

Display property animation for SMIL

<!-- Please describe your changes on the following line: -->
https://bugzilla.mozilla.org/show_bug.cgi?id=1385089

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17944)
<!-- Reviewable:end -->

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,8 +18,8 @@ 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;
+use data::{ElementData, RestyleData};
 use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthesizer};
 use element_state::{ElementState, DocumentState, NS_DOCUMENT_STATE_WINDOW_INACTIVE};
@@ -63,7 +63,9 @@ use gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT;
 use gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel;
 use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE;
 use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS;
+use gecko_bindings::structs::nsChangeHint;
 use gecko_bindings::structs::nsIDocument_DocumentTheme as DocumentTheme;
+use gecko_bindings::structs::nsRestyleHint;
 use gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI};
 use logical_geometry::WritingMode;
 use media_queries::Device;
@@ -670,6 +672,64 @@ impl<'le> GeckoElement<'le> {
     pub fn owner_document_quirks_mode(&self) -> QuirksMode {
         self.as_node().owner_doc().mCompatMode.into()
     }
+
+    /// Only safe to call on the main thread, with exclusive access to the element and
+    /// its ancestors.
+    /// This function is also called after display property changed for SMIL animation.
+    ///
+    /// Also this function schedules style flush.
+    unsafe fn maybe_restyle<'a>(&self,
+                                data: &'a mut ElementData,
+                                animation_only: bool) -> Option<&'a mut RestyleData> {
+        use dom::{AnimationOnlyDirtyDescendants, DirtyDescendants};
+
+        // Don't generate a useless RestyleData if the element hasn't been styled.
+        if !data.has_styles() {
+            return None;
+        }
+
+        // Propagate the bit up the chain.
+        if let Some(p) = self.traversal_parent() {
+            if animation_only {
+                p.note_descendants::<AnimationOnlyDirtyDescendants>();
+            } else {
+                p.note_descendants::<DirtyDescendants>();
+            }
+        };
+
+        bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0);
+
+        // Ensure and return the RestyleData.
+        Some(&mut data.restyle)
+    }
+
+    /// Set restyle and change hints to the element data.
+    pub fn note_explicit_hints(&self,
+                               restyle_hint: nsRestyleHint,
+                               change_hint: nsChangeHint) {
+        use gecko::restyle_damage::GeckoRestyleDamage;
+        use invalidation::element::restyle_hints::RestyleHint;
+
+        let damage = GeckoRestyleDamage::new(change_hint);
+        debug!("note_explicit_hints: {:?}, restyle_hint={:?}, change_hint={:?}",
+               self, restyle_hint, change_hint);
+
+        let restyle_hint: RestyleHint = restyle_hint.into();
+        debug_assert!(!(restyle_hint.has_animation_hint() &&
+                        restyle_hint.has_non_animation_hint()),
+                      "Animation restyle hints should not appear with non-animation restyle hints");
+
+        let mut maybe_data = self.mutate_data();
+        let maybe_restyle_data = maybe_data.as_mut().and_then(|d| unsafe {
+            self.maybe_restyle(d, restyle_hint.has_animation_hint())
+        });
+        if let Some(restyle_data) = maybe_restyle_data {
+            restyle_data.hint.insert(restyle_hint.into());
+            restyle_data.damage |= damage;
+        } else {
+            debug!("(Element not styled, discarding hints)");
+        }
+    }
 }
 
 /// Converts flags from the layout used by rust-selectors to the layout used
@@ -1107,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(
-            self.process_animations(
+            context,
-                context,
+            &mut data.styles.primary,
-                &mut data.styles.primary,
+            &mut new_styles.primary.as_mut().unwrap(),
-                &mut new_styles.primary.as_mut().unwrap(),
+            data.restyle.hint,
-                important_rules_changed,
+            important_rules_changed,
-            );
+        );
-        }
 
         // First of all, update the styles.
         let old_styles = mem::replace(&mut data.styles, new_styles);

ports/geckolib/glue.rs

@@ -2,7 +2,6 @@
  * 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/. */
 
-use atomic_refcell::AtomicRefMut;
 use cssparser::{Parser, ParserInput};
 use cssparser::ToCss as ParserToCss;
 use env_logger::LogBuilder;
@@ -14,8 +13,7 @@ use std::fmt::Write;
 use std::ptr;
 use style::context::{CascadeInputs, QuirksMode, SharedStyleContext, StyleContext};
 use style::context::ThreadLocalStyleContext;
-use style::data::{ElementData, ElementStyles, RestyleData};
+use style::data::ElementStyles;
-use style::dom::{AnimationOnlyDirtyDescendants, DirtyDescendants};
 use style::dom::{ShowSubtreeData, TElement, TNode};
 use style::element_state::ElementState;
 use style::error_reporting::{NullReporter, ParseErrorReporter};
@@ -26,7 +24,6 @@ use style::gecko::restyle_damage::GeckoRestyleDamage;
 use style::gecko::selector_parser::PseudoElement;
 use style::gecko::traversal::RecalcStyleOnly;
 use style::gecko::wrapper::GeckoElement;
-use style::gecko_bindings::bindings;
 use style::gecko_bindings::bindings::{RawGeckoElementBorrowed, RawGeckoElementBorrowedOrNull};
 use style::gecko_bindings::bindings::{RawGeckoKeyframeListBorrowed, RawGeckoKeyframeListBorrowedMut};
 use style::gecko_bindings::bindings::{RawServoDeclarationBlockBorrowed, RawServoDeclarationBlockStrong};
@@ -91,7 +88,7 @@ use style::gecko_bindings::sugar::ownership::{FFIArcHelpers, HasFFI, HasArcFFI,
 use style::gecko_bindings::sugar::ownership::{HasSimpleFFI, Strong};
 use style::gecko_bindings::sugar::refptr::RefPtr;
 use style::gecko_properties::style_structs;
-use style::invalidation::element::restyle_hints::{self, RestyleHint};
+use style::invalidation::element::restyle_hints;
 use style::media_queries::{MediaList, parse_media_query_list};
 use style::parallel;
 use style::parser::{ParserContext, self};
@@ -2723,57 +2720,11 @@ pub extern "C" fn Servo_CSSSupports(cond: *const nsACString) -> bool {
     }
 }
 
-/// Only safe to call on the main thread, with exclusive access to the element and
-/// its ancestors.
-unsafe fn maybe_restyle<'a>(data: &'a mut AtomicRefMut<ElementData>,
-                            element: GeckoElement,
-                            animation_only: bool)
-    -> Option<&'a mut RestyleData>
-{
-    // Don't generate a useless RestyleData if the element hasn't been styled.
-    if !data.has_styles() {
-        return None;
-    }
-
-    // Propagate the bit up the chain.
-    if let Some(p) = element.traversal_parent() {
-        if animation_only {
-            p.note_descendants::<AnimationOnlyDirtyDescendants>();
-        } else {
-            p.note_descendants::<DirtyDescendants>();
-        }
-    };
-
-    bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(element.0);
-
-    // Ensure and return the RestyleData.
-    Some(&mut data.restyle)
-}
-
 #[no_mangle]
 pub extern "C" fn Servo_NoteExplicitHints(element: RawGeckoElementBorrowed,
                                           restyle_hint: nsRestyleHint,
                                           change_hint: nsChangeHint) {
-    let element = GeckoElement(element);
+    GeckoElement(element).note_explicit_hints(restyle_hint, change_hint);
-    let damage = GeckoRestyleDamage::new(change_hint);
-    debug!("Servo_NoteExplicitHints: {:?}, restyle_hint={:?}, change_hint={:?}",
-           element, restyle_hint, change_hint);
-
-    let restyle_hint: RestyleHint = restyle_hint.into();
-    debug_assert!(!(restyle_hint.has_animation_hint() &&
-                    restyle_hint.has_non_animation_hint()),
-                  "Animation restyle hints should not appear with non-animation restyle hints");
-
-    let mut maybe_data = element.mutate_data();
-    let maybe_restyle_data = maybe_data.as_mut().and_then(|d| unsafe {
-        maybe_restyle(d, element, restyle_hint.has_animation_hint())
-    });
-    if let Some(restyle_data) = maybe_restyle_data {
-        restyle_data.hint.insert(restyle_hint.into());
-        restyle_data.damage |= damage;
-    } else {
-        debug!("(Element not styled, discarding hints)");
-    }
 }
 
 #[no_mangle]

ports/geckolib/lib.rs

@@ -4,7 +4,6 @@
 
 #![deny(warnings)]
 
-extern crate atomic_refcell;
 extern crate cssparser;
 extern crate env_logger;
 extern crate libc;