Bug 185299 – REGRESSION (r230574): Interrupted hardware transitions don't behave correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

The fix for https://bugs.webkit.org/show_bug.cgi?id=184518 broke interrupted transitions and suspended animations when moving to a different tab.

[Antoine Quint]

<rdar://problem/39630230>

[Antoine Quint]

Created attachment 339541 [details]
Patch for EWS, not for review.

[Simon Fraser (smfr)]

Comment on attachment 339541 [details]
Patch for EWS, not for review.

View in context: https://bugs.webkit.org/attachment.cgi?id=339541&action=review

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3118
> +    if (m_animationsToProcess.contains(animation.m_name))
> +        m_animationsToProcess.remove(animation.m_name);

You have two hash lookups when you could use only one. Either just unconditionally remove, or find and remove using the iterator.

[Antoine Quint]

(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 339541 [details]
> Patch for EWS, not for review.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339541&action=review
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3118
> > +    if (m_animationsToProcess.contains(animation.m_name))
> > +        m_animationsToProcess.remove(animation.m_name);
> 
> You have two hash lookups when you could use only one. Either just
> unconditionally remove, or find and remove using the iterator.

Oh right, remove returns a bool, so I guess it works whether there is a key or not.

Can you think of a way to test this? I think the only way to check whether an animation is going is if we can get at the play state of CA animations. Is this possible?

[Antoine Quint]

Created attachment 339582 [details]
Patch

[Simon Fraser (smfr)]

I think you could test this via a long-running transition with a negative start delay; hover and uncover (or changes styles) and then ref-test that the box is not in its untransformed location.

[Antoine Quint]

Created attachment 340306 [details]
Patch

[Antoine Quint]

Committed r231765: <https://trac.webkit.org/changeset/231765>

[Dawei Fenton (:realdawei)]

The test for this issue (transitions/interrupted-transition-hardware.html is flaky on the bots: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=transitions%2Finterrupted-transition-hardware.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 185668

[Antoine Quint]

Committed r231840: <https://trac.webkit.org/changeset/231840>

[Antoine Quint]

Committed r231842: <https://trac.webkit.org/changeset/231842>

[Antoine Quint]

Committed r231851: <https://trac.webkit.org/changeset/231851>