[webkit-reviews] review granted: [Bug 17203] REGRESSION: High CPU usage loading HTML5 spec : [Attachment 19005] Patch with ChangeLog

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 8 13:00:48 PST 2008


Eric Seidel <eric at webkit.org> has granted Dave Hyatt <hyatt at apple.com>'s
request for review:
Bug 17203: REGRESSION: High CPU usage loading HTML5 spec
http://bugs.webkit.org/show_bug.cgi?id=17203

Attachment 19005: Patch with ChangeLog
http://bugs.webkit.org/attachment.cgi?id=19005&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
finishedParsingChildren() should probably be isFinishedParsingChildren() to
make it more clear that it's just a bool lookup (and less likely to be confused
with finishParsingChildren())

checkFirstChildRules and checkLastChildRules should be renamed now that they
don't return a bool (otherwise it's confusing with checkEmptyRules)  I suggest
markChildrenAffectedByLastChildRules() or similar.

We should eventually add some COMPILE_ASSERTs based on the size of the various
classes we're trying to keep small.  (Filed bug 17217 about that just now). 
Such would make it very easy when making these sorts of changes to know that
you're not inflating core classes by accident.

I've never been clear on why the "(m_element == e)" checks are there.  Perhaps
such could be moved into a more descriptive bool?
bool isMappedStyleRule = (m_element == e); // or whatever it's actually
checking.

Otherwise looks good. r=me.


More information about the webkit-reviews mailing list