Summary: | REGRESSION (r230574): Interrupted hardware transitions don't behave correctly | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, dino, ews-watchlist, realdawei, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari Technology Preview | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 185668 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Antoine Quint
2018-05-04 06:55:20 PDT
Created attachment 339541 [details]
Patch for EWS, not for review.
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. (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? Created attachment 339582 [details]
Patch
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. Created attachment 340306 [details]
Patch
Committed r231765: <https://trac.webkit.org/changeset/231765> 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 Re-opened since this is blocked by bug 185668 Committed r231840: <https://trac.webkit.org/changeset/231840> Committed r231842: <https://trac.webkit.org/changeset/231842> Committed r231851: <https://trac.webkit.org/changeset/231851> |