Before we can get started implementing more of the APIs of Animation, we need to implement the animation class that animations can dispatch.
<rdar://problem/35966325>
Created attachment 328965 [details] Patch
Created attachment 328973 [details] Patch
Comment on attachment 328973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328973&action=review > Source/WebCore/animation/AnimationPlaybackEvent.cpp:46 > + : Event(type, true, false) What is the middle 'true' here? I guess it is a different constructor to the one you're using above? > Source/WebCore/animation/AnimationPlaybackEvent.cpp:58 > + if (m_currentTime) > + return m_currentTime->milliseconds(); > + return std::nullopt; We normally do these the other way around. if (!m_currentTime) return std::nullopt; return actualvalue; > Source/WebCore/animation/AnimationPlaybackEvent.cpp:71 > +EventInterface AnimationPlaybackEvent::eventInterface() const > +{ > + return AnimationPlaybackEventInterfaceType; > +} This can be inline in the header file. > Source/WebCore/animation/AnimationPlaybackEvent.h:40 > + static Ref<AnimationPlaybackEvent> create(const AtomicString& type, const AnimationPlaybackEventInit& initializer, IsTrusted isTrusted = IsTrusted::No) Not your issue, but I wish it was Trusted::NotTrusted, although Trusted::Trusted would look weird.
(In reply to Dean Jackson from comment #4) > Comment on attachment 328973 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=328973&action=review > > > Source/WebCore/animation/AnimationPlaybackEvent.cpp:46 > > + : Event(type, true, false) > > What is the middle 'true' here? I guess it is a different constructor to the > one you're using above? Yes, one is with the initialiser, this one is Event(const AtomicString& type, bool canBubble, bool cancelable). > > Source/WebCore/animation/AnimationPlaybackEvent.cpp:58 > > + if (m_currentTime) > > + return m_currentTime->milliseconds(); > > + return std::nullopt; > > We normally do these the other way around. > > if (!m_currentTime) > return std::nullopt; > > return actualvalue; Will fix. > > Source/WebCore/animation/AnimationPlaybackEvent.cpp:71 > > +EventInterface AnimationPlaybackEvent::eventInterface() const > > +{ > > + return AnimationPlaybackEventInterfaceType; > > +} > > This can be inline in the header file. Will fix.
Committed r225745: <https://trac.webkit.org/changeset/225745>
Committed r225749: <https://trac.webkit.org/changeset/225749>
Committed r225750: <https://trac.webkit.org/changeset/225750>