Bug 102601 – Add ScriptWrappable to more WebCore classes which are commonly JS-wrapped

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Add ScriptWrappable to more WebCore classes which are commonly JS-wrapped

[Eric Seidel (no email)]

Created attachment 174833 [details]
Patch

[Eric Seidel (no email)]

These were all found using:
https://bugs.webkit.org/show_bug.cgi?id=102539#c2
and then surfing around to a few common sites (google.com, amazon.com, yahoo.com, apple.com) and running robohornet, parts of dromaeo, html5test.com and acid3.

I'm sure there are many more little ones to find, but I doubt even this patch will show up in benchmarks.

This patch should be a small memory savings (there are not many of these objects created) and a small perf savings on a few sites.

I think if we want to do more of these, we should come up with a better way to test for over/under-usage of ScriptWrappable.  The current manual-testing solution is not sustainable in the long term. :)

[Adam Barth]

Comment on attachment 174833 [details]
Patch

Ok

[WebKit Review Bot]

Comment on attachment 174833 [details]
Patch

Clearing flags on attachment: 174833

Committed r135058: <http://trac.webkit.org/changeset/135058>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

I suspect there are *many* more classes in WebCore which are only ever used from the JS DOM API and thus technically should be ScriptWrappable.  Most notable being all the SVG pod types and DOM objects.  But since those didn't show up in my brief testing, I doubt they're particularly performance critical.

Ideally we'd come up with a better model for making sure we always have the right set of ScriptWrappable objects.  Adam suggested we might eventually make objects with an IDL have to opt-out of the bindings assuming ScriptWrappable-ness, and thus catch many more cases.  Not sure.