[webkit-reviews] review granted: [Bug 26963] Reproducible crash at FontCache::getFontData() when a custom font is used in a pseudo-style : [Attachment 32328] Validate the pseudo-style cache on style recalc

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 6 20:03:33 PDT 2009


Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 26963: Reproducible crash at FontCache::getFontData() when a custom font is
used in a pseudo-style
https://bugs.webkit.org/show_bug.cgi?id=26963

Attachment 32328: Validate the pseudo-style cache on style recalc
https://bugs.webkit.org/attachment.cgi?id=32328&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
> +bool Element::validatePseudoStyleCache(const RenderStyle* currentStyle,
RenderStyle* newStyle)

I don't understand why this function is named "validate" yet returns a boolean.
Typically if functions return a value they are named after the value. I think
the value means "needs to dump the style cache" or something like that, so
validate seems an unhelpful name.

> +    Vector<RenderStyle*, 10> pseudoStyleCache;

I'd prefer a typedef so the magic number 10 isn't repeated here.

> +	   if (*newPseudoStyle.get() != *pseudoStyleCache[i]) {

Do you really need the call to get() here? If so, I'm surprised you do.

Otherwise, based on Hyatt's review and my reading of the code, r=me


More information about the webkit-reviews mailing list