Bug 71660 – Remove [CustomGetter] IDL for window.Audio of V8

• Component: DOM
• Status: RESOLVED FIXED

[Kentaro Hara]

The getter for HTMLAudioElementConstructor of V8 does not need to be a custom getter.

[Kentaro Hara]

Created attachment 113844 [details]
Patch

[Adam Barth]

Comment on attachment 113844 [details]
Patch

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

> Source/WebCore/page/DOMWindow.idl:630
> -        attribute [CustomGetter, Conditional=VIDEO, EnabledAtRuntime] HTMLAudioElementConstructor Audio; // Usable with the new operator
> +        attribute [JSCCustomGetter, Conditional=VIDEO, EnabledAtRuntime] HTMLAudioElementConstructorConstructor Audio; // Usable with the new operator

I don't fully understand why HTMLAudioElementConstructorConstructor is needed in place of HTMLAudioElementConstructor.  It seems like Audio is indeed just an HTMLAudioElementConstructor because calling "new Audio()" gives you an HTMLAudioElement.  Maybe this is something we should change in the code generator?

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 113844 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113844&action=review
> 
> > Source/WebCore/page/DOMWindow.idl:630
> > -        attribute [CustomGetter, Conditional=VIDEO, EnabledAtRuntime] HTMLAudioElementConstructor Audio; // Usable with the new operator
> > +        attribute [JSCCustomGetter, Conditional=VIDEO, EnabledAtRuntime] HTMLAudioElementConstructorConstructor Audio; // Usable with the new operator
> 
> I don't fully understand why HTMLAudioElementConstructorConstructor is needed in place of HTMLAudioElementConstructor.  It seems like Audio is indeed just an HTMLAudioElementConstructor because calling "new Audio()" gives you an HTMLAudioElement.  Maybe this is something we should change in the code generator?

In my understanding...

- window.HTMLAudioElement and window.Audio are different things. In other words, they have different wrapper types (i.e. WebCore::BatchedAttribute::WrapperTypeInfo should be different). Currently, the wrapper type of window.HTMLAudioElement is implemented as class V8HTMLAudioElement, and the wrapper type of window.Audio is implemented as V8HTMLAudioElementConstructor.

- "attribute HTMLAudioElementConstructor HTMLAudioElement" in DOMWindow.idl indicates that the wrapper type of window.HTMLAudioElement is V8HTMLAudioElement.

- "attribute HTMLAudioElementConstructorConstructor Audio" in DOMWindow.idl indicates that the wrapper type of window.Audio is V8HTMLAudioElementConstructor.

[Adam Barth]

How does Image and HTMLImageElement work?

[Kentaro Hara]

(In reply to comment #4)
> How does Image and HTMLImageElement work?

Image and HTMLImageElement, and Option and HTMLOptionElement are working in a similar way as I am trying to do for Audio and HTMLAudioElement in this patch, although they are still achieving it using a custom getter (I am going to remove their cutom getters in follow-up patches).

- "attribute HTMLImageElementConstructor HTMLImageElement" in DOMWindow.idl generates the code to set the wrapper type of window.HTMLImageElement to V8HTMLImageElement. Specifically, the generated code in V8DOMWindow.cpp is as follows:

{"HTMLImageElement", DOMWindowInternal::DOMWindowConstructorGetter, 0, &V8HTMLImageElement::info, static_cast<v8::AccessControl>(v8::DEFAULT), static_cast<v8::PropertyAttribute>(v8::ReadOnly), 0 /* on instance */},

- "attribute [CustomGetter] HTMLImageElementConstructor Image" generates a code to call a custom getter that sets the wrapper type of window.Image to V8HTMLImageElementConstructor. Specifically, the custom getter written in V8DOMWindowCustom.cpp is as follows:

v8::Handle<v8::Value> V8DOMWindow::ImageAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    DOMWindow* window = V8DOMWindow::toNative(info.Holder());
    return V8DOMWrapper::getConstructor(&V8HTMLImageElementConstructor::info, window);
}

[Adam Barth]

Comment on attachment 113844 [details]
Patch

Ok.  I'm not sure this is the world's most elegant design, but I understand the reasoning behind it.  Thanks for explaining!

[WebKit Review Bot]

Comment on attachment 113844 [details]
Patch

Clearing flags on attachment: 113844

Committed r99481: <http://trac.webkit.org/changeset/99481>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.