[webkit-reviews] review granted: [Bug 202192] [Web Animations] Move Document.getAnimations() to DocumentOrShadowRoot : [Attachment 395563] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 6 07:42:18 PDT 2020
Antti Koivisto <koivisto at iki.fi> has granted Antoine Quint
<graouts at apple.com>'s request for review:
Bug 202192: [Web Animations] Move Document.getAnimations() to
DocumentOrShadowRoot
https://bugs.webkit.org/show_bug.cgi?id=202192
Attachment 395563: Patch
https://bugs.webkit.org/attachment.cgi?id=395563&action=review
--- Comment #3 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 395563
--> https://bugs.webkit.org/attachment.cgi?id=395563
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=395563&action=review
> Source/WebCore/dom/Document.cpp:8104
> -Vector<RefPtr<WebAnimation>> Document::getAnimations()
> +Vector<RefPtr<WebAnimation>>
Document::getAnimations(IncludeAnimationsTargetingElementsInShadowRoot
includeAnimationsTargetingElementsInShadowRoot)
It might be cleaner to have a separate internal function that takes the
argument and have the argument-free getAnimations() just call that.
Internal clients would all switch to the new function.
> Source/WebCore/dom/Document.cpp:8120
> + for (auto& animation : m_timeline->getAnimations()) {
> + auto* effect = animation->effect();
> + ASSERT(is<KeyframeEffect>(animation->effect()));
> + auto* target = downcast<KeyframeEffect>(*effect).target();
Couldn't getAnimations()/new internal function just return a list of
KeyframeEffects is those are the only things it can contain?
> Source/WebCore/dom/Document.h:1488
> + Vector<RefPtr<WebAnimation>>
getAnimations(IncludeAnimationsTargetingElementsInShadowRoot =
IncludeAnimationsTargetingElementsInShadowRoot::No);
Then you wouldn't need a default parameter either.
More information about the webkit-reviews
mailing list