Skip to content

Commit

Permalink
Bug 1443427 - Don't flush throttled animations in Animation::FlushSty…
Browse files Browse the repository at this point in the history
…le(). r=birtles

Animation::FlushStyle() gets called only for CSS animations/transitions'
playState changes in JS or ready Promise for CSS animations.  In either case
throttled animation state, which is, to be precise, transformed position or
opacity value on the compositor, doesn't affect those results.

The first test case for CSS animations and the first test case for CSS
transitions in this patch fail without this fix.

MozReview-Commit-ID: EVym4qputL4
  • Loading branch information
Hiroyuki Ikezoe committed Apr 11, 2018
1 parent 3f9e857 commit 4802e3f
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 10 deletions.
5 changes: 3 additions & 2 deletions dom/animation/Animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1418,11 +1418,12 @@ Animation::UpdateEffect()
}

void
Animation::FlushStyle() const
Animation::FlushUnanimatedStyle() const
{
nsIDocument* doc = GetRenderedDocument();
if (doc) {
doc->FlushPendingNotifications(FlushType::Style);
doc->FlushPendingNotifications(
ChangesToFlush(FlushType::Style, false /* flush animations */));
}
}

Expand Down
6 changes: 5 additions & 1 deletion dom/animation/Animation.h
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,11 @@ class Animation
void UpdateFinishedState(SeekFlag aSeekFlag,
SyncNotifyFlag aSyncNotifyFlag);
void UpdateEffect();
void FlushStyle() const;
/**
* Flush all pending styles other than throttled animation styles (e.g.
* animations running on the compositor).
*/
void FlushUnanimatedStyle() const;
void PostUpdate();
void ResetFinishedPromise();
void MaybeResolveFinishedPromise();
Expand Down
101 changes: 101 additions & 0 deletions dom/animation/test/mozilla/file_restyles.html
Original file line number Diff line number Diff line change
Expand Up @@ -1807,6 +1807,107 @@

await ensureElementRemoval(div);
});

add_task(
async function no_restyling_for_throttled_animation_on_querying_play_state() {
var div = addDiv(null, { style: 'animation: opacity 100s' });
var animation = div.getAnimations()[0];
var sibling = addDiv(null);

await animation.ready;
ok(SpecialPowers.wrap(animation).isRunningOnCompositor);

var markers = observeAnimSyncStyling(() => {
sibling.style.opacity = '0.5';
is(animation.playState, 'running',
'Animation.playState should be running');
});
is(markers.length, 0,
'Animation.playState should not flush throttled animation in the ' +
'case where there are only style changes that don\'t affect the ' +
'throttled animation');

await ensureElementRemoval(div);
await ensureElementRemoval(sibling);
}
);

add_task(
async function restyling_for_throttled_animation_on_querying_play_state() {
var div = addDiv(null, { style: 'animation: opacity 100s' });
var animation = div.getAnimations()[0];

await animation.ready;
ok(SpecialPowers.wrap(animation).isRunningOnCompositor);

var markers = observeAnimSyncStyling(() => {
div.style.animationPlayState = 'paused';
is(animation.playState, 'paused',
'Animation.playState should be reflected by pending style');
});

is(markers.length, 1,
'Animation.playState should flush throttled animation style that ' +
'affects the throttled animation');

await ensureElementRemoval(div);
}
);

add_task(
async function no_restyling_for_throttled_transition_on_querying_play_state() {
var div = addDiv(null, { style: 'transition: opacity 100s; opacity: 0' });
var sibling = addDiv(null);

getComputedStyle(div).opacity;
div.style.opacity = 1;

var transition = div.getAnimations()[0];

await transition.ready;
ok(SpecialPowers.wrap(transition).isRunningOnCompositor);

var markers = observeAnimSyncStyling(() => {
sibling.style.opacity = '0.5';
is(transition.playState, 'running',
'Animation.playState should be running');
});

is(markers.length, 0,
'Animation.playState should not flush throttled transition in the ' +
'case where there are only style changes that don\'t affect the ' +
'throttled animation');

await ensureElementRemoval(div);
await ensureElementRemoval(sibling);
}
);

add_task(
async function restyling_for_throttled_transition_on_querying_play_state() {
var div = addDiv(null, { style: 'transition: opacity 100s; opacity: 0' });
getComputedStyle(div).opacity;
div.style.opacity = '1';

var transition = div.getAnimations()[0];

await transition.ready;
ok(SpecialPowers.wrap(transition).isRunningOnCompositor);

var markers = observeAnimSyncStyling(() => {
div.style.transitionProperty = 'none';
is(transition.playState, 'idle',
'Animation.playState should be reflected by pending style change ' +
'which cancel the transition');
});

is(markers.length, 1,
'Animation.playState should flush throttled transition style that ' +
'affects the throttled animation');

await ensureElementRemoval(div);
}
);
});

</script>
Expand Down
8 changes: 4 additions & 4 deletions layout/style/nsAnimationManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ CSSAnimation::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
mozilla::dom::Promise*
CSSAnimation::GetReady(ErrorResult& aRv)
{
FlushStyle();
FlushUnanimatedStyle();
return Animation::GetReady(aRv);
}

Expand All @@ -76,7 +76,7 @@ CSSAnimation::PlayStateFromJS() const
{
// Flush style to ensure that any properties controlling animation state
// (e.g. animation-play-state) are fully updated.
FlushStyle();
FlushUnanimatedStyle();
return Animation::PlayStateFromJS();
}

Expand All @@ -85,7 +85,7 @@ CSSAnimation::PendingFromJS() const
{
// Flush style since, for example, if the animation-play-state was just
// changed its possible we should now be pending.
FlushStyle();
FlushUnanimatedStyle();
return Animation::PendingFromJS();
}

Expand All @@ -94,7 +94,7 @@ CSSAnimation::PlayFromJS(ErrorResult& aRv)
{
// Note that flushing style below might trigger calls to
// PlayFromStyle()/PauseFromStyle() on this object.
FlushStyle();
FlushUnanimatedStyle();
Animation::PlayFromJS(aRv);
}

Expand Down
6 changes: 3 additions & 3 deletions layout/style/nsTransitionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ CSSTransition::GetTransitionProperty(nsString& aRetVal) const
AnimationPlayState
CSSTransition::PlayStateFromJS() const
{
FlushStyle();
FlushUnanimatedStyle();
return Animation::PlayStateFromJS();
}

Expand All @@ -158,15 +158,15 @@ CSSTransition::PendingFromJS() const
// that the transition will be canceled, we need to report false here.
// Hence we need to flush, but only when we're pending.
if (Pending()) {
FlushStyle();
FlushUnanimatedStyle();
}
return Animation::PendingFromJS();
}

void
CSSTransition::PlayFromJS(ErrorResult& aRv)
{
FlushStyle();
FlushUnanimatedStyle();
Animation::PlayFromJS(aRv);
}

Expand Down

0 comments on commit 4802e3f

Please sign in to comment.