[webkit-reviews] review granted: [Bug 69801] Regarding constructor, replace [ConstructorWith=...] IDL with [CallWith=...] IDL : [Attachment 110833] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 69801: Regarding constructor, replace [ConstructorWith=...] IDL with
[CallWith=...] IDL
https://bugs.webkit.org/show_bug.cgi?id=69801

Attachment 110833: Patch
https://bugs.webkit.org/attachment.cgi?id=110833&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110833&action=review


> Source/WebCore/ChangeLog:7
> +	   As for constructors, [ConstructorWith=...] has the same meaning as
[CallWith=...].
> +	   We should deprecate [ConstructorWith=...]. This is a clean-up bug
for the bug 65839.

It's somewhat less obvious what CallWith means as an attribute of the whole
interface.  I guess it makes sense for JavaScript where the interface *is* the
constructor, but it probably makes somewhat less sense for other languages.  I
still think this patch is a win though.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1549
> -    if ($dataNode->extendedAttributes->{"ConstructorWith"} &&
$dataNode->extendedAttributes->{"ConstructorWith"} eq "ScriptExecutionContext")
{
> +    if ($dataNode->extendedAttributes->{"CallWith"} &&
$dataNode->extendedAttributes->{"CallWith"} eq "ScriptExecutionContext") {

Do we not need a similar change to CodeGeneratorJSC.pm ?  I guess all these
interfaces have JSCustomConstructor.

> Source/WebCore/fileapi/FileReader.idl:-41
> -	   ConstructorWith=ScriptExecutionContext,

This doesn't need CallWith ?  I guess not if we can compile without it.