Bug 33563

Summary: Transition followed by animation fails to run the animation sometimes
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase
none
Patch darin: review+

Description Simon Fraser (smfr) 2010-01-12 17:23:15 PST
Created attachment 46416 [details]
Testcase

The attached testcase shows an issue where, if a transition fires at the same time as an animation starts, and if the animation has an animation-delay similar to the duration of the transition, then sometimes the animation fails to run (with hardware accel.).
Comment 1 Simon Fraser (smfr) 2010-01-12 17:23:34 PST
<rdar://problem/7535633>
Comment 2 Simon Fraser (smfr) 2010-01-13 13:56:32 PST
Created attachment 46501 [details]
Patch
Comment 3 Darin Adler 2010-01-13 15:37:52 PST
Comment on attachment 46501 [details]
Patch

> +        Fix this by including they keyframes name (or empty string for transitions) in the labels

Typo: "they keyframes"

> +        * platform/graphics/mac/GraphicsLayerCA.h: Some new methods and signataure changes.

Typo: "signataure"

> +    void setAnimationOnLayer(CAPropertyAnimation*, AnimatedPropertyID, const String&, int index, double timeOffset);
> +    bool removeAnimationFromLayer(AnimatedPropertyID, const String&, int index);
> +    void pauseAnimationOnLayer(AnimatedPropertyID, const String&, int index, double timeOffset);

I think these new string arguments need an argument name for clarity.

>      String animationId = propertyIdToString(property);
>      animationId.append("_");
> +    if (!keyframesName.isEmpty()) {
> +        animationId.append(keyframesName);
> +        animationId.append("_");
> +    }
> +    animationId.append("_");
>      animationId.append(String::number(index));

This function is using an inefficient idiom for building a String. The String class allocates a new buffer every time you append to it. You should use StringBuilder or Vector<UChar> or some other alternate strategy.

r=me
Comment 4 Simon Fraser (smfr) 2010-01-13 20:55:01 PST
Committed with review comments addressed:
http://trac.webkit.org/changeset/53236