Skip to content

[web-animations] remove the notion of an overriding start time when computing timing#44459

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
graouts:eng/web-animations-remove-the-notion-of-an-explicit-start-time-when-computing-timing
Apr 24, 2025
Merged

[web-animations] remove the notion of an overriding start time when computing timing#44459
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
graouts:eng/web-animations-remove-the-notion-of-an-explicit-start-time-when-computing-timing

Conversation

@graouts
Copy link
Contributor

@graouts graouts commented Apr 24, 2025

0f7edc3

[web-animations] remove the notion of an overriding start time when computing timing
https://bugs.webkit.org/show_bug.cgi?id=292000

Reviewed by Anne van Kesteren.

In 226359@main and 226577@main we added the notion of an overriding start time
when computing timing, to work around some compatibility issue with CSS Transitions
that is not longer required following the timing changes introduced by 294049@main.

We now remove this notion entirely.

* Source/WebCore/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations:
* Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:
* Source/WebCore/animation/AnimationEffect.cpp:
(WebCore::AnimationEffect::resolutionData const):
(WebCore::AnimationEffect::getBasicTiming):
(WebCore::AnimationEffect::getComputedTiming):
* Source/WebCore/animation/AnimationEffect.h:
* Source/WebCore/animation/CSSTransition.cpp:
(WebCore::CSSTransition::CSSTransition):
(WebCore::CSSTransition::resolve):
* Source/WebCore/animation/CSSTransition.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::apply):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::currentTime const):
(WebCore::WebAnimation::resolve):
* Source/WebCore/animation/WebAnimation.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::updateCSSTransitionsForStyleableAndProperty):

Canonical link: https://commits.webkit.org/294056@main

7130211

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 win-tests
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios 🧪 mac-wk2 🛠 gtk
✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 vision-sim 🧪 mac-wk2-stress 🧪 api-gtk
🧪 vision-wk2 🧪 mac-intel-wk2 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts self-assigned this Apr 24, 2025
@graouts graouts added the Animations Bugs related to CSS + SVG animations and transitions label Apr 24, 2025
@graouts graouts requested review from annevk, anttijk, heycam, nt1m and smfr April 24, 2025 08:03
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to introduce protectedAnimation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick search for Ref { in Source/WebCore/animation shows that this is the only place where we'd use such a new method, so I'm not sure it's worth it. That said, I'm sure there's plenty left to do to make that directory adhere to the memory safety programming guidelines, so perhaps in the future.

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #33018 (e7cde98)

⚠️ Found 1 fixed file! Please update expectations in Source/[Project]/SaferCPPExpectations by running the following command and update your pull request:

  • Tools/Scripts/update-safer-cpp-expectations -p WebCore --UncheckedCallArgsChecker animation/CSSTransition.cpp --UncountedCallArgsChecker animation/CSSTransition.cpp

@graouts graouts force-pushed the eng/web-animations-remove-the-notion-of-an-explicit-start-time-when-computing-timing branch from e7cde98 to 7130211 Compare April 24, 2025 10:21
@graouts graouts added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 24, 2025
…omputing timing

https://bugs.webkit.org/show_bug.cgi?id=292000

Reviewed by Anne van Kesteren.

In 226359@main and 226577@main we added the notion of an overriding start time
when computing timing, to work around some compatibility issue with CSS Transitions
that is not longer required following the timing changes introduced by 294049@main.

We now remove this notion entirely.

* Source/WebCore/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations:
* Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:
* Source/WebCore/animation/AnimationEffect.cpp:
(WebCore::AnimationEffect::resolutionData const):
(WebCore::AnimationEffect::getBasicTiming):
(WebCore::AnimationEffect::getComputedTiming):
* Source/WebCore/animation/AnimationEffect.h:
* Source/WebCore/animation/CSSTransition.cpp:
(WebCore::CSSTransition::CSSTransition):
(WebCore::CSSTransition::resolve):
* Source/WebCore/animation/CSSTransition.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::apply):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::currentTime const):
(WebCore::WebAnimation::resolve):
* Source/WebCore/animation/WebAnimation.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::updateCSSTransitionsForStyleableAndProperty):

Canonical link: https://commits.webkit.org/294056@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/web-animations-remove-the-notion-of-an-explicit-start-time-when-computing-timing branch from 7130211 to 0f7edc3 Compare April 24, 2025 10:23
@webkit-commit-queue
Copy link
Collaborator

Committed 294056@main (0f7edc3): https://commits.webkit.org/294056@main

Reviewed commits have been landed. Closing PR #44459 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 0f7edc3 into WebKit:main Apr 24, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 24, 2025
@graouts graouts deleted the eng/web-animations-remove-the-notion-of-an-explicit-start-time-when-computing-timing branch April 24, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Animations Bugs related to CSS + SVG animations and transitions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants