Auto merge of #17400 - hiikezoe:dont-do-normal-traversal-for-animation-only-restyle-element, r=heycam

Don't run normal traversal for animation only restyle element

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

---
<!-- 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/17400)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-06-19 20:19:16 -07:00 committed by GitHub
commit 546c9db7a7
3 changed files with 28 additions and 13 deletions

View file

@ -239,7 +239,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
let mut any_invalidated = false; let mut any_invalidated = false;
while let Some(sibling) = current { while let Some(sibling) = current {
let mut sibling_data = sibling.get_data().map(|d| d.borrow_mut()); let mut sibling_data = sibling.mutate_data();
let sibling_data = sibling_data.as_mut().map(|d| &mut **d); let sibling_data = sibling_data.as_mut().map(|d| &mut **d);
let mut sibling_invalidator = TreeStyleInvalidator::new( let mut sibling_invalidator = TreeStyleInvalidator::new(
@ -302,7 +302,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
None => continue, None => continue,
}; };
let mut child_data = child.get_data().map(|d| d.borrow_mut()); let mut child_data = child.mutate_data();
let child_data = child_data.as_mut().map(|d| &mut **d); let child_data = child_data.as_mut().map(|d| &mut **d);
let mut child_invalidator = TreeStyleInvalidator::new( let mut child_invalidator = TreeStyleInvalidator::new(

View file

@ -326,14 +326,21 @@ pub trait DomTraversal<E: TElement> : Sync {
// the element if the element has animation only dirty // the element if the element has animation only dirty
// descendants bit, animation-only restyle hint or recascade. // descendants bit, animation-only restyle hint or recascade.
if traversal_flags.for_animation_only() { if traversal_flags.for_animation_only() {
if el.has_animation_only_dirty_descendants() { // Skip elements that have no style data since animation-only
return true; // restyle is not necessary for the elements.
}
let data = match el.borrow_data() { let data = match el.borrow_data() {
Some(d) => d, Some(d) => d,
None => return false, None => return false,
}; };
if !data.has_styles() {
return false;
}
if el.has_animation_only_dirty_descendants() {
return true;
}
return data.restyle.hint.has_animation_hint() || return data.restyle.hint.has_animation_hint() ||
data.restyle.hint.has_recascade_self(); data.restyle.hint.has_recascade_self();
} }
@ -455,7 +462,11 @@ pub trait DomTraversal<E: TElement> : Sync {
let el = kid.as_element(); let el = kid.as_element();
if el.as_ref().and_then(|el| el.borrow_data()) if el.as_ref().and_then(|el| el.borrow_data())
.map_or(false, |d| d.has_styles()) { .map_or(false, |d| d.has_styles()) {
unsafe { parent.set_dirty_descendants(); } if self.shared_context().traversal_flags.for_animation_only() {
unsafe { parent.set_animation_only_dirty_descendants(); }
} else {
unsafe { parent.set_dirty_descendants(); }
}
} }
} }
f(thread_local, kid); f(thread_local, kid);
@ -855,14 +866,14 @@ where
None => continue, None => continue,
}; };
let mut child_data =
unsafe { D::ensure_element_data(&child).borrow_mut() };
// If the child is unstyled, we don't need to set up any restyling. // If the child is unstyled, we don't need to set up any restyling.
if !child_data.has_styles() { if child.borrow_data().map_or(true, |d| !d.has_styles()) {
continue; continue;
} }
let mut child_data =
unsafe { D::ensure_element_data(&child).borrow_mut() };
trace!(" > {:?} -> {:?} + {:?}, pseudo: {:?}", trace!(" > {:?} -> {:?} + {:?}, pseudo: {:?}",
child, child,
child_data.restyle.hint, child_data.restyle.hint,

View file

@ -300,7 +300,9 @@ pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
return false; return false;
} }
element.has_dirty_descendants() || element.borrow_data().unwrap().restyle.contains_restyle_data() element.has_dirty_descendants() ||
element.has_animation_only_dirty_descendants() ||
element.borrow_data().unwrap().restyle.contains_restyle_data()
} }
/// Checks whether the rule tree has crossed its threshold for unused nodes, and /// Checks whether the rule tree has crossed its threshold for unused nodes, and
@ -2801,7 +2803,9 @@ pub extern "C" fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed) {
let root = GeckoElement(root); let root = GeckoElement(root);
fn assert_subtree_is_clean<'le>(el: GeckoElement<'le>) { fn assert_subtree_is_clean<'le>(el: GeckoElement<'le>) {
debug_assert!(!el.has_dirty_descendants() && !el.has_animation_only_dirty_descendants()); debug_assert!(!el.has_dirty_descendants() && !el.has_animation_only_dirty_descendants(),
"{:?} has still dirty bit {:?} or animation-only dirty bit {:?}",
el, el.has_dirty_descendants(), el.has_animation_only_dirty_descendants());
for child in el.as_node().traversal_children() { for child in el.as_node().traversal_children() {
if let Some(child) = child.as_element() { if let Some(child) = child.as_element() {
assert_subtree_is_clean(child); assert_subtree_is_clean(child);