Bug 218011 - REGRESSION (r263729): transform transition doesn't restart
Summary: REGRESSION (r263729): transform transition doesn't restart
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-21 02:06 PDT by Antoine Quint
Modified: 2020-10-21 11:11 PDT (History)
3 users (show)

See Also:


Attachments
Test (879 bytes, text/html)
2020-10-21 02:06 PDT, Antoine Quint
no flags Details
Patch (7.60 KB, patch)
2020-10-21 05:15 PDT, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-10-21 02:06:20 PDT
In the attached test case clicking on the button animates the black rectangle using a transitions. When the transition completes, the transition style is removed. However, clicking the button again will fail to animate again, the transition doesn't start again.
Comment 1 Antoine Quint 2020-10-21 02:06:46 PDT
Created attachment 411966 [details]
Test
Comment 2 Antoine Quint 2020-10-21 02:07:01 PDT
<rdar://problem/70035130>
Comment 3 Antoine Quint 2020-10-21 02:18:46 PDT
The problem is that the second time we attempt to start a transition, under AnimationTimeline::updateCSSTransitionsForStyleableAndProperty(), we should be taking the first code branch but the third clause is failing:

        // 1. If all of the following are true:
        //   - the element does not have a running transition for the property,
        //   - the before-change style is different from and can be interpolated with the after-change style for that property,
        //   - the element does not have a completed transition for the property or the end value of the completed transition is different from the after-change style for the property,
        //   - there is a matching transition-property value, and
        //   - the combined duration is greater than 0s,

Specifically, this is this code:

        propertyInStyleMatchesValueForTransitionInMap(property, afterChangeStyle, styleable.ensureCompletedTransitionsByProperty())
Comment 4 Antoine Quint 2020-10-21 02:26:39 PDT
Under the call to getAnimations(), we take this code branch:

    // A CSS Transition might have completed since the last time animations were updated so we must
    // update the running and completed transitions membership in that case.
    if (is<CSSTransition>(animation) && matchingBackingAnimation && styleable.hasRunningTransitionsForProperty(property) && animation->playState() == WebAnimation::PlayState::Finished) {
        styleable.ensureCompletedTransitionsByProperty().set(property, styleable.ensureRunningTransitionsByProperty().take(property));
        animation = nullptr;
    }

After this, styleable.hasRunningTransitionsForProperty(property) is false and styleable.hasCompletedTransitionsForProperty(property) is true.

Then, upon style recalc due to resetting the transition and transform styles on the target, we take this code branch:

    else if (styleable.hasCompletedTransitionsForProperty(property) && !propertyInStyleMatchesValueForTransitionInMap(property, afterChangeStyle, styleable.ensureCompletedTransitionsByProperty())) {
        // 2. Otherwise, if the element has a completed transition for the property and the end value of the completed transition is different from
        //    the after-change style for the property, then implementations must remove the completed transition from the set of completed transitions.
        styleable.ensureCompletedTransitionsByProperty().remove(property);
    }

After this, styleable.hasRunningTransitionsForProperty(property) and styleable.hasCompletedTransitionsForProperty(property) are both false.

However, the next time we enter AnimationTimeline::updateCSSTransitionsForStyleableAndProperty() upon attempting to restart a transition, styleable.hasRunningTransitionsForProperty(property) is false but styleable.hasCompletedTransitionsForProperty(property) is back to being true. That's the problem, there should not be a completed running transition for this property.
Comment 5 Antoine Quint 2020-10-21 02:56:48 PDT
This is because we set the completed transition again in DocumentTimeline::transitionDidComplete() under DocumentTimelinesController::updateAnimationsAndSendEvents() in a later update. There is a disconnect between the animation's running state as set during the updateAnimationsAndSendEvents() routine and the running/completed transitions recorded on Styleable.

We most likely need to ensure the animation is actually in the finished running state when moving to the completed list.
Comment 6 Antoine Quint 2020-10-21 05:15:37 PDT
Created attachment 411973 [details]
Patch
Comment 7 Antoine Quint 2020-10-21 11:11:00 PDT
Committed r268809: <https://trac.webkit.org/changeset/268809>