132228 2014-04-27 01:53:11 -0700 [GTK] TextTrack kind and mode attributes are enums since r166180 2014-04-28 01:07:29 -0700 1 1 1 Unclassified WebKit Bindings 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Gtk P2 Normal --- 131672 1 cgarcia webkit-unassigned gustavo pnormand oldest_to_newest 1004389 0 cgarcia 2014-04-27 01:53:11 -0700 We don't support enum values yet in GObject DOM bindings, but they are internally strings anyway, so we can keep the old implementations using strings as custom functions until we properly support enums. 1004390 1 230257 cgarcia 2014-04-27 01:54:56 -0700 Created attachment 230257 Patch 1004398 2 230257 mrobinson 2014-04-27 04:07:43 -0700 Comment on attachment 230257 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230257&action=review Looks good with a few minor suggestions. > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:74 > + gchar* result = convertToUTF8String(item->kind()); > + return result; Can you just return convertToUTF8String(item->kind()); ? > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:103 > + WebCore::TextTrack* item = WebKit::core(self); > + WTF::String convertedValue = WTF::String::fromUTF8(value); > + item->setMode(convertedValue); Can't this just be WebKit::core(self)->setMode(WTF::String::fromUTF8(value)); ? 1004495 3 cgarcia 2014-04-28 00:11:27 -0700 (In reply to comment #2) > (From update of attachment 230257 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230257&action=review > > Looks good with a few minor suggestions. Thanks > > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:74 > > + gchar* result = convertToUTF8String(item->kind()); > > + return result; > > Can you just return convertToUTF8String(item->kind()); ? Sure, this is actually generated code, but added manually. I think this happens in the generated code because if the method raises exceptions there's code to deal with them, between the two lines. > > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:103 > > + WebCore::TextTrack* item = WebKit::core(self); > > + WTF::String convertedValue = WTF::String::fromUTF8(value); > > + item->setMode(convertedValue); > > Can't this just be WebKit::core(self)->setMode(WTF::String::fromUTF8(value)); ? Ditto. 1004502 4 cgarcia 2014-04-28 01:07:29 -0700 Committed r167876: <http://trac.webkit.org/changeset/167876> 230257 2014-04-27 01:54:56 -0700 2014-04-27 04:07:42 -0700 Patch wkdom-text-track-enums.diff text/plain 5217 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBlNThjYmRlLi40ZmIzYTNkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjQg QEAKIDIwMTQtMDQtMjcgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgogCisgICAgICAgIFtHVEtdIFRleHRUcmFjayBraW5kIGFuZCBtb2RlIGF0dHJpYnV0ZXMgYXJl IGVudW1zIHNpbmNlIHIxNjYxODAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTEzMjIyOAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFdlIGRvbid0IHN1cHBvcnQgZW51bSB2YWx1ZXMgeWV0IGluIEdPYmplY3Qg RE9NIGJpbmRpbmdzLCBidXQgdGhleQorICAgICAgICBhcmUgaW50ZXJuYWxseSBzdHJpbmdzIGFu eXdheSwgc28gd2UgY2FuIGtlZXAgdGhlIG9sZAorICAgICAgICBpbXBsZW1lbnRhdGlvbnMgdXNp bmcgc3RyaW5ncyBhcyBjdXN0b20gZnVuY3Rpb25zIHVudGlsIHdlCisgICAgICAgIHByb3Blcmx5 IHN1cHBvcnQgZW51bXMuCisKKyAgICAgICAgKiBiaW5kaW5ncy9nb2JqZWN0L1dlYktpdERPTUN1 c3RvbS5jcHA6CisgICAgICAgICh3ZWJraXRfZG9tX3RleHRfdHJhY2tfZ2V0X2tpbmQpOgorICAg ICAgICAod2Via2l0X2RvbV90ZXh0X3RyYWNrX2dldF9tb2RlKToKKyAgICAgICAgKHdlYmtpdF9k b21fdGV4dF90cmFja19zZXRfbW9kZSk6CisgICAgICAgICogYmluZGluZ3MvZ29iamVjdC9XZWJL aXRET01DdXN0b20uaDoKKyAgICAgICAgKiBiaW5kaW5ncy9nb2JqZWN0L1dlYktpdERPTUN1c3Rv bS5zeW1ib2xzOgorCisyMDE0LTA0LTI3ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFA aWdhbGlhLmNvbT4KKwogICAgICAgICBbR1RLXSBUZXh0VHJhY2s6OmFkZEN1ZSBjYW4gcmFpc2Ug YW4gZXhjZXB0aW9uIHNpbmNlIHIxNjM5NzQKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTEzMjIyNwogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy9nb2JqZWN0L1dlYktpdERPTUN1c3RvbS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5k aW5ncy9nb2JqZWN0L1dlYktpdERPTUN1c3RvbS5jcHAKaW5kZXggMGM0MGQ3Ny4uZDc4NzM4MCAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvZ29iamVjdC9XZWJLaXRET01DdXN0 b20uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2dvYmplY3QvV2ViS2l0RE9NQ3Vz dG9tLmNwcApAQCAtMjgsNiArMjgsMTEgQEAKICNpbmNsdWRlICJXZWJLaXRET01Qcml2YXRlLmgi CiAjaW5jbHVkZSAiZ29iamVjdC9Db252ZXJ0VG9VVEY4U3RyaW5nLmgiCiAKKyNpZiBFTkFCTEUo VklERU8pCisjaW5jbHVkZSAiVGV4dFRyYWNrLmgiCisjaW5jbHVkZSAiV2ViS2l0RE9NVGV4dFRy YWNrUHJpdmF0ZS5oIgorI2VuZGlmCisKIHVzaW5nIG5hbWVzcGFjZSBXZWJLaXQ7CiAKIGdib29s ZWFuIHdlYmtpdF9kb21faHRtbF90ZXh0X2FyZWFfZWxlbWVudF9pc19lZGl0ZWQoV2ViS2l0RE9N SFRNTFRleHRBcmVhRWxlbWVudCogYXJlYSkKQEAgLTU2LDQgKzYxLDQ3IEBAIHZvaWQgd2Via2l0 X2RvbV9odG1sX21lZGlhX2VsZW1lbnRfc2V0X2N1cnJlbnRfdGltZShXZWJLaXRET01IVE1MTWVk aWFFbGVtZW50KiBzCiAjZW5kaWYgLyogRU5BQkxFKFZJREVPKSAqLwogfQogCisvLyBXZWJLaXRE T01UZXh0VHJhY2s6a2luZCBhbmQgV2ViS2l0RE9NVGV4dFRyYWNrOm1vZGUgd2VyZSBjb252ZXJ0 ZWQgaW50byBlbnVtcyBpbiByMTY2MTgwLgorLy8gU2luY2Ugd2UgZG9uJ3Qgc3VwcG9ydCBlbnVt cyBpbiBvdXIgYmluZGluZ3MgeWV0LCBrZWVwIHRoZXNlIGltcGxlbWVudGF0aW9uIGhlcmUgYXMg Y3VzdG9tIHVudGlsCisvLyB3ZSBhZGQgcHJvcGVyIHN1cHBvcnQgZm9yIGVudW1zLgorZ2NoYXIq IHdlYmtpdF9kb21fdGV4dF90cmFja19nZXRfa2luZChXZWJLaXRET01UZXh0VHJhY2sqIHNlbGYp Cit7CisjaWYgRU5BQkxFKFZJREVPX1RSQUNLKQorICAgIFdlYkNvcmU6OkpTTWFpblRocmVhZE51 bGxTdGF0ZSBzdGF0ZTsKKyAgICBnX3JldHVybl92YWxfaWZfZmFpbChXRUJLSVRfRE9NX0lTX1RF WFRfVFJBQ0soc2VsZiksIDApOworICAgIFdlYkNvcmU6OlRleHRUcmFjayogaXRlbSA9IFdlYktp dDo6Y29yZShzZWxmKTsKKyAgICBnY2hhciogcmVzdWx0ID0gY29udmVydFRvVVRGOFN0cmluZyhp dGVtLT5raW5kKCkpOworICAgIHJldHVybiByZXN1bHQ7CisjZWxzZQorICAgIFdFQktJVF9XQVJO X0ZFQVRVUkVfTk9UX1BSRVNFTlQoIlZpZGVvIFRyYWNrIikKKyAgICByZXR1cm4gMDsKKyNlbmRp ZiAvKiBFTkFCTEUoVklERU9fVFJBQ0spICovCit9CiAKK2djaGFyKiB3ZWJraXRfZG9tX3RleHRf dHJhY2tfZ2V0X21vZGUoV2ViS2l0RE9NVGV4dFRyYWNrKiBzZWxmKQoreworI2lmIEVOQUJMRShW SURFT19UUkFDSykKKyAgICBXZWJDb3JlOjpKU01haW5UaHJlYWROdWxsU3RhdGUgc3RhdGU7Cisg ICAgZ19yZXR1cm5fdmFsX2lmX2ZhaWwoV0VCS0lUX0RPTV9JU19URVhUX1RSQUNLKHNlbGYpLCAw KTsKKyAgICBXZWJDb3JlOjpUZXh0VHJhY2sqIGl0ZW0gPSBXZWJLaXQ6OmNvcmUoc2VsZik7Cisg ICAgZ2NoYXIqIHJlc3VsdCA9IGNvbnZlcnRUb1VURjhTdHJpbmcoaXRlbS0+bW9kZSgpKTsKKyAg ICByZXR1cm4gcmVzdWx0OworI2Vsc2UKKyAgICBXRUJLSVRfV0FSTl9GRUFUVVJFX05PVF9QUkVT RU5UKCJWaWRlbyBUcmFjayIpCisgICAgcmV0dXJuIDA7CisjZW5kaWYgLyogRU5BQkxFKFZJREVP X1RSQUNLKSAqLworfQorCit2b2lkIHdlYmtpdF9kb21fdGV4dF90cmFja19zZXRfbW9kZShXZWJL aXRET01UZXh0VHJhY2sqIHNlbGYsIGNvbnN0IGdjaGFyKiB2YWx1ZSkKK3sKKyNpZiBFTkFCTEUo VklERU9fVFJBQ0spCisgICAgV2ViQ29yZTo6SlNNYWluVGhyZWFkTnVsbFN0YXRlIHN0YXRlOwor ICAgIGdfcmV0dXJuX2lmX2ZhaWwoV0VCS0lUX0RPTV9JU19URVhUX1RSQUNLKHNlbGYpKTsKKyAg ICBnX3JldHVybl9pZl9mYWlsKHZhbHVlKTsKKyAgICBXZWJDb3JlOjpUZXh0VHJhY2sqIGl0ZW0g PSBXZWJLaXQ6OmNvcmUoc2VsZik7CisgICAgV1RGOjpTdHJpbmcgY29udmVydGVkVmFsdWUgPSBX VEY6OlN0cmluZzo6ZnJvbVVURjgodmFsdWUpOworICAgIGl0ZW0tPnNldE1vZGUoY29udmVydGVk VmFsdWUpOworI2Vsc2UKKyAgICBXRUJLSVRfV0FSTl9GRUFUVVJFX05PVF9QUkVTRU5UKCJWaWRl byBUcmFjayIpCisjZW5kaWYgLyogRU5BQkxFKFZJREVPX1RSQUNLKSAqLworfQpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvZ29iamVjdC9XZWJLaXRET01DdXN0b20uaCBiL1Nv dXJjZS9XZWJDb3JlL2JpbmRpbmdzL2dvYmplY3QvV2ViS2l0RE9NQ3VzdG9tLmgKaW5kZXggYWVk NTEyOS4uMWQwNTBiNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvZ29iamVj dC9XZWJLaXRET01DdXN0b20uaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9nb2JqZWN0 L1dlYktpdERPTUN1c3RvbS5oCkBAIC01MCw2ICs1MCwzMyBAQCBXRUJLSVRfQVBJIHZvaWQgd2Vi a2l0X2RvbV9odG1sX21lZGlhX2VsZW1lbnRfc2V0X2N1cnJlbnRfdGltZShXZWJLaXRET01IVE1M TWVkaQogICovCiBXRUJLSVRfQVBJIGdib29sZWFuIHdlYmtpdF9kb21faHRtbF9pbnB1dF9lbGVt ZW50X2lzX2VkaXRlZChXZWJLaXRET01IVE1MSW5wdXRFbGVtZW50KiBpbnB1dCk7CiAKKy8qKgor ICogd2Via2l0X2RvbV90ZXh0X3RyYWNrX2dldF9raW5kOgorICogQHNlbGY6IEEgI1dlYktpdERP TVRleHRUcmFjaworICoKKyAqIFJldHVybnM6IEEgI2djaGFyCisgKi8KK1dFQktJVF9BUEkgZ2No YXIqCit3ZWJraXRfZG9tX3RleHRfdHJhY2tfZ2V0X2tpbmQoV2ViS2l0RE9NVGV4dFRyYWNrKiBz ZWxmKTsKKworLyoqCisgKiB3ZWJraXRfZG9tX3RleHRfdHJhY2tfZ2V0X21vZGU6CisgKiBAc2Vs ZjogQSAjV2ViS2l0RE9NVGV4dFRyYWNrCisgKgorICogUmV0dXJuczogQSAjZ2NoYXIKKyAqLwor V0VCS0lUX0FQSSBnY2hhcioKK3dlYmtpdF9kb21fdGV4dF90cmFja19nZXRfbW9kZShXZWJLaXRE T01UZXh0VHJhY2sqIHNlbGYpOworCisvKioKKyAqIHdlYmtpdF9kb21fdGV4dF90cmFja19zZXRf bW9kZToKKyAqIEBzZWxmOiBBICNXZWJLaXRET01UZXh0VHJhY2sKKyAqIEB2YWx1ZTogQSAjZ2No YXIKKyAqCisgKi8KK1dFQktJVF9BUEkgdm9pZAord2Via2l0X2RvbV90ZXh0X3RyYWNrX3NldF9t b2RlKFdlYktpdERPTVRleHRUcmFjayogc2VsZiwgY29uc3QgZ2NoYXIqIHZhbHVlKTsKKwogR19F TkRfREVDTFMKIAogI2VuZGlmCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9n b2JqZWN0L1dlYktpdERPTUN1c3RvbS5zeW1ib2xzIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3Mv Z29iamVjdC9XZWJLaXRET01DdXN0b20uc3ltYm9scwppbmRleCA5MjAyOWI0Li40M2E1NjViIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9nb2JqZWN0L1dlYktpdERPTUN1c3Rv bS5zeW1ib2xzCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2dvYmplY3QvV2ViS2l0RE9N Q3VzdG9tLnN5bWJvbHMKQEAgLTEsMyArMSw2IEBACiBnYm9vbGVhbiB3ZWJraXRfZG9tX2h0bWxf dGV4dF9hcmVhX2VsZW1lbnRfaXNfZWRpdGVkKFdlYktpdERPTUhUTUxUZXh0QXJlYUVsZW1lbnQq KQogZ2Jvb2xlYW4gd2Via2l0X2RvbV9odG1sX2lucHV0X2VsZW1lbnRfaXNfZWRpdGVkKFdlYktp dERPTUhUTUxJbnB1dEVsZW1lbnQqKQogdm9pZCB3ZWJraXRfZG9tX2h0bWxfbWVkaWFfZWxlbWVu dF9zZXRfY3VycmVudF90aW1lKFdlYktpdERPTUhUTUxNZWRpYUVsZW1lbnQqLCBnZG91YmxlLCBH RXJyb3IqKikKK3ZvaWQgd2Via2l0X2RvbV90ZXh0X3RyYWNrX3NldF9tb2RlKFdlYktpdERPTVRl eHRUcmFjayosIGNvbnN0IGdjaGFyKikKK2djaGFyKiB3ZWJraXRfZG9tX3RleHRfdHJhY2tfZ2V0 X21vZGUoV2ViS2l0RE9NVGV4dFRyYWNrKikKK2djaGFyKiB3ZWJraXRfZG9tX3RleHRfdHJhY2tf Z2V0X2tpbmQoV2ViS2l0RE9NVGV4dFRyYWNrKikK