[webkit-reviews] review requested: [Bug 45896] CSS: Fix crash in getTimingFunctionValue() : [Attachment 67803] Proposed patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has asked	for review:
Bug 45896: CSS: Fix crash in getTimingFunctionValue()
https://bugs.webkit.org/show_bug.cgi?id=45896

Attachment 67803: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=67803&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 13da8c8..7277379 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,15 @@
> +2010-09-16  Andreas Kling  <andreas.kling at nokia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   CSS: Fix crash in getTimingFunctionValue()
> +	   https://bugs.webkit.org/show_bug.cgi?id=45896
> +
> +	   Use RefPtrs to avoid deleting the TimingFunctions prematurely.

You should say here why you didn't add any tests.

> diff --git a/WebCore/css/CSSComputedStyleDeclaration.cpp
b/WebCore/css/CSSComputedStyleDeclaration.cpp
> index ce96e1c..f351cd7 100644
> --- a/WebCore/css/CSSComputedStyleDeclaration.cpp
> +++ b/WebCore/css/CSSComputedStyleDeclaration.cpp
> @@ -514,12 +514,12 @@ static PassRefPtr<CSSValue>
getTimingFunctionValue(const AnimationList* animList
>      RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
>      if (animList) {
>	   for (size_t i = 0; i < animList->size(); ++i) {
> -	       const TimingFunction* tf =
animList->animation(i)->timingFunction().get();
> +	       RefPtr<TimingFunction> tf =
animList->animation(i)->timingFunction();

I don't see why this RefPtr is needed. How can
animList->animation(i)->timingFunction() go bad here?