This is the fourth level of the unzipping process described in https://bugs.webkit.org/show_bug.cgi?id=66567.
Created attachment 105687 [details]
Created attachment 105705 [details]
Comment on attachment 105705 [details]
Attachment 105705 [details] did not pass gtk-ews (gtk):
Created attachment 105803 [details]
Comment on attachment 105803 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=105803&action=review
> + static JSArray* create(JSGlobalData&, Structure*, unsigned initialLength, ArrayCreationMode createMode);
The argument name here, createMode, is not helpful and should be omitted.
> + static JSFunction* create(ExecState*, JSGlobalObject*, Structure*, int length, const Identifier& name, NativeFunction);
Is making these functions non-inline a good choice speed-wise?
> + void finishCreation(ExecState* exec, const Identifier& ident, JSValue value, unsigned attributes)
Normally we try not to abbreviate. So the name here would be identifier rather than ident.
Created attachment 105874 [details]
Created attachment 105882 [details]
Comment on attachment 105882 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=105882&action=review
> +void JSCallbackObject<Parent>::finishCreation(JSGlobalData& globalData)
> + UNUSED_PARAM(globalData);
Why not just leave the parameter name out instead? That’s the best way to mark an argument as unused in C++ functions.
> + finishCreation(exec->globalData(), jsNumber(timeClip(time)));
I would think we’d want the jsNumber function call inside the finishCreation function.
Created attachment 105975 [details]
Fixing review issues
Created attachment 105990 [details]
Resetting test bindings results
Created attachment 106010 [details]
Comment on attachment 106010 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=106010&action=review
Hmmm, there are no changelogs :-(
> + finishCreation(*globalData, globalObject, name);
Is it intentional the InternalFunction retains the finishCreation in its constructor?
> Hmmm, there are no changelogs :-(
Hmm dunno what happened to them…they were in earlier versions of the patch lol.
> Is it intentional the InternalFunction retains the finishCreation in its constructor?
Yep. It had its own isolated version of constructorBody() before, and now it's being changed to finishCreation(). InternalFunction is on the 4th level of the hierarchy below JSCell. If you look at its immediate superclass, JSObjectWithGlobalObject, its finishCreation() was removed from the constructor, meaning that it needs to be pushed down into the constructor body of InternalFunction.
Created attachment 106051 [details]
Comment on attachment 106051 [details]
Clearing flags on attachment: 106051
Committed r94364: <http://trac.webkit.org/changeset/94364>
All reviewed patches have been landed. Closing bug.