If an animation starts on the impl thread before it starts on the main thread, we currently clobber the synchronized start time. This is a bug.
Created attachment 138369 [details] Patch
Comment on attachment 138369 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138369&action=review > Source/WebCore/ChangeLog:5 > + The comment from your test would go well here as well. > Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h:84 > + void setStartTime(double startTime) { m_startTime = startTime; m_hasSetStartTime = true; } one statement per line
Created attachment 138373 [details] Patch (In reply to comment #2) > (From update of attachment 138369 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138369&action=review > > > Source/WebCore/ChangeLog:5 > > + > > The comment from your test would go well here as well. Done. > > > Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h:84 > > + void setStartTime(double startTime) { m_startTime = startTime; m_hasSetStartTime = true; } > > one statement per line Moved to the cpp.
Comment on attachment 138373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138373&action=review Looks OK but try to get rid of the new member if you don't really need it. > Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h:130 > + bool m_hasSetStartTime; we don't really need a bool, do we? m_startTime is initialized to 0 and can only become non-zero by a setStartTime() call or code internal to CCActiveAnimation, so would it be enough to just check for 0?
Created attachment 138821 [details] Patch (In reply to comment #4) > (From update of attachment 138373 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138373&action=review > > Looks OK but try to get rid of the new member if you don't really need it. > > > Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h:130 > > + bool m_hasSetStartTime; > > we don't really need a bool, do we? m_startTime is initialized to 0 and can only become non-zero by a setStartTime() call or code internal to CCActiveAnimation, so would it be enough to just check for 0? That's true. Removed. (Also, I had to update the unit test because I was setting the time to 0 and expecting hasSetStartTime() to be true.)
Created attachment 138833 [details] Patch
Comment on attachment 138833 [details] Patch Clearing flags on attachment: 138833 Committed r115226: <http://trac.webkit.org/changeset/115226>
All reviewed patches have been landed. Closing bug.