119725 2013-08-12 20:57:53 -0700 Simplify RadioNodeList's anonymous indexed getter 2013-08-13 09:13:39 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED BlinkMergeCandidate P2 Normal --- 1 rniwa cdumez cdumez commit-queue esprehn+autocc ggaren kling koivisto kondapallykalyan oliver oldest_to_newest 916527 0 rniwa 2013-08-12 20:57:53 -0700 Consider merging https://chromium.googlesource.com/chromium/blink/+/c06e3ea129f0a16eb4275a2c6bb9e77094bda632 Make RadioNodeList's anonymous indexed getter argument mandatory and drop the [IsIndex] IDL extended attribute. 'optional' and [IsIndex] have no effect on the generated code of anonymous indexed getters: - There is no way for the caller to omit the index - The generated code uses ObjectTemplate::SetIndexedPropertyHandler() and the IndexedPropertyGetterCallback takes a uint32_t in argument. There is no web-exposed behavior change, the generated bindings code is identical. 916617 1 208608 cdumez 2013-08-13 01:38:50 -0700 Created attachment 208608 Patch 916628 2 208608 rniwa 2013-08-13 02:02:09 -0700 Comment on attachment 208608 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208608&action=review > Source/WebCore/ChangeLog:12 > + This change has strictly no impact on the generated bindings code > + but it simplifies the IDL. Doesn't IsIndex cause INDEX_SIZE_ERR to be thrown when the index is negative? 916635 3 cdumez 2013-08-13 02:49:32 -0700 (In reply to comment #2) > (From update of attachment 208608 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208608&action=review > > > Source/WebCore/ChangeLog:12 > > + This change has strictly no impact on the generated bindings code > > + but it simplifies the IDL. > > Doesn't IsIndex cause INDEX_SIZE_ERR to be thrown when the index is negative? In theory, yes but not for anonymous indexed getters. As I said in the Changelog, the generated code is *exactly* the same. Also note that throwing an INDEX_SIZE_ERR for negative values in indexed getter would not be according to specification. 916737 4 208608 commit-queue 2013-08-13 09:13:36 -0700 Comment on attachment 208608 Patch Clearing flags on attachment: 208608 Committed r154003: <http://trac.webkit.org/changeset/154003> 916738 5 commit-queue 2013-08-13 09:13:39 -0700 All reviewed patches have been landed. Closing bug. 208608 2013-08-13 01:38:50 -0700 2013-08-13 09:13:35 -0700 Patch bug-119725-20130813113848.patch text/plain 1437 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTUzOTkyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggY2JmYTMxMzQxYzkwZGM0 N2E3Y2M0ZjNjNTRhNzU1YWY3ZmFkYTIzZS4uNjM4NDRlMTcwZmMwMzY1ZTc2MGY2YjlmOTNmMmJi M2Q3NWE0NWI4NiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEzLTA4LTEzICBDaHJp c3RvcGhlIER1bWV6ICA8Y2guZHVtZXpAc2lzYS5zYW1zdW5nLmNvbT4KKworICAgICAgICBTaW1w bGlmeSBSYWRpb05vZGVMaXN0J3MgYW5vbnltb3VzIGluZGV4ZWQgZ2V0dGVyCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTk3MjUKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTaW1wbGlmeSBSYWRpb05vZGVM aXN0J3MgYW5vbnltb3VzIGluZGV4ZWQgZ2V0dGVyIGJ5IGRyb3BwaW5nIFtJc0luZGV4XQorICAg ICAgICBJREwgZXh0ZW5kZWQgYXR0cmlidXRlIGZyb20gaXRzIHBhcmFtZXRlciBhbmQgbWFraW5n IGl0IG1hbmRhdG9yeS4KKworICAgICAgICBUaGlzIGNoYW5nZSBoYXMgc3RyaWN0bHkgbm8gaW1w YWN0IG9uIHRoZSBnZW5lcmF0ZWQgYmluZGluZ3MgY29kZQorICAgICAgICBidXQgaXQgc2ltcGxp ZmllcyB0aGUgSURMLgorCisgICAgICAgIE5vIG5ldyB0ZXN0LCBubyBiZWhhdmlvciBjaGFuZ2Uu CisKKyAgICAgICAgKiBodG1sL1JhZGlvTm9kZUxpc3QuaWRsOgorCiAyMDEzLTA4LTEzICBBbmRy ZWkgQnVjdXIgIDxhYnVjdXJAYWRvYmUuY29tPgogCiAgICAgICAgIFtDU1MgUmVnaW9uc10gQ29t cHV0ZSBjb3JyZWN0IHJlZ2lvbiByYW5nZXMgZm9yIGJveGVzCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9odG1sL1JhZGlvTm9kZUxpc3QuaWRsIGIvU291cmNlL1dlYkNvcmUvaHRtbC9SYWRp b05vZGVMaXN0LmlkbAppbmRleCA2ZTM0OGU0OTg3NzkyMjg3ZGQ5MzM2MDBjNDk0ODExNzIzNGFl ODkzLi5jMGVkMzJjN2NhODI3NGM2ZDMxY2M0YjNjNWFlODNiNGU1NzMzMGRkIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9odG1sL1JhZGlvTm9kZUxpc3QuaWRsCisrKyBiL1NvdXJjZS9XZWJD b3JlL2h0bWwvUmFkaW9Ob2RlTGlzdC5pZGwKQEAgLTI5LDUgKzI5LDUgQEAKIF0gaW50ZXJmYWNl IFJhZGlvTm9kZUxpc3QgOiBOb2RlTGlzdCB7CiAgICAgYXR0cmlidXRlIERPTVN0cmluZyB2YWx1 ZTsKIAotICAgIGdldHRlciBOb2RlIChbSXNJbmRleCxEZWZhdWx0PVVuZGVmaW5lZF0gb3B0aW9u YWwgdW5zaWduZWQgbG9uZyBpbmRleCk7CisgICAgZ2V0dGVyIE5vZGUgKHVuc2lnbmVkIGxvbmcg aW5kZXgpOwogfTsK