RESOLVED FIXED 155022
Enable DOM class create functions to take parameters in case of JSBuiltinConstructor 
https://bugs.webkit.org/show_bug.cgi?id=155022
Summary Enable DOM class create functions to take parameters in case of JSBuiltinCons...
youenn fablet
Reported 2016-03-04 08:55:12 PST
This is needed for bug 154729
Attachments
Patch (12.65 KB, patch)
2016-03-04 09:06 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Fixing DOMWrapper::create(...) case (12.71 KB, patch)
2016-03-04 09:42 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (12.64 KB, patch)
2016-03-06 09:44 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2016-03-04 09:06:24 PST
Created attachment 273002 [details] Patch
youenn fablet
Comment 2 2016-03-04 09:42:47 PST
Created attachment 273005 [details] Fixing DOMWrapper::create(...) case
Darin Adler
Comment 3 2016-03-05 18:07:32 PST
Comment on attachment 273005 [details] Fixing DOMWrapper::create(...) case View in context: https://bugs.webkit.org/attachment.cgi?id=273005&action=review > Source/WebCore/bindings/js/JSDOMConstructor.h:125 > + JSC::EncodedJSValue callConstructor(JSC::ExecState&, JSC::JSObject&); > + JSC::EncodedJSValue callConstructor(JSC::ExecState&, JSC::JSObject*); > // Usually defined for each specialization class. I don’t think these two should be grouped with the comment below. Consider adding a blank line. > Source/WebCore/bindings/js/JSDOMConstructor.h:261 > + ASSERT(castedThis); Not sure this assertion is valuable. > Source/WebCore/bindings/js/JSDOMWrapper.h:83 > + template<typename T> > + static constexpr auto test(int) -> decltype(T::create(), bool()) { return true; } > + template<typename T> > + static constexpr bool test(...) { return false; } I think the would read better as one liners instead of split into two lines.
youenn fablet
Comment 4 2016-03-06 09:44:50 PST
Created attachment 273137 [details] Patch for landing
youenn fablet
Comment 5 2016-03-06 09:52:43 PST
Thanks for the review. Landing patch uploaded according comments.
WebKit Commit Bot
Comment 6 2016-03-06 12:44:39 PST
Comment on attachment 273137 [details] Patch for landing Clearing flags on attachment: 273137 Committed r197642: <http://trac.webkit.org/changeset/197642>
WebKit Commit Bot
Comment 7 2016-03-06 12:44:44 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.