Bug 235503 - Pass the Document through bindings to KeyframeEffect::setKeyframes()
Summary: Pass the Document through bindings to KeyframeEffect::setKeyframes()
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: 2022-01-24 01:33 PST by Antoine Quint
Modified: 2022-01-24 11:09 PST (History)
11 users (show)

See Also:


Attachments
Patch (16.41 KB, patch)
2022-01-24 01:41 PST, Antoine Quint
sam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2022-01-24 01:33:42 PST
Pass the Document through bindings to KeyframeEffect::setKeyframes()
Comment 1 Antoine Quint 2022-01-24 01:41:15 PST
Created attachment 449790 [details]
Patch
Comment 2 Sam Weinig 2022-01-24 09:07:02 PST
Comment on attachment 449790 [details]
Patch

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

> Source/WebCore/animation/KeyframeEffect.cpp:210
> -        auto* scriptExecutionContext = jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject)->scriptExecutionContext();
> -        if (is<Document>(scriptExecutionContext)) {
> -            if (downcast<Document>(*scriptExecutionContext).settings().webAnimationsCompositeOperationsEnabled())
> -                baseProperties.composite = baseKeyframe.composite;
> -        }
> +        if (document.settings().webAnimationsCompositeOperationsEnabled())
> +            baseProperties.composite = baseKeyframe.composite;

This seems right, but in future, you can actually get settings directly from the ScriptExecutionContext via the settingsValues() accessor.
Comment 3 Antoine Quint 2022-01-24 10:34:56 PST
(In reply to Sam Weinig from comment #2)
> Comment on attachment 449790 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449790&action=review
> 
> > Source/WebCore/animation/KeyframeEffect.cpp:210
> > -        auto* scriptExecutionContext = jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject)->scriptExecutionContext();
> > -        if (is<Document>(scriptExecutionContext)) {
> > -            if (downcast<Document>(*scriptExecutionContext).settings().webAnimationsCompositeOperationsEnabled())
> > -                baseProperties.composite = baseKeyframe.composite;
> > -        }
> > +        if (document.settings().webAnimationsCompositeOperationsEnabled())
> > +            baseProperties.composite = baseKeyframe.composite;
> 
> This seems right, but in future, you can actually get settings directly from
> the ScriptExecutionContext via the settingsValues() accessor.

Good to know! This patch could have just passed the ScriptExecutionContext in then, but as it turns out we also needed a Document further down to establish a CSSParserContext so it turned out to be handy.
Comment 4 Antoine Quint 2022-01-24 10:41:33 PST
Committed r288452 (246341@trunk): <https://commits.webkit.org/246341@trunk>
Comment 5 Radar WebKit Bug Importer 2022-01-24 10:42:19 PST
<rdar://problem/87978492>
Comment 6 Antoine Quint 2022-01-24 11:09:28 PST
Committed r288454 (246343@trunk): <https://commits.webkit.org/246343@trunk>