Bug 56117

Summary:

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

Product:

WebKit

Reporter:

anton muhin <antonm>

Component:

WebKit Misc.

Assignee:

anton muhin <antonm>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, gregsimon, mnaganov, vitalyr

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none

anton muhin

Reported 2011-03-10 10:10:48 PST

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

Attachments
Patch (3.41 KB, patch) 2011-03-10 10:23 PST, anton muhin	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

anton muhin

Comment 1 2011-03-10 10:18:42 PST

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

Comment 2 2011-03-10 10:23:36 PST

Created attachment 85346 [details] Patch

Adam Barth

Comment 3 2011-03-10 13:57:15 PST

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

Comment 4 2011-03-11 03:12:28 PST

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 5 2011-03-11 04:11:03 PST

Comment on attachment 85346 [details] Patch Clearing flags on attachment: 85346 Committed r80842: <http://trac.webkit.org/changeset/80842>

WebKit Commit Bot

Comment 6 2011-03-11 04:11:07 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.