[webkit-reviews] review granted: [Bug 38963] Animation keyframe timing functions are applying incorrectly : [Attachment 58531] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 38963: Animation keyframe timing functions are applying incorrectly
https://bugs.webkit.org/show_bug.cgi?id=38963

Attachment 58531: Patch
https://bugs.webkit.org/attachment.cgi?id=58531&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    AnimationList() {}
> +    AnimationList(const AnimationList& o);

We normally put a space between the braces in "{}".

There’s no need for the argument name "o". (In the definition we prefer names
to letters for arguments, so there other would be better then o.)

It’s best practice to also defined an assignment operator if you define a copy
constructor. There are three ways you can deal with this:

    1) Declare a private assignment operator and do not define it.

    2) Use Noncopyable to do the above, but this will require a change to the
copy constructor.

    3) Define a public assignment operator that uses swap (see
<http://www.gotw.ca/gotw/059.htm>).

	AnimationList& operator=(const AnimationList& other)
	{
	    AnimationList copy(other);
	    swap(copy);
	    return *this;
	}

If you do (3) we also need to write the swap function, but that is typically
simple. I suggest doing either (1) or (3) rather than checking this in as-is.