[Webkit-unassigned] [Bug 110926] Can't use fastGetAttribute with WebCore::HTMLNames::classAttr when element is a SVGStyledElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110926





--- Comment #44 from Tim 'mithro' Ansell <mithro at mithis.com>  2013-04-09 01:51:12 PST ---
(From update of attachment 196067)
View in context: https://bugs.webkit.org/attachment.cgi?id=196067&action=review

>> Source/WebCore/dom/Element.h:862
>> +    return fastGetAttribute(HTMLNames::classAttr);
> 
> This is a great idea, but I believe the execution is still a little bit incorrect.
> 
> It’s a programming error to call fastGetAttribute on classAttr from any call site other than this function. Thus the fastAttributeLookupAllowed function should return false for classAttr. Thus, this line of code is wrong, and the fastAttributeLookupAllowed function also is wrong to return "true" for classAttr since it’s not allowed.
> 
> There are a number of ways to fix this. The simplest is to make a separate private inline function for the “guts” of fastGetAttribute, without the fastAttributeLookupAllowed assertion, and call that here. There are various other suitable refactoring. Even for SVG elements, there is no reason to do the styleAttr check that getAttribute does when fetching a classAttr, so that’s a case we could optimize further if desired.
> 
> Have we done performance tests on the difference in performance from calling getAttribute vs. "if (isSVG) getAttribute() else fastGetAttribute()"? If it’s not a measurable difference then we should consider that implementation.

This patch doesn't affect fastAttributeLookupAllowed. The fastAttributeLookupAllowed function already returns *false* for classAttr when the object is an SVG object, which is why this patch even started.

This patch does precisely;
if (isSVG) getAttribute() else fastGetAttribute()

As Eric mentioned
>Calling fastGetAttribute(classAttr) is measurably more efficient than calling getAttribute(classAttr).  We go to great lengths to avoid getAttribute in many places: http://trac.webkit.org/browser/trunk/Source/WebCore/css/StyleResolver.cpp#L985

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.