[webkit-reviews] review denied: [Bug 22870] Animations and Transitions started in the same recalcStyle() cycle should have identical start times : [Attachment 26035] Patch, including LayoutTest files

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 18 13:38:55 PST 2008


Dave Hyatt <hyatt at apple.com> has denied Chris Marrin <cmarrin at apple.com>'s
request for review:
Bug 22870: Animations and Transitions started in the same recalcStyle() cycle
should have identical start times
https://bugs.webkit.org/show_bug.cgi?id=22870

Attachment 26035: Patch, including LayoutTest files
https://bugs.webkit.org/attachment.cgi?id=26035&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
+    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.


More information about the webkit-reviews mailing list