Currently the start times animations get is set by a call to currentTime() at the moment the animation is beng activated. So animations that should be started at the same time have slightly different start times. It's possible for some types of animations (rotations for instance) to therefore be slightly off from each other. It also makes animation events not fire at quite the same time. This can be fixed by registering an animation time value at the start of recalcStyle() and using that for all animations and transitions that start in that cycle.
Created attachment 26035 [details] Patch, including LayoutTest files Fixed https://bugs.webkit.org/show_bug.cgi?id=22870 I added a recalcStyleTime value to Document. This is updated at the start of the document's recalcStyle cycle and also when the document starts loading, since animations are started then as well. Then I always use that value when setting animation start times, so all animations started in the same cycle have the same start time. The test cases checked in test this, but in the case of the 'left' test it actually doesn't make any difference in most cases. This is because values are clamped to whole pixels, so the start times would have to be pretty far off for the test to fail using the old currentTime() model. Still, under really heavy load, it's possible for the test to fail without these changes. The 'transform' test is another story. It animates to the full resolution of a floating point number, so the test fails miserably without this fix.
Comment on attachment 26035 [details] Patch, including LayoutTest files + m_styleRecalcStartTime = -1; A named constant would be nice here instead of -1. + if (m_styleRecalcStartTime < 0) + const_cast<Document*>(this)->m_styleRecalcStartTime = currentTime(); Just make m_styleRecalcStartTime mutable. I think if (m_styleRecalcStartTime == cRecalcStyleTimeNotSet) will be more readable also (once you add the named constant). + if (m_startTime <= 0) { m_startTime = param; + } These brackets shouldn't be there. I do not understand why you would reset the m_styleRecalcStartTime when parsing is finished. That seems unnecessary. Finishing parsing is a pretty irrelevant milestone as far as the page rendering (the user may have been interacting with the page for a long time while it's still parsing in fact). I think you should just remove that code.
Created attachment 26161 [details] Replacement patch This fixes the issues identified and does the implementation a bit differently. Rather than maintaining a time value in Document, it is now in AnimationController. Document now makes begin/end calls to AnimationController at the start and end of recalcStyle. Right now, the time value is reset in the begin call and the end call does nothing. But it's there to keep things symmetrical. And I will be needing the end call in the future for some of the sync logic yet to do.
Comment on attachment 26161 [details] Replacement patch I like this patch better than the previous one. Nice. r=me
Sending LayoutTests/ChangeLog Sending LayoutTests/animations/animation-test-helpers.js Adding LayoutTests/animations/simultaneous-start-left-expected.txt Adding LayoutTests/animations/simultaneous-start-left.html Adding LayoutTests/animations/simultaneous-start-transform-expected.txt Adding LayoutTests/animations/simultaneous-start-transform.html Sending WebCore/ChangeLog Sending WebCore/dom/Document.cpp Sending WebCore/page/animation/AnimationBase.cpp Sending WebCore/page/animation/AnimationBase.h Sending WebCore/page/animation/AnimationController.cpp Sending WebCore/page/animation/AnimationController.h Sending WebCore/page/animation/KeyframeAnimation.cpp Transmitting file data ............. Committed revision 39667.