[webkit-reviews] review granted: [Bug 179777] Clean up KeyframeEffect : [Attachment 327069] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Antoine Quint
<graouts at apple.com>'s request for review:
Bug 179777: Clean up KeyframeEffect
https://bugs.webkit.org/show_bug.cgi?id=179777

Attachment 327069: Patch

https://bugs.webkit.org/attachment.cgi?id=327069&action=review




--- Comment #2 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 327069
  --> https://bugs.webkit.org/attachment.cgi?id=327069
Patch

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

> Source/WebCore/animation/KeyframeEffect.cpp:111
> +	   Vector<CSSPropertyID> properties(numberOfCSSProperties);
>	   for (unsigned k = 0; k < numberOfCSSProperties; ++k) {
> -	       properties.append(styleProperties->propertyAt(k).id());
> +	       properties.uncheckedAppend(styleProperties->propertyAt(k).id());

This is not correct. This will allocate numberOfCSSProperties CSSPropertyID
default constructed objects; => we are both allocating capacity and changing
the size of the Vector. Then we are using unchecked append to append new
elements outside the bounds of the Vector. If we allocate up-front then we
should be modifying the existing elements in the Vector. That is, we should not
using uncheckedAppend. Alternatively, we should use
Vector::reserveInitialCapacity() and Vector::uncheckedAppend() to allocate the
underlying buffer without object construction and then safely construct new
objects in the first free position in the buffer (increasing the size of the
Vector).