[webkit-reviews] review granted: [Bug 7511] make win32 DumpRenderTree run without crashing : [Attachment 6772] patch to fix remaining crashes

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 7511: make win32 DumpRenderTree run without crashing
http://bugzilla.opendarwin.org/show_bug.cgi?id=7511

Attachment 6772: patch to fix remaining crashes
http://bugzilla.opendarwin.org/attachment.cgi?id=6772&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'd prefer that the macro names like QNAME_DEFAULT_CONSTRUCTOR and
DOM_HTMLNAMES_HIDE_GLOBALS match more closely -- either both should use a
prefix or neither.

It seems to me that the !AVOID_STATIC_CONSTRUCTORS case is now a bit broken, so
I'm not sure why AVOID_STATIC_CONSTRUCTORS still exists. It seems we now have a
new technique for avoiding static constructors that is more portable, so we can
do it everywhere.

I'm not happy with all the casts to void*, although I suppose they are
necessary.

I don't understand why the changes here for static constructors are not also in
JavaScriptCore.

Instead of __VA_ARGS__ the DEFINE_GLOBAL macro could be changed to take a
parenthesized list of constructor parameters as a single macro argument, and
hence work around a problem you were addressing here.

The patch has tabs in it, in the DocumentImpl.cpp part.

r=me