[webkit-dev] On the hazards of fast attribute lookup

[Andreas Kling]

Hey WebKittens!

WebCore::Element has two functions covering with most of your attribute
retrieval use cases:

const AtomicString& getAttribute(const QualifiedName&) const;
bool hasAttribute(const QualifiedName&) const;

An optimization was introduced in <http://trac.webkit.org/changeset/59281>
to allow faster lookup of certain attributes. It bypasses resolution of the
“style” attribute, and doesn’t update animatable SVG attributes before
returning them. This makes it slightly faster (and perfectly safe)  to use
fastGetAttribute() and fastHasAttribute() for anything that isn’t
HTMLNames::styleAttr or one of the SVG animatable attributes. The functions
are documented in Element.h as follows:

// Call this to get the value of an attribute that is known not to be the
style
// attribute or one of the SVG animatable attributes.

Unfortunately since the introduction of the fast attribute functions, there
has been some confusion about when and where it’s actually safe to use them.
In fact, I discovered quite a few bugs when taking a closer look at this.

Here is the full list of SVG animatable attributes in WebKit at the time of
writing:

In the HTMLNames namespace: class

In the XLinkNames namepace: href

In the SVGNames namespace: amplitude, azimuth, baseFrequency, bias,
clipPathUnits, cx, cy diffuseConstant, divisor, dx, dy edgeMode, elevation,
exponent, externalResourcesRequired filterRes, filterUnits, fx, fy
gradientTransform, gradientUnits, height, in2 in, intercept, k1, k2 k3, k4,
kernelMatrix, kernelUnitLength lengthAdjust, limitingConeAngle,
markerHeight, markerUnits markerWidth, maskContentUnits, maskUnits, method
mode, numOctaves, offset, operator order, orient, pathLength,
patternContentUnits patternTransform, patternUnits, pointsAtX, pointsAtY
pointsAtZ, preserveAlpha, preserveAspectRatio, primitiveUnits radius, r,
refX, refY result, rotate, rx, ry scale, seed slope, spacing
specularConstant, specularExponent spreadMethod, startOffset stdDeviation,
stitchTiles surfaceScale, tableValues target, targetX targetY, transform
type, values viewBox, width x1, x2 x xChannelSelector, y1 y2, y
yChannelSelector, z

The worst thing is that there’s been nothing stopping you from asking the
fast functions about these attributes, you’ll simply get incorrect results
in some cases.

To get us out of this mess, I’ve added a assertions to verify that we aren’t
micro-optimizing ourselves in the foot. The fast lookup functions now assert
that fastAttributeLookupAllowed() is true for the requested attribute. For
non-SVG elements, it simply checks that you aren’t asking for the “style”
attribute. For SVG elements, it calls out to
SVGElement::isAnimatableAttribute().

Oh, and these changes landed in <http://trac.webkit.org/changeset/97670>.

Thanks for reading, and happy hacking! :)

-Kling
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20111018/2402b920/attachment.html>