Auto merge of #17834 - emilio:animation-fc-crash, r=heycam

stylo: Fix the interaction of frame construction restyles with animation restyles.

Fixes bug 1383001.
This commit is contained in:
bors-servo 2017-07-24 01:09:24 -07:00 committed by GitHub
commit 30d6d6024b
5 changed files with 37 additions and 53 deletions

View file

@ -298,22 +298,16 @@ impl ElementData {
self.styles.primary.is_some()
}
/// Returns whether we have any outstanding style invalidation.
pub fn has_invalidations(&self) -> bool {
self.restyle.hint.has_self_invalidations()
}
/// Returns the kind of restyling that we're going to need to do on this
/// element, based of the stored restyle hint.
pub fn restyle_kind(&self,
shared_context: &SharedStyleContext)
-> RestyleKind {
pub fn restyle_kind(
&self,
shared_context: &SharedStyleContext
) -> RestyleKind {
if shared_context.traversal_flags.for_animation_only() {
return self.restyle_kind_for_animation(shared_context);
}
debug_assert!(!self.has_styles() || self.has_invalidations(),
"Should've stopped earlier");
if !self.has_styles() {
return RestyleKind::MatchAndCascade;
}
@ -335,9 +329,10 @@ impl ElementData {
}
/// Returns the kind of restyling for animation-only restyle.
pub fn restyle_kind_for_animation(&self,
shared_context: &SharedStyleContext)
-> RestyleKind {
fn restyle_kind_for_animation(
&self,
shared_context: &SharedStyleContext,
) -> RestyleKind {
debug_assert!(shared_context.traversal_flags.for_animation_only());
debug_assert!(self.has_styles(),
"Unstyled element shouldn't be traversed during \
@ -350,8 +345,8 @@ impl ElementData {
if hint.has_animation_hint() {
return RestyleKind::CascadeWithReplacements(hint & RestyleHint::for_animations());
}
return RestyleKind::CascadeOnly;
return RestyleKind::CascadeOnly;
}
/// Return true if important rules are different.
@ -362,9 +357,11 @@ impl ElementData {
/// the check which properties do they want.
/// If it costs too much, get_properties_overriding_animations() should return a set
/// containing only opacity and transform properties.
pub fn important_rules_are_different(&self,
rules: &StrongRuleNode,
guards: &StylesheetGuards) -> bool {
pub fn important_rules_are_different(
&self,
rules: &StrongRuleNode,
guards: &StylesheetGuards
) -> bool {
debug_assert!(self.has_styles());
let (important_rules, _custom) =
self.styles.primary().rules().get_properties_overriding_animations(&guards);

View file

@ -470,19 +470,12 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
/// Flags this element as having handled already its snapshot.
unsafe fn set_handled_snapshot(&self);
/// Returns whether the element's styles are up-to-date.
fn has_current_styles(&self, data: &ElementData) -> bool {
if self.has_snapshot() && !self.handled_snapshot() {
return false;
}
data.has_styles() && !data.has_invalidations()
}
/// Returns whether the element's styles are up-to-date for |traversal_flags|.
fn has_current_styles_for_traversal(&self,
data: &ElementData,
traversal_flags: TraversalFlags) -> bool {
fn has_current_styles_for_traversal(
&self,
data: &ElementData,
traversal_flags: TraversalFlags,
) -> bool {
if traversal_flags.for_animation_only() {
// In animation-only restyle we never touch snapshots and don't
// care about them. But we can't assert '!self.handled_snapshot()'
@ -499,7 +492,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
return false;
}
data.has_styles() && !data.has_invalidations()
data.has_styles() && !data.restyle.hint.has_non_animation_hint()
}
/// Flags an element and its ancestors with a given `DescendantsBit`.

View file

@ -52,17 +52,7 @@ where
{
let parent_el = element.inheritance_parent();
let parent_data = parent_el.as_ref().and_then(|e| e.borrow_data());
let parent_style = parent_data.as_ref().map(|d| {
// Sometimes Gecko eagerly styles things without processing
// pending restyles first. In general we'd like to avoid this,
// but there can be good reasons (for example, needing to
// construct a frame for some small piece of newly-added
// content in order to do something specific with that frame,
// but not wanting to flush all of layout).
debug_assert!(cfg!(feature = "gecko") ||
parent_el.unwrap().has_current_styles(d));
d.styles.primary()
});
let parent_style = parent_data.as_ref().map(|d| d.styles.primary());
let mut layout_parent_el = parent_el.clone();
let layout_parent_data;

View file

@ -374,9 +374,8 @@ pub trait DomTraversal<E: TElement> : Sync {
parent: E,
parent_data: &ElementData,
) -> bool {
// See the comment on `cascade_node` for why we allow this on Gecko.
debug_assert!(cfg!(feature = "gecko") ||
parent.has_current_styles(parent_data));
parent.has_current_styles_for_traversal(parent_data, context.shared.traversal_flags));
// If the parent computed display:none, we don't style the subtree.
if parent_data.styles.is_display_none() {

View file

@ -275,21 +275,26 @@ pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
(Root::Normal, Restyle::ForThrottledAnimationFlush)
=> TraversalFlags::empty(),
(Root::UnstyledChildrenOnly, Restyle::Normal) |
(Root::UnstyledChildrenOnly, Restyle::ForNewlyBoundElement) |
(Root::UnstyledChildrenOnly, Restyle::ForThrottledAnimationFlush)
(Root::UnstyledChildrenOnly, Restyle::ForNewlyBoundElement)
=> UNSTYLED_CHILDREN_ONLY,
(Root::Normal, Restyle::ForCSSRuleChanges) => FOR_CSS_RULE_CHANGES,
(Root::Normal, Restyle::ForReconstruct) => FOR_RECONSTRUCT,
_ => panic!("invalid combination of TraversalRootBehavior and TraversalRestyleBehavior"),
};
let needs_animation_only_restyle = element.has_animation_only_dirty_descendants() ||
element.has_animation_restyle_hints();
if needs_animation_only_restyle {
traverse_subtree(element,
raw_data,
traversal_flags | ANIMATION_ONLY,
unsafe { &*snapshots });
// It makes no sense to do an animation restyle when we're restyling
// newly-inserted content.
if !traversal_flags.contains(UNSTYLED_CHILDREN_ONLY) {
let needs_animation_only_restyle =
element.has_animation_only_dirty_descendants() ||
element.has_animation_restyle_hints();
if needs_animation_only_restyle {
traverse_subtree(element,
raw_data,
traversal_flags | ANIMATION_ONLY,
unsafe { &*snapshots });
}
}
if restyle_behavior == Restyle::ForThrottledAnimationFlush {
@ -2814,7 +2819,7 @@ pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
TraversalFlags::empty()
};
debug_assert!(element.has_current_styles_for_traversal(&*data, flags),
"Resolving style on element without current styles");
"Resolving style on {:?} without current styles: {:?}", element, data);
data.styles.primary().clone().into()
}