Pass the Document through bindings to KeyframeEffect::setKeyframes()

• Component: Animations
• Status: RESOLVED FIXED

[Antoine Quint]

Pass the Document through bindings to KeyframeEffect::setKeyframes()

[Antoine Quint]

Created attachment 449790 [details]
Patch

[Sam Weinig]

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.

[Antoine Quint]

(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.

[Antoine Quint]

Committed r288452 (246341@trunk): <https://commits.webkit.org/246341@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/87978492>

[Antoine Quint]

Committed r288454 (246343@trunk): <https://commits.webkit.org/246343@trunk>