Bug 155022 - Enable DOM class create functions to take parameters in case of JSBuiltinConstructor
Summary: Enable DOM class create functions to take parameters in case of JSBuiltinCons...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 154729
  Show dependency treegraph
 
Reported: 2016-03-04 08:55 PST by youenn fablet
Modified: 2016-03-06 12:44 PST (History)
5 users (show)

See Also:


Attachments
Patch (12.65 KB, patch)
2016-03-04 09:06 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Fixing DOMWrapper::create(...) case (12.71 KB, patch)
2016-03-04 09:42 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (12.64 KB, patch)
2016-03-06 09:44 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-03-04 08:55:12 PST
This is needed for bug 154729
Comment 1 youenn fablet 2016-03-04 09:06:24 PST
Created attachment 273002 [details]
Patch
Comment 2 youenn fablet 2016-03-04 09:42:47 PST
Created attachment 273005 [details]
Fixing DOMWrapper::create(...) case
Comment 3 Darin Adler 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.
Comment 4 youenn fablet 2016-03-06 09:44:50 PST
Created attachment 273137 [details]
Patch for landing
Comment 5 youenn fablet 2016-03-06 09:52:43 PST
Thanks for the review.
Landing patch uploaded according comments.
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2016-03-06 12:44:44 PST
All reviewed patches have been landed.  Closing bug.