Bug 65347 – Refactor JS objects to allocate in static create methods rather than constructors

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

Now that we have static create methods and we can assert that we allocate only when we're not initializing another object, we want to move as much of the allocation done inside of constructors as possible into their static create methods.  Currently we clear the m_initializingObject flag inside of JSGlobalData if we do any sort of nested allocation within a constructor and then set it back to its original value after that allocation is complete.  Ideally, we wouldn't have to do this at all.

[Mark Hahnenberg]

Created attachment 103570 [details]
Moved all allocation out of initialization lists in subclass of JSCell

[Mark Hahnenberg]

Created attachment 103633 [details]
Fixing build errors

[Early Warning System Bot]

Comment on attachment 103633 [details]
Fixing build errors

Attachment 103633 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9357258

[Mark Hahnenberg]

Created attachment 103642 [details]
Fixing build errors

[Geoffrey Garen]

Comment on attachment 103642 [details]
Fixing build errors

View in context: https://bugs.webkit.org/attachment.cgi?id=103642&action=review

The rest of this patch looks good. But let's get rid of that cast.

> Source/WebCore/bindings/js/JSDOMBinding.h:91
> -        return cacheDOMStructure(globalObject, WrapperClass::createStructure(exec->globalData(), WrapperClass::createPrototype(exec, globalObject)), &WrapperClass::s_info);
> +        return cacheDOMStructure(globalObject, WrapperClass::createStructure(exec->globalData(), JSC::JSValue((JSC::JSCell*)WrapperClass::createPrototype(exec, globalObject))), &WrapperClass::s_info);

Let's find the missing #include that necessitated this cast. I'd rather not add a cast like this if we don't have to.

[Mark Hahnenberg]

Created attachment 103920 [details]
Revised patch

[Early Warning System Bot]

Comment on attachment 103920 [details]
Revised patch

Attachment 103920 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9379723

[Mark Hahnenberg]

Created attachment 103928 [details]
Fixing qt build

[Geoffrey Garen]

Comment on attachment 103928 [details]
Fixing qt build

r=me

[WebKit Review Bot]

Comment on attachment 103928 [details]
Fixing qt build

Clearing flags on attachment: 103928

Committed r93059: <http://trac.webkit.org/changeset/93059>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mark Hahnenberg]

Created attachment 103952 [details]
Moving all allocation in constructors into finishAllocations() method

[Mark Hahnenberg]

Created attachment 103958 [details]
Fixing build errors

[Mark Hahnenberg]

Created attachment 103970 [details]
Fixing build errors

[Mark Hahnenberg]

Reopening this bug because its description/goal is not yet complete.  Also opening other bugs to resolve this discrepancy a step at a time.

[Mark Hahnenberg]

All subtasks complete, closing bug.