Skip to content

Commit

Permalink
Cleanup code that was used for verifying styling results for throttle…
Browse files Browse the repository at this point in the history
…d animation flush in post traversal.

Now that we do process normal traversal even in the case of throttled animation
flush so that we don't need to do special handling for the case.

Note about the comment in has_current_styles():
the remaining animation hints is not caused by either this patch or the
previous patch in this patch series, it's been there in the first place, but
it should be fixed somehow later. See bug 1389675.

MozReview-Commit-ID: JojHufxNCiS
  • Loading branch information
Hiroyuki Ikezoe authored and bholley committed Aug 12, 2017
1 parent c0de5eb commit 015a249
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 27 deletions.
22 changes: 22 additions & 0 deletions components/style/dom.rs
Expand Up @@ -462,6 +462,28 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
data.has_styles() && !data.restyle.hint.has_non_animation_invalidations()
}

/// Returns whether the element's styles are up-to-date after traversal
/// (i.e. in post traversal).
fn has_current_styles(&self, data: &ElementData) -> bool {
if self.has_snapshot() && !self.handled_snapshot() {
return false;
}

data.has_styles() &&
// TODO(hiro): When an animating element moved into subtree of
// contenteditable element, there remains animation restyle hints in
// post traversal. It's generally harmless since the hints will be
// processed in a next styling but ideally it should be processed soon.
//
// Without this, we get failures in:
// layout/style/crashtests/1383319.html
// layout/style/crashtests/1383001.html
//
// https://bugzilla.mozilla.org/show_bug.cgi?id=1389675 tracks fixing
// this.
!data.restyle.hint.has_non_animation_invalidations()
}

/// Flag that this element has a descendant for style processing.
///
/// Only safe to call with exclusive access to the element.
Expand Down
30 changes: 3 additions & 27 deletions ports/geckolib/glue.rs
Expand Up @@ -2775,10 +2775,8 @@ pub extern "C" fn Servo_NoteExplicitHints(element: RawGeckoElementBorrowed,

#[no_mangle]
pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,
raw_flags: ServoTraversalFlags,
was_restyled: *mut bool) -> nsChangeHint
{
let flags = TraversalFlags::from_bits_truncate(raw_flags);
let mut was_restyled = unsafe { was_restyled.as_mut().unwrap() };
let element = GeckoElement(element);

Expand All @@ -2787,25 +2785,7 @@ pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,
*was_restyled = data.restyle.is_restyle();

let damage = data.restyle.damage;
if flags.for_animation_only() {
if !*was_restyled {
// Don't touch elements if the element was not restyled
// in throttled animation flush.
debug!("Skip post traversal for throttled animation flush {:?} restyle={:?}",
element, data.restyle);
return nsChangeHint(0);
}
// In the case where we call this function for post traversal for
// flusing throttled animations (i.e. without normal restyle
// traversal), we need to preserve restyle hints for normal restyle
// traversal. Restyle hints for animations have been already
// removed during animation-only traversal.
debug_assert!(!data.restyle.hint.has_animation_hint(),
"Animation restyle hints should have been already removed");
data.clear_restyle_flags_and_damage();
} else {
data.clear_restyle_state();
}
data.clear_restyle_state();
damage
}
None => {
Expand All @@ -2821,21 +2801,17 @@ pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,

#[no_mangle]
pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
_raw_data: RawServoStyleSetBorrowed,
raw_flags: ServoTraversalFlags)
_raw_data: RawServoStyleSetBorrowed)
-> ServoStyleContextStrong
{
let flags = TraversalFlags::from_bits_truncate(raw_flags);
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.
debug_assert!(element.has_current_styles_for_traversal(&*data, flags),
debug_assert!(element.has_current_styles(&*data),
"Resolving style on {:?} without current styles: {:?}", element, data);
data.styles.primary().clone().into()
}
Expand Down

0 comments on commit 015a249

Please sign in to comment.