Bug 77662 – [chromium] Add impl-thread support for fill-mode and direction css animation properties

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

There are css animation properties that aren’t yet supported by the cc-thread animation code. If we detect an unsupported feature, it is possible to return false in GraphicsLayer::addAnimation to revert to the usual, main-thread animation code, but we should support most/all css animation properties. Specifically: animation-direction, animation-delay, animation-timing-function, and animation-fill-mode.

This should not block turning the feature on by default, since we gracefully degrade.

Created attachment 135354 [details]
Patch

[James Robinson]

Comment on attachment 135354 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [chromium] Add impl-thread support for all css animation properties.

I think "all" is a bit ambitious. Can you say specifically what this patch is adding support for?

> Source/WebKit/chromium/src/TextFieldDecoratorImpl.cpp:36
> +#include "Image.h"

this looks unrelated

> Source/WebKit/chromium/tests/CCActiveAnimationTest.cpp:72
> +TEST(CCActiveAnimationTest, TrimTimeAlternating)

is this sufficient test coverage?  should we have some tests for reverse as well?

Created attachment 140950 [details]
Patch

(In reply to comment #3)
> (From update of attachment 135354 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135354&action=review
>
> > Source/WebCore/ChangeLog:3
> > +        [chromium] Add impl-thread support for all css animation properties.
>
> I think "all" is a bit ambitious. Can you say specifically what this patch is adding support for?
Done.
>
> > Source/WebKit/chromium/src/TextFieldDecoratorImpl.cpp:36
> > +#include "Image.h"
>
> this looks unrelated
Removed.
>
> > Source/WebKit/chromium/tests/CCActiveAnimationTest.cpp:72
> > +TEST(CCActiveAnimationTest, TrimTimeAlternating)
>
> is this sufficient test coverage?  should we have some tests for reverse as well?
It isn't. Added tests to cover reverse.

[James Robinson]

Comment on attachment 140950 [details]
Patch

Cool, R=me. Thanks for the extra tests

[WebKit Review Bot]

Comment on attachment 140950 [details]
Patch

Clearing flags on attachment: 140950

Committed r116554: <http://trac.webkit.org/changeset/116554>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.