Bug 183371 - [Web Animations] Add a new CSSAnimation subclass of WebAnimation
Summary: [Web Animations] Add a new CSSAnimation subclass of WebAnimation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks: 183373
  Show dependency treegraph
 
Reported: 2018-03-06 09:00 PST by Antoine Quint
Modified: 2018-03-06 10:34 PST (History)
3 users (show)

See Also:


Attachments
Patch (19.23 KB, patch)
2018-03-06 09:03 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (20.15 KB, patch)
2018-03-06 09:24 PST, Antoine Quint
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2018-03-06 09:00:16 PST
As we implement CSS Animations over the Web Animations engine, we’ll need a way to represent an animation created using CSS Animations. To do so, we’ll add a new CSSAnimation subclass of WebAnimation. This Radar covers the basic task of adding the new class to WebCore.
Comment 1 Antoine Quint 2018-03-06 09:00:25 PST
<rdar://problem/38181724>
Comment 2 Antoine Quint 2018-03-06 09:03:06 PST
Created attachment 335101 [details]
Patch
Comment 3 Antoine Quint 2018-03-06 09:24:41 PST
Created attachment 335104 [details]
Patch
Comment 4 EWS Watchlist 2018-03-06 09:27:55 PST
Attachment 335104 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:21:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Dean Jackson 2018-03-06 09:39:15 PST
Comment on attachment 335104 [details]
Patch

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

> Source/WebCore/animation/CSSAnimation.cpp:40
> +    auto& document = target.document();
> +
> +    auto result = adoptRef(*new CSSAnimation(document));
> +
> +    return result;

why not just return adoptRef(*new CSSAnimation(target.document());

> Source/WebCore/animation/CSSAnimation.h:42
> +    const String& animationName() const { return m_animationName; }

You have no way to set the animation name yet.
Comment 6 Antoine Quint 2018-03-06 10:02:54 PST
(In reply to Dean Jackson from comment #5)
> Comment on attachment 335104 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335104&action=review
> 
> > Source/WebCore/animation/CSSAnimation.cpp:40
> > +    auto& document = target.document();
> > +
> > +    auto result = adoptRef(*new CSSAnimation(document));
> > +
> > +    return result;
> 
> why not just return adoptRef(*new CSSAnimation(target.document());
> 
> > Source/WebCore/animation/CSSAnimation.h:42
> > +    const String& animationName() const { return m_animationName; }
> 
> You have no way to set the animation name yet.

Indeed, this is just setting the stage for future patches where we'll be adding more code to CSSAnimation::create().
Comment 7 Antoine Quint 2018-03-06 10:34:30 PST
Committed r229327: <https://trac.webkit.org/changeset/229327>