From 9b166c5cda51e4ed6eb6fa740900caadfddf8e0d Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 19 Jun 2017 14:55:35 +0900 Subject: [PATCH 1/4] Use mutate_data() instead of get_data().map() with borrow_mut(). --- components/style/invalidation/element/invalidator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index ed759076e23..2ed012d37d1 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -239,7 +239,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E> let mut any_invalidated = false; 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 mut sibling_invalidator = TreeStyleInvalidator::new( @@ -302,7 +302,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E> 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 mut child_invalidator = TreeStyleInvalidator::new( From 028c0e4a438713758d334ef808a0cba5d4bfd548 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 19 Jun 2017 14:55:57 +0900 Subject: [PATCH 2/4] Don't traverse elements that have no style data in animation-only restyle. Animation-only restyle only works with elements that have already been styled. --- components/style/traversal.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 004c2485afd..707f2c2783c 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -326,14 +326,21 @@ pub trait DomTraversal : Sync { // the element if the element has animation only dirty // descendants bit, animation-only restyle hint or recascade. if traversal_flags.for_animation_only() { - if el.has_animation_only_dirty_descendants() { - return true; - } - + // Skip elements that have no style data since animation-only + // restyle is not necessary for the elements. let data = match el.borrow_data() { Some(d) => d, 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() || data.restyle.hint.has_recascade_self(); } From 5a8e2562d5b9880f3cfa91fb3a375585bd14bd3c Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 19 Jun 2017 14:56:26 +0900 Subject: [PATCH 3/4] Check the child is unstyled without creating element data in preprocess_children. If we check it and skip the child after ensure_element_data() call, the child will have an empty element data, so we will succeed element_data.is_some() check unexpectedly. --- components/style/traversal.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 707f2c2783c..eb51f7d7763 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -862,14 +862,14 @@ where 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 !child_data.has_styles() { + if child.borrow_data().map_or(true, |d| !d.has_styles()) { continue; } + let mut child_data = + unsafe { D::ensure_element_data(&child).borrow_mut() }; + trace!(" > {:?} -> {:?} + {:?}, pseudo: {:?}", child, child_data.restyle.hint, From a3da636f695053d63c529e0998cdbc8e1ff0e915 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 19 Jun 2017 15:01:16 +0900 Subject: [PATCH 4/4] Don't traverse any elements that needed only for animation-only restyles in normal traversal. Before this patch, we were setting the dirty descendants bit in animation-only restyles and it triggered unnecessary traversal for elements that does not need the traversal (i.e no need selector matching). --- components/style/traversal.rs | 6 +++++- ports/geckolib/glue.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/components/style/traversal.rs b/components/style/traversal.rs index eb51f7d7763..a265f49ba2c 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -462,7 +462,11 @@ pub trait DomTraversal : Sync { 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 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); diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index a3ad8344824..b099fd242e8 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -300,7 +300,9 @@ pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed, 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 @@ -2801,7 +2803,9 @@ pub extern "C" fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed) { let root = GeckoElement(root); 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() { if let Some(child) = child.as_element() { assert_subtree_is_clean(child);