Bug 12060 – SVG needs to be wired into the shared attribute system

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Eric Seidel (no email)]

SVG needs to be wired into the shared attribute system

HTML has a mechanism for sharing attribute nodes.  SVG has not yet implemented the proper DOM methods in order to wire into this system.

I just haven't bothered to learn much about the attribute sharing system.  But anyone who has, should find this to be an easy bug to fix.  I don't expect one needs to know much about the actual SVG attributes to be able to fix this.

[Eric Seidel (no email)]

Created attachment 12452 [details]
a first step in that direction (breaks layout tests)

Unfortunately this breaks all the SVG layout tests.  I expect that this patch is causing us to share RenderStyles and as a result exposing other bugs in SVG code.  I'm not certain however.

[Eric Seidel (no email)]

Created attachment 12453 [details]
Fixed patch

This patch has a beautiful attribute lookup system.  Eventually perhaps that system should be a .in file and auto-generated instead of being inline, but it's OK for now.

One could make a test to check that <g width="10px"> is not actually mapped to g.style.width, but I didn't bother.

[Eric Seidel (no email)]

My patch seems to have no affect on SVG style sharing however.  I turned on STYLE_SHARING_STATS and saw that simple SVGs are already sharing styles (and share no more after this change), and complicated SVGs (like plan.svg) are sharing no styles at all, even with this change.

It looks (from my minimal testing) that style sharing work when only attributes are used (those are mapped correctly -- at least after my change).  inline style declarations seem to kill style sharing, plan.svg is full of inline style declarations.

[Eric Seidel (no email)]

It looks like
bool CSSStyleSelector::canShareStyleWithElement(Node* n)
has !s->inlineStyleDecl() as part of its check.  That would explain why style sharing doesn't work at all for plan.svg.

[Eric Seidel (no email)]

I filed:
http://bugs.webkit.org/show_bug.cgi?id=12281
asking for the style sharing system to support inline styles.

[Dave Hyatt]

Comment on attachment 12453 [details]
Fixed patch

static HashMap<AtomicStringImpl*, int>* propertyNameToIdMap = 0;

Remove the "= 0".  Statics in a function are already initialized to 0.

r=me

[Eric Seidel (no email)]

Well, lots more could be wired up.  But this is a big step in the right direction.

r18876,