Bug 1383589 - Process post traversal if the root element was restyled during flushing throttled animations. r?emilio draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 25 Jul 2017 06:46:26 +0900
changeset 614727 82a3f1669f373210ef444f1b7f94a4ad4ea55356
parent 614726 ef7a9d523ce4442188a9325dcd681ed5f84720ee
child 638934 f85377e22567d4ebbb19300006bd7da38ff1c43a
push id70091
push userhikezoe@mozilla.com
push dateMon, 24 Jul 2017 22:05:18 +0000
reviewersemilio
bugs1383589
milestone56.0a1
Bug 1383589 - Process post traversal if the root element was restyled during flushing throttled animations. r?emilio The test case in this patch freezes without this fix. MozReview-Commit-ID: 6Rb9XmtAmpM
layout/base/ServoRestyleManager.cpp
layout/style/ServoBindingList.h
layout/style/crashtests/1383589-1.html
layout/style/crashtests/crashtests.list
servo/components/style/dom.rs
servo/ports/geckolib/glue.rs
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -865,18 +865,17 @@ ServoRestyleManager::DoProcessPendingRes
     // Recreate style contexts, and queue up change hints (which also handle
     // lazy frame construction).
     {
       AutoRestyleTimelineMarker marker(
         mPresContext->GetDocShell(), forThrottledAnimationFlush);
       DocumentStyleRootIterator iter(doc);
       while (Element* root = iter.GetNextStyleRoot()) {
         ServoRestyleState state(*styleSet, currentChanges);
-        if (!forThrottledAnimationFlush ||
-            root->HasAnimationOnlyDirtyDescendantsForServo()) {
+        if (Servo_NeedsPostTraversal(root, aRestyleBehavior)) {
           anyStyleChanged |=
             ProcessPostTraversal(root, nullptr, state, aRestyleBehavior);
         }
       }
     }
 
     // Process the change hints.
     //
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -492,16 +492,20 @@ SERVO_BINDING_FUNC(Servo_Shutdown, void)
 // Restyle and change hints.
 SERVO_BINDING_FUNC(Servo_NoteExplicitHints, void, RawGeckoElementBorrowed element,
                    nsRestyleHint restyle_hint, nsChangeHint change_hint)
 SERVO_BINDING_FUNC(Servo_TakeChangeHint,
                    nsChangeHint,
                    RawGeckoElementBorrowed element,
                    mozilla::TraversalRestyleBehavior restyle_behavior,
                    bool* was_restyled)
+SERVO_BINDING_FUNC(Servo_NeedsPostTraversal,
+                   bool,
+                   RawGeckoElementBorrowed root,
+                   mozilla::TraversalRestyleBehavior restyle_behavior)
 SERVO_BINDING_FUNC(Servo_ResolveStyle, ServoStyleContextStrong,
                    RawGeckoElementBorrowed element,
                    RawServoStyleSetBorrowed set,
                    mozilla::TraversalRestyleBehavior restyle_behavior)
 SERVO_BINDING_FUNC(Servo_ResolvePseudoStyle, ServoStyleContextStrong,
                    RawGeckoElementBorrowed element,
                    mozilla::CSSPseudoElementType pseudo_type,
                    bool is_probe,
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1383589-1.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<script>
+window.addEventListener('load', () => {
+  document.documentElement.animate([{ transform: 'none' }], 10000);
+  requestAnimationFrame(() => {
+    SpecialPowers.getDOMWindowUtils(window)
+                 .sendMouseEvent("mousemove", 100, 100, 1, 0, 1, 0);
+    requestAnimationFrame(() => {
+      document.documentElement.classList.remove('reftest-wait');
+    });
+  });
+});
+</script>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -180,11 +180,12 @@ load 1377053-1.html
 load 1377256-1.html
 load 1378064-1.html
 load 1378814.html
 load 1380800.html
 load link-transition-before.html
 load 1381682.html
 load 1382672.html
 load 1382710.html
+pref(dom.animations-api.core.enabled,true) load 1383589-1.html
 load 1383001.html
 load 1383001-2.html
 load 1383319.html
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -490,16 +490,30 @@ pub trait TElement : Eq + PartialEq + De
 
         if self.has_snapshot() && !self.handled_snapshot() {
             return false;
         }
 
         data.has_styles() && !data.restyle.hint.has_non_animation_invalidations()
     }
 
+    /// Returns whether the style root element needs to be processed in post
+    /// traversal.
+    fn needs_post_traversal(&self,
+                            traversal_flags: TraversalFlags) -> bool {
+        if traversal_flags.for_animation_only() {
+            return self.has_animation_only_dirty_descendants() ||
+                   self.borrow_data().unwrap().restyle.is_restyle();
+        }
+
+        self.has_dirty_descendants() ||
+        self.has_animation_only_dirty_descendants() ||
+        self.borrow_data().unwrap().restyle.contains_restyle_data()
+    }
+
     /// Flags an element and its ancestors with a given `DescendantsBit`.
     ///
     /// TODO(emilio): We call this conservatively from restyle_element_internal
     /// because we never flag unstyled stuff. A different setup for this may be
     /// a bit cleaner, but it's probably not worth to invest on it right now
     /// unless necessary.
     unsafe fn note_descendants<B: DescendantsBit<Self>>(&self);
 
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -293,36 +293,33 @@ pub extern "C" fn Servo_TraverseSubtree(
             traverse_subtree(element,
                              raw_data,
                              traversal_flags | ANIMATION_ONLY,
                              unsafe { &*snapshots });
         }
     }
 
     if restyle_behavior == Restyle::ForThrottledAnimationFlush {
-        return element.has_animation_only_dirty_descendants() ||
-               element.borrow_data().unwrap().restyle.is_restyle();
+        return element.needs_post_traversal(traversal_flags | ANIMATION_ONLY)
     }
 
     traverse_subtree(element,
                      raw_data,
                      traversal_flags,
                      unsafe { &*snapshots });
 
     if restyle_behavior == Restyle::ForNewlyBoundElement {
         // In this mode, we only ever restyle new elements, so there is no
         // need for a post-traversal, and the borrow_data().unwrap() call below
         // could panic, so we don't bother computing whether a post-traversal
         // is required.
         return false;
     }
 
-    element.has_dirty_descendants() ||
-    element.has_animation_only_dirty_descendants() ||
-    element.borrow_data().unwrap().restyle.contains_restyle_data()
+    element.needs_post_traversal(traversal_flags)
 }
 
 /// Checks whether the rule tree has crossed its threshold for unused nodes, and
 /// if so, frees them.
 #[no_mangle]
 pub extern "C" fn Servo_MaybeGCRuleTree(raw_data: RawServoStyleSetBorrowed) {
     let per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     unsafe {
@@ -2791,38 +2788,51 @@ pub extern "C" fn Servo_TakeChangeHint(e
             GeckoRestyleDamage::empty()
         }
     };
 
     debug!("Servo_TakeChangeHint: {:?}, damage={:?}", element, damage);
     damage.as_change_hint()
 }
 
+fn traversal_flag_for_post_traversal(restyle_behavior: structs::TraversalRestyleBehavior)
+                                     -> TraversalFlags {
+    use self::structs::TraversalRestyleBehavior as Restyle;
+
+    if restyle_behavior == Restyle::ForThrottledAnimationFlush {
+        ANIMATION_ONLY
+    } else {
+        TraversalFlags::empty()
+    }
+}
+
+#[no_mangle]
+pub extern "C" fn Servo_NeedsPostTraversal(root: RawGeckoElementBorrowed,
+                                           restyle_behavior: structs::TraversalRestyleBehavior)
+                                           -> bool {
+    let flags = traversal_flag_for_post_traversal(restyle_behavior);
+    GeckoElement(root).needs_post_traversal(flags)
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
                                      _raw_data: RawServoStyleSetBorrowed,
                                      restyle_behavior: structs::TraversalRestyleBehavior)
                                      -> ServoStyleContextStrong
 {
-    use self::structs::TraversalRestyleBehavior as Restyle;
-
     let element = GeckoElement(element);
     debug!("Servo_ResolveStyle: {:?}", element);
     let data =
         element.borrow_data().expect("Resolving style on unstyled element");
 
     // TODO(emilio): Downgrade to debug assertions when close to release.
     assert!(data.has_styles(), "Resolving style on unstyled element");
     // In the case where we process for throttled animation, there remaings
     // restyle hints other than animation hints.
-    let flags = if restyle_behavior == Restyle::ForThrottledAnimationFlush {
-        ANIMATION_ONLY
-    } else {
-        TraversalFlags::empty()
-    };
+    let flags = traversal_flag_for_post_traversal(restyle_behavior);
     debug_assert!(element.has_current_styles_for_traversal(&*data, flags),
                   "Resolving style on {:?} without current styles: {:?}", element, data);
     data.styles.primary().clone().into()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
                                            pseudo_type: CSSPseudoElementType,