71660 2011-11-07 03:35:32 -0800 Remove [CustomGetter] IDL for window.Audio of V8 2011-11-07 14:52:22 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 71093 0 haraken webkit-unassigned abarth dominicc japhet ojan webkit.review.bot oldest_to_newest 497050 0 haraken 2011-11-07 03:35:32 -0800 The getter for HTMLAudioElementConstructor of V8 does not need to be a custom getter. 497069 1 113844 haraken 2011-11-07 04:33:06 -0800 Created attachment 113844 Patch 497233 2 113844 abarth 2011-11-07 09:21:22 -0800 Comment on attachment 113844 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? 497424 3 haraken 2011-11-07 13:25:46 -0800 (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. 497437 4 abarth 2011-11-07 13:42:58 -0800 How does Image and HTMLImageElement work? 497457 5 haraken 2011-11-07 14:07:38 -0800 (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); } 497469 6 113844 abarth 2011-11-07 14:21:44 -0800 Comment on attachment 113844 Patch Ok. I'm not sure this is the world's most elegant design, but I understand the reasoning behind it. Thanks for explaining! 497484 7 113844 webkit.review.bot 2011-11-07 14:39:58 -0800 Comment on attachment 113844 Patch Clearing flags on attachment: 113844 Committed r99481: <http://trac.webkit.org/changeset/99481> 497485 8 webkit.review.bot 2011-11-07 14:40:03 -0800 All reviewed patches have been landed. Closing bug. 113844 2011-11-07 04:33:06 -0800 2011-11-07 14:39:58 -0800 Patch bug-71660-20111107043305.patch text/plain 5807 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogOTkzNzYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAxNzMzYzY1YjIyOTQzMDI3 OGNjNGZhMTcxNWZlNDc2NGIyMWM1ODAzLi42ZTY0ODNhYzEzOWQyNTRlM2FiZGE0MzhhOTM0NDVk NWE5YmRhOTg4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjYgQEAKKzIwMTEtMTEtMDcgIEtlbnRh cm8gSGFyYSAgPGhhcmFrZW5AY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJlbW92ZSBbQ3VzdG9t R2V0dGVyXSBJREwgZnJvbSAnSFRNTEF1ZGlvRWxlbWVudENvbnN0cnVjdG9yJyBvZiBWOAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzE2NjAKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDdXJyZW50bHksICJu ZXcgQXVkaW8oKSIgdXNlcyBhIGN1c3RvbSBnZXR0ZXIgdG8gZ2V0IHRoZSB0ZW1wbGF0ZQorICAg ICAgICBvZiAnSFRNTEF1ZGlvRWxlbWVudENvbnN0cnVjdG9yJy4gSG93ZXZlciwgdGhpcyBnZXR0 ZXIgZG9lcyBub3QgbmVlZCB0bworICAgICAgICBiZSBjdXN0b20gYW5kIGNhbiB1c2UgYSBkZWZh dWx0IGdldHRlciAnRE9NV2luZG93SW50ZXJuYWw6OkRPTVdpbmRvd0NvbnN0cnVjdG9yR2V0dGVy Jy4KKworICAgICAgICBUZXN0czogZmFzdC9qcy9jdXN0b20tY29uc3RydWN0b3JzLmh0bWwKKyAg ICAgICAgICAgICAgIG1lZGlhL2F1ZGlvLWNvbnN0cnVjdG9yLmh0bWwKKyAgICAgICAgICAgICAg IG1lZGlhL2F1ZGlvLWNvbnN0cnVjdG9yLXNyYy5odG1sCisgICAgICAgICAgICAgICBtZWRpYS9h dWRpby1jb25zdHJ1Y3Rvci1wcmVsb2FkLmh0bWwKKyAgICAgICAgICAgICAgIG1lZGlhL2F1ZGlv LWNvbnRyb2xzLWRvLW5vdC1mYWRlLW91dC5odG1sCisgICAgICAgICAgICAgICBtZWRpYS9hdWRp by1jb250cm9scy1yZW5kZXJpbmcuaHRtbAorCisgICAgICAgICogYmluZGluZ3MvdjgvY3VzdG9t L1Y4RE9NV2luZG93Q3VzdG9tLmNwcDogUmVtb3ZlZCBhIGN1c3RvbSBnZXR0ZXIuCisgICAgICAg ICogcGFnZS9ET01XaW5kb3cuaWRsOiBSZW1vdmVkIFtDdXN0b21HZXR0ZXJdIElETCBmcm9tICdI VE1MQXVkaW9FbGVtZW50Q29uc3RydWN0b3InIG9mIFY4LiBSZW5hbWVkICdIVE1MQXVkaW9FbGVt ZW50Q29uc3RydWN0b3InIHRvICdIVE1MQXVkaW9FbGVtZW50Q29uc3RydWN0b3JDb25zdHJ1Y3Rv cicgaW4gb3JkZXIgdG8gbWFrZSAnbmV3IEF1ZGlvKCknIHVzZSB0aGUgdGVtcGxhdGUgb2YgKG5v dCAnSFRNTEF1ZGlvRWxlbWVudCcgYnV0KSAnSFRNTEF1ZGlvRWxlbWVudENvbnN0cnVjdG9yJy4g Tm90ZSB0aGF0IENvZGVHZW5lcmF0b3IqLnBtIHN0cmlwcyAvQ29uc3RydWN0b3IkLyBmcm9tIGFu IElETCB0eXBlLgorICAgICAgICAqIGJpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBt OgorICAgICAgICAoR2VuZXJhdGVJbXBsZW1lbnRhdGlvbik6IElmIGFuIElETCB0eXBlIGlzICdY WFhYQ29uc3RydWN0b3JDb25zdHJ1Y3RvcicsIHRoZW4gd2UgZG8gbm90IGFkZCBhIGhlYWRlciBm aWxlICdYWFhYQ29uc3RydWN0b3IuaCcgdG8gSlNET01XaW5kb3cuY3BwLCBiZWNhdXNlIHRoZSBO YW1lZENvbnN0cnVjdG9yIGRlY2xhcmF0aW9uIGlzIHdyaXR0ZW4gaW4gdGhlIGhlYWRlciBmaWxl IG9mIGNsYXNzIFhYWFggJ1hYWFguaCcuIEluY2lkZW50YWxseSwgdGhlIHJlYXNvbiB3aHkgQ29k ZUdlbmVyYXRvclY4LnBtIGRvZXMgbm90IG5lZWQgYSBjb3JyZXNwb25kaW5nIGNoYW5nZSBpcyB0 aGF0IFY4IGlzIHN0aWxsIGdlbmVyYXRpbmcgdGhlIE5hbWVkQ29uc3RydWN0b3IgZGVjbGFyYXRp b24gaW4gaXRzIGRlZGljYXRlZCBoZWFkZXIgZmlsZSAnWFhYWENvbnN0cnVjdG9yLmgnLiBUaGlz IFY4IGlzc3VlIHdpbGwgYmUgZml4ZWQgaW4gdGhlIHVwY29taW5nIHBhdGNoIHRoYXQgaW1wbGVt ZW50cyBbTmFtZWRDb25zdHJ1Y3Rvcl0gSURMIGluIFY4LgorCiAyMDExLTExLTAzICBGaWxpcCBQ aXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgSlNDIHNob3VsZCBiZSBhYmxlIHRv IHNhbXBsZSBpdHNlbGYgaW4gYSBtb3JlIGZsZXhpYmxlIHdheSB0aGFuIGp1c3Qgc2FtcGxpbmcg ZmxhZ3MKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdl bmVyYXRvckpTLnBtIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJh dG9ySlMucG0KaW5kZXggNDAzN2EzMjg1OTJkODU5N2U4ZDliZWZlYmQ5MDE3YWY4NGYzZDY3Ni4u YjkzZDZiNDBiOWM2YzAzYjk5YWIzMzYwZTBiYTJhZmRkZTJmNTY1OCAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG0KKysrIGIvU291 cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy9Db2RlR2VuZXJhdG9ySlMucG0KQEAgLTE4MTcs NyArMTgxNywxMCBAQCBzdWIgR2VuZXJhdGVJbXBsZW1lbnRhdGlvbgogICAgICAgICAgICAgICAg ICAgICAgICAgfSBlbHNpZiAoJGF0dHJpYnV0ZS0+c2lnbmF0dXJlLT50eXBlID1+IC9Db25zdHJ1 Y3RvciQvKSB7CiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbXkgJGNvbnN0cnVjdG9yVHlw ZSA9ICRhdHRyaWJ1dGUtPnNpZ25hdHVyZS0+dHlwZTsKICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAkY29uc3RydWN0b3JUeXBlID1+IHMvQ29uc3RydWN0b3IkLy87Ci0gICAgICAgICAgICAg ICAgICAgICAgICAgICAgaWYgKCRjb25zdHJ1Y3RvclR5cGUgbmUgIkRPTU9iamVjdCIpIHsKKyAg ICAgICAgICAgICAgICAgICAgICAgICAgICAjICRjb25zdHJ1Y3RvclR5cGUgfj0gL0NvbnN0cnVj dG9yJC8gaW5kaWNhdGVzIHRoYXQgaXQgaXMgTmFtZWRDb25zdHJ1Y3Rvci4KKyAgICAgICAgICAg ICAgICAgICAgICAgICAgICAjIFdlIGRvIG5vdCBnZW5lcmF0ZSB0aGUgaGVhZGVyIGZpbGUgZm9y IE5hbWVkQ29uc3RydWN0b3Igb2YgY2xhc3MgWFhYWCwKKyAgICAgICAgICAgICAgICAgICAgICAg ICAgICAjIHNpbmNlIHdlIGdlbmVyYXRlIHRoZSBOYW1lZENvbnN0cnVjdG9yIGRlY2xhcmF0aW9u IGludG8gdGhlIGhlYWRlciBmaWxlIG9mIGNsYXNzIFhYWFguCisgICAgICAgICAgICAgICAgICAg ICAgICAgICAgaWYgKCRjb25zdHJ1Y3RvclR5cGUgbmUgIkRPTU9iamVjdCIgYW5kICRjb25zdHJ1 Y3RvclR5cGUgIX4gL0NvbnN0cnVjdG9yJC8pIHsKICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgQWRkVG9JbXBsSW5jbHVkZXMoIkpTIiAuICRjb25zdHJ1Y3RvclR5cGUgLiAiLmgiLCAk YXR0cmlidXRlLT5zaWduYXR1cmUtPmV4dGVuZGVkQXR0cmlidXRlcy0+eyJDb25kaXRpb25hbCJ9 KTsKICAgICAgICAgICAgICAgICAgICAgICAgICAgICB9CiAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgcHVzaChAaW1wbENvbnRlbnQsICIgICAgLy8gU2hhZG93aW5nIGEgYnVpbHQtaW4gY29u c3RydWN0b3JcbiIpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvY3Vz dG9tL1Y4RE9NV2luZG93Q3VzdG9tLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L2N1 c3RvbS9WOERPTVdpbmRvd0N1c3RvbS5jcHAKaW5kZXggM2EzZTdhNTU1MDgwOGVhMWY1YjI3NjBm NDU0NTA2MTA2NWI0MTM3Yi4uZTVmYWYwYjAyNmFkNDlhZjJlODM4ZDIwYmEwZDMwMjdhMjQ5OGY4 OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvY3VzdG9tL1Y4RE9NV2lu ZG93Q3VzdG9tLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9jdXN0b20vVjhE T01XaW5kb3dDdXN0b20uY3BwCkBAIC01Niw3ICs1Niw2IEBACiAjaW5jbHVkZSAiVjhFdmVudExp c3RlbmVyLmgiCiAjaW5jbHVkZSAiVjhHQ0ZvckNvbnRleHREaXNwb3NlLmgiCiAjaW5jbHVkZSAi VjhIaWRkZW5Qcm9wZXJ0eU5hbWUuaCIKLSNpbmNsdWRlICJWOEhUTUxBdWRpb0VsZW1lbnRDb25z dHJ1Y3Rvci5oIgogI2luY2x1ZGUgIlY4SFRNTENvbGxlY3Rpb24uaCIKICNpbmNsdWRlICJWOEhU TUxJbWFnZUVsZW1lbnRDb25zdHJ1Y3Rvci5oIgogI2luY2x1ZGUgIlY4SFRNTE9wdGlvbkVsZW1l bnRDb25zdHJ1Y3Rvci5oIgpAQCAtMjI3LDE2ICsyMjYsNiBAQCB2b2lkIFY4RE9NV2luZG93Ojpv cGVuZXJBY2Nlc3NvclNldHRlcih2ODo6TG9jYWw8djg6OlN0cmluZz4gbmFtZSwgdjg6OkxvY2Fs PHY4OgogICAgIGluZm8uVGhpcygpLT5TZXQobmFtZSwgdmFsdWUpOwogfQogCi0jaWYgRU5BQkxF KFZJREVPKQotCi12ODo6SGFuZGxlPHY4OjpWYWx1ZT4gVjhET01XaW5kb3c6OkF1ZGlvQWNjZXNz b3JHZXR0ZXIodjg6OkxvY2FsPHY4OjpTdHJpbmc+IG5hbWUsIGNvbnN0IHY4OjpBY2Nlc3Nvcklu Zm8mIGluZm8pCi17Ci0gICAgRE9NV2luZG93KiB3aW5kb3cgPSBWOERPTVdpbmRvdzo6dG9OYXRp dmUoaW5mby5Ib2xkZXIoKSk7Ci0gICAgcmV0dXJuIFY4RE9NV3JhcHBlcjo6Z2V0Q29uc3RydWN0 b3IoJlY4SFRNTEF1ZGlvRWxlbWVudENvbnN0cnVjdG9yOjppbmZvLCB3aW5kb3cpOwotfQotCi0j ZW5kaWYKLQogdjg6OkhhbmRsZTx2ODo6VmFsdWU+IFY4RE9NV2luZG93OjpJbWFnZUFjY2Vzc29y R2V0dGVyKHY4OjpMb2NhbDx2ODo6U3RyaW5nPiBuYW1lLCBjb25zdCB2ODo6QWNjZXNzb3JJbmZv JiBpbmZvKQogewogICAgIERPTVdpbmRvdyogd2luZG93ID0gVjhET01XaW5kb3c6OnRvTmF0aXZl KGluZm8uSG9sZGVyKCkpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9ET01XaW5k b3cuaWRsIGIvU291cmNlL1dlYkNvcmUvcGFnZS9ET01XaW5kb3cuaWRsCmluZGV4IDkxMjcwMjdj OWQzOWQ2OGMxZDJmNzk3MGY0ZmY5ZjkwNGE2YTg0M2UuLmU4ODQ3ZGZkODczNzdjMWRhNzBlMGE2 Yjc5MmQxNTJkNTllNzc3MzkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NV2lu ZG93LmlkbAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL0RPTVdpbmRvdy5pZGwKQEAgLTYyNyw3 ICs2MjcsNyBAQCBtb2R1bGUgd2luZG93IHsKIAogICAgICAgICBhdHRyaWJ1dGUgU3RvcmFnZUNv bnN0cnVjdG9yIFN0b3JhZ2U7CiAKLSAgICAgICAgYXR0cmlidXRlIFtDdXN0b21HZXR0ZXIsIENv bmRpdGlvbmFsPVZJREVPLCBFbmFibGVkQXRSdW50aW1lXSBIVE1MQXVkaW9FbGVtZW50Q29uc3Ry dWN0b3IgQXVkaW87IC8vIFVzYWJsZSB3aXRoIHRoZSBuZXcgb3BlcmF0b3IKKyAgICAgICAgYXR0 cmlidXRlIFtKU0NDdXN0b21HZXR0ZXIsIENvbmRpdGlvbmFsPVZJREVPLCBFbmFibGVkQXRSdW50 aW1lXSBIVE1MQXVkaW9FbGVtZW50Q29uc3RydWN0b3JDb25zdHJ1Y3RvciBBdWRpbzsgLy8gVXNh YmxlIHdpdGggdGhlIG5ldyBvcGVyYXRvcgogICAgICAgICBhdHRyaWJ1dGUgW0NvbmRpdGlvbmFs PVZJREVPLCBFbmFibGVkQXRSdW50aW1lXSBIVE1MQXVkaW9FbGVtZW50Q29uc3RydWN0b3IgSFRN TEF1ZGlvRWxlbWVudDsKICAgICAgICAgYXR0cmlidXRlIFtDb25kaXRpb25hbD1WSURFTywgRW5h YmxlZEF0UnVudGltZV0gSFRNTE1lZGlhRWxlbWVudENvbnN0cnVjdG9yIEhUTUxNZWRpYUVsZW1l bnQ7CiAgICAgICAgIGF0dHJpYnV0ZSBbQ29uZGl0aW9uYWw9VklERU8sIEVuYWJsZWRBdFJ1bnRp bWVdIEhUTUxWaWRlb0VsZW1lbnRDb25zdHJ1Y3RvciBIVE1MVmlkZW9FbGVtZW50Owo=