Bug 56117 – [v8] Change the way group id for CSS objects is calculated

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[anton muhin]

[v8] Change the way group id for CSS objects is calculated

[anton muhin]

Currently we simply traverse parents of StyleBase objects.  However, in contrast to DOM proper objects, those C++ pointers do not correspond to JS references.  For example, there is apparently no way to reach CSSStyleDeclaration if not from immediate CSSRule parent.

We hit exactly this problem: for inline styles, WebKit places CSSStyleDeclarations immediately under CSSStyleSheet (note there is no CSSRule in between).  Previous algorithm grouped all those inline styles under this style sheet (which is per document), now we don't group those objects.

See also https://bugs.webkit.org/show_bug.cgi?id=55899 and http://code.google.com/p/chromium/issues/detail?id=75118

[anton muhin]

Created attachment 85346 [details]
Patch

[Adam Barth]

Comment on attachment 85346 [details]
Patch

Ok.  It's too bad it's hard to test for the absence of these memory leaks.  This code is complex and subtle.  I don't fully understand the traversal you're doing here.  It seems to rely on the details of the CSS object model, which I don't know how stable they are.  I'm marking this r+ for now, but that's mostly because I don't know who else would be better to review it...

[anton muhin]

Thanks a lot, Adam.

Yes, it's indeed scary and I am not an expert in CSS OM model either.

(In reply to comment #3)
> (From update of attachment 85346 [details])
> Ok.  It's too bad it's hard to test for the absence of these memory leaks.  This code is complex and subtle.  I don't fully understand the traversal you're doing here.  It seems to rely on the details of the CSS object model, which I don't know how stable they are.  I'm marking this r+ for now, but that's mostly because I don't know who else would be better to review it...

[WebKit Commit Bot]

Comment on attachment 85346 [details]
Patch

Clearing flags on attachment: 85346

Committed r80842: <http://trac.webkit.org/changeset/80842>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.