Bug 182903 – [Web Animations] Store all parsed keyframe input information in a single structure

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

[Web Animations] Store all parsed keyframe input information in a single structure

[Antoine Quint]

Created attachment 334107 [details]
Patch

[Dean Jackson]

Comment on attachment 334107 [details]
Patch

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

> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:85
> +        keyframe.computedOffset = keyframe.offset ? keyframe.offset.value() : 0;

keyframe.offset.value_or(0)

> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:442
> +        ParsedKeyframe parsedKeyframe;
> +        parsedKeyframe.easing = sourceParsedKeyframe.easing;
> +        parsedKeyframe.offset = sourceParsedKeyframe.offset;
> +        parsedKeyframe.composite = sourceParsedKeyframe.composite;
> +        parsedKeyframe.unparsedStyle = sourceParsedKeyframe.unparsedStyle;
> +        parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
> +        parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
> +        parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();

You should make a operator= for this.

[Antoine Quint]

(In reply to Dean Jackson from comment #2)
> Comment on attachment 334107 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334107&action=review
> 
> > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:85
> > +        keyframe.computedOffset = keyframe.offset ? keyframe.offset.value() : 0;
> 
> keyframe.offset.value_or(0)

Very cool.

> > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:442
> > +        ParsedKeyframe parsedKeyframe;
> > +        parsedKeyframe.easing = sourceParsedKeyframe.easing;
> > +        parsedKeyframe.offset = sourceParsedKeyframe.offset;
> > +        parsedKeyframe.composite = sourceParsedKeyframe.composite;
> > +        parsedKeyframe.unparsedStyle = sourceParsedKeyframe.unparsedStyle;
> > +        parsedKeyframe.computedOffset = sourceParsedKeyframe.computedOffset;
> > +        parsedKeyframe.timingFunction = sourceParsedKeyframe.timingFunction;
> > +        parsedKeyframe.style = sourceParsedKeyframe.style->mutableCopy();
> 
> You should make a operator= for this.

I intentionally did not provide one because I would like to avoid unexpected copies of ParsedKeyframe.

[Antoine Quint]

Committed r228702: <https://trac.webkit.org/changeset/228702>

[Radar WebKit Bug Importer]

<rdar://problem/37677856>