[webkit-reviews] review requested: [Bug 84605] [chromium] Do not clobber synchronized start times. : [Attachment 138821] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 25 09:09:07 PDT 2012


vollick at chromium.org has asked	for review:
Bug 84605: [chromium] Do not clobber synchronized start times.
https://bugs.webkit.org/show_bug.cgi?id=84605

Attachment 138821: Patch
https://bugs.webkit.org/attachment.cgi?id=138821&action=review

------- Additional Comments from vollick at chromium.org
(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.)


More information about the webkit-reviews mailing list