Bug 33563 - Transition followed by animation fails to run the animation sometimes
Summary: Transition followed by animation fails to run the animation sometimes
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
Keywords: InRadar
Depends on:
Reported: 2010-01-12 17:23 PST by Simon Fraser (smfr)
Modified: 2010-01-13 20:55 PST (History)
1 user (show)

See Also:

Testcase (1.38 KB, text/html)
2010-01-12 17:23 PST, Simon Fraser (smfr)
no flags Details
Patch (11.57 KB, patch)
2010-01-13 13:56 PST, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-01-12 17:23:15 PST
Created attachment 46416 [details]

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
Comment 2 Simon Fraser (smfr) 2010-01-13 13:56:32 PST
Created attachment 46501 [details]
Comment 3 Darin Adler 2010-01-13 15:37:52 PST
Comment on attachment 46501 [details]

> +        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.

Comment 4 Simon Fraser (smfr) 2010-01-13 20:55:01 PST
Committed with review comments addressed: