101206 2012-11-05 03:49:27 -0800 [V8] toV8(impl) should return null if impl is 0 2012-11-05 10:41:43 -0800 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 haraken haraken abarth dglazkov japhet webkit.review.bot oldest_to_newest 758401 0 haraken 2012-11-05 03:49:27 -0800 toV8(impl) should return null if impl is 0. However, V8HTMLCollectionCustom::toV8() does not have the null check. I don't think there is a call path that hits the null check, we should add it because all other toV8()s have the null check. 758404 1 172311 haraken 2012-11-05 03:51:00 -0800 Created attachment 172311 Patch 758408 2 172314 haraken 2012-11-05 03:55:04 -0800 Created attachment 172314 Patch 758468 3 172314 webkit.review.bot 2012-11-05 06:10:14 -0800 Comment on attachment 172314 Patch Attachment 172314 did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14683224 New failing tests: storage/indexeddb/keyrange.html 758470 4 172323 haraken 2012-11-05 06:13:48 -0800 Created attachment 172323 Patch 758472 5 haraken 2012-11-05 06:15:09 -0800 I removed a change for IDBKeyCustom.cpp from the patch. I will address it in another patch. 758635 6 172323 abarth 2012-11-05 10:03:09 -0800 Comment on attachment 172323 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172323&action=review > Source/WebCore/ChangeLog:12 > + No tests. I think there will be no call path that hits the change. Should we add an ASSERT instead? 758637 7 haraken 2012-11-05 10:05:05 -0800 (In reply to comment #6) > (From update of attachment 172323 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172323&action=review > > > Source/WebCore/ChangeLog:12 > > + No tests. I think there will be no call path that hits the change. > > Should we add an ASSERT instead? That's possible. That being said, all other toV8() in CodeGeneratorV8.pm and custom bindings have 'if(!impl) return v8NullWithCheck(isolate)', which looks like an idiom. Both are OK to me. 758646 8 172323 abarth 2012-11-05 10:12:41 -0800 Comment on attachment 172323 Patch Yeah, we should be consistent with the generated code. 758676 9 172323 webkit.review.bot 2012-11-05 10:41:40 -0800 Comment on attachment 172323 Patch Clearing flags on attachment: 172323 Committed r133496: <http://trac.webkit.org/changeset/133496> 758677 10 webkit.review.bot 2012-11-05 10:41:43 -0800 All reviewed patches have been landed. Closing bug. 172311 2012-11-05 03:51:00 -0800 2012-11-05 03:55:01 -0800 Patch bug-101206-20121105124915.patch text/plain 1924 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzNDMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMThiOTEyNGVkZDU1ZTBk NGE0MGI4ZGJhZWE1YjE4MDFhMDc3MzMxMy4uM2JjZTI0NTIxZWEyNDUzNjdjYTFmZjJkOTljOTll YzRiZTUyYTlhMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIyIEBACiAyMDEyLTExLTA1ICBLZW50 YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KIAorICAgICAgICBbVjhdIHRvVjgoaW1w bCkgc2hvdWxkIHJldHVybiBudWxsIGlmIGltcGwgaXMgMAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxMjA2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgdG9WOChpbXBsKSBzaG91bGQgcmV0dXJuIG51bGwg aWYgaW1wbCBpcyAwLiBIb3dldmVyLAorICAgICAgICBWOEhUTUxDb2xsZWN0aW9uQ3VzdG9tOjp0 b1Y4KCkgZG9lcyBub3QgaGF2ZSB0aGUgbnVsbCBjaGVjay4KKyAgICAgICAgSSBkb24ndCB0aGlu ayB0aGVyZSBpcyBhIGNhbGwgcGF0aCB0aGF0IGhpdHMgdGhlIG51bGwgY2hlY2ssCisgICAgICAg IHdlIHNob3VsZCBhZGQgaXQgYmVjYXVzZSBhbGwgb3RoZXIgdG9WOCgpcyBoYXZlIHRoZSBudWxs IGNoZWNrLgorCisgICAgICAgIE5vIHRlc3RzLiBUaGVyZSB3aWxsIGJlIG5vIGNhbGwgcGF0aCB0 aGF0IGhpdHMgdGhlIG51bGwgY2hlY2suCisKKyAgICAgICAgKiBiaW5kaW5ncy92OC9jdXN0b20v VjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHA6CisgICAgICAgIChXZWJDb3JlOjp0b1Y4KToKKwor MjAxMi0xMS0wNSAgS2VudGFybyBIYXJhICA8aGFyYWtlbkBjaHJvbWl1bS5vcmc+CisKICAgICAg ICAgW1Y4XSBEaXNwb3NlKCkgYW5kIENsZWFyKCkgc2hvdWxkIGJlIGFsd2F5cyBjb3VwbGVkIGZv ciBzYWZldHkKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTEwMTE5MQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9jdXN0b20v VjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9j dXN0b20vVjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHAKaW5kZXggZGJmZjI5MjQ2MTkzZGM1MjRl YjI2OTNhOTA3NDgyZGE2MTg2ZjcwMC4uMDQ5MTVkNzkyYjFkYmE4NGVmYjVmOWMzMGExYzgxZTBh NDY4MzM4ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvY3VzdG9tL1Y4 SFRNTENvbGxlY3Rpb25DdXN0b20uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4 L2N1c3RvbS9WOEhUTUxDb2xsZWN0aW9uQ3VzdG9tLmNwcApAQCAtODYsNiArODYsOSBAQCB2ODo6 SGFuZGxlPHY4OjpWYWx1ZT4gVjhIVE1MQ29sbGVjdGlvbjo6bmFtZWRJdGVtQ2FsbGJhY2soY29u c3Qgdjg6OkFyZ3VtZW50cyYgYQogCiB2ODo6SGFuZGxlPHY4OjpWYWx1ZT4gdG9WOChIVE1MQ29s bGVjdGlvbiogaW1wbCwgdjg6OkhhbmRsZTx2ODo6T2JqZWN0PiBjcmVhdGlvbkNvbnRleHQsIHY4 OjpJc29sYXRlKiBpc29sYXRlKQogeworICAgIGlmICghaW1wbCkKKyAgICAgICAgcmV0dXJuIHY4 TnVsbFdpdGhDaGVjayhpc29sYXRlKTsKKwogICAgIGlmIChpbXBsLT50eXBlKCkgPT0gRG9jQWxs KQogICAgICAgICByZXR1cm4gdG9WOChzdGF0aWNfY2FzdDxIVE1MQWxsQ29sbGVjdGlvbio+KGlt cGwpLCBjcmVhdGlvbkNvbnRleHQsIGlzb2xhdGUpOwogICAgIHJldHVybiBWOEhUTUxDb2xsZWN0 aW9uOjp3cmFwKGltcGwsIGNyZWF0aW9uQ29udGV4dCwgaXNvbGF0ZSk7Cg== 172314 2012-11-05 03:55:04 -0800 2012-11-05 06:13:44 -0800 Patch bug-101206-20121105125318.patch text/plain 2539 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzNDMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMThiOTEyNGVkZDU1ZTBk NGE0MGI4ZGJhZWE1YjE4MDFhMDc3MzMxMy4uYzIwYzI0ZDkwMzc4ZDFjZmViYjExMDgyMDIyOTE5 ZTkxYTdkMTcyNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI0IEBACiAyMDEyLTExLTA1ICBLZW50 YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KIAorICAgICAgICBbVjhdIHRvVjgoaW1w bCkgc2hvdWxkIHJldHVybiBudWxsIGlmIGltcGwgaXMgMAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxMjA2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgdG9WOChpbXBsKSBzaG91bGQgcmV0dXJuIG51bGwg aWYgaW1wbCBpcyAwLiBIb3dldmVyLAorCisgICAgICAgIC0gVjhIVE1MQ29sbGVjdGlvbjo6dG9W OCgpIGRvZXMgbm90IGhhdmUgdGhlIG51bGwgY2hlY2suCisgICAgICAgIC0gVjhJREJLZXk6OnRv VjgoKSByZXR1cm5zIHVuZGVmaW5lZCBpbnN0ZWFkIG9mIG51bGwuCisKKyAgICAgICAgVGhpcyBw YXRjaCBmaXhlcyB0aGVtLiBBbGwgb3RoZXIgdG9WOCgpcyByZXR1cm4gbnVsbC4KKworICAgICAg ICBObyB0ZXN0cy4gSSB0aGluayB0aGVyZSB3aWxsIGJlIG5vIGNhbGwgcGF0aCB0aGF0IGhpdHMg dGhlIGNoYW5nZS4KKworICAgICAgICAqIGJpbmRpbmdzL3Y4L2N1c3RvbS9WOEhUTUxDb2xsZWN0 aW9uQ3VzdG9tLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnRvVjgpOgorCisyMDEyLTExLTA1ICBL ZW50YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KKwogICAgICAgICBbVjhdIERpc3Bv c2UoKSBhbmQgQ2xlYXIoKSBzaG91bGQgYmUgYWx3YXlzIGNvdXBsZWQgZm9yIHNhZmV0eQogICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxMTkxCiAKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L2N1c3RvbS9WOEhUTUxDb2xsZWN0 aW9uQ3VzdG9tLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L2N1c3RvbS9WOEhUTUxD b2xsZWN0aW9uQ3VzdG9tLmNwcAppbmRleCBkYmZmMjkyNDYxOTNkYzUyNGViMjY5M2E5MDc0ODJk YTYxODZmNzAwLi4wNDkxNWQ3OTJiMWRiYTg0ZWZiNWY5YzMwYTFjODFlMGE0NjgzMzhlIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9jdXN0b20vVjhIVE1MQ29sbGVjdGlv bkN1c3RvbS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvY3VzdG9tL1Y4SFRN TENvbGxlY3Rpb25DdXN0b20uY3BwCkBAIC04Niw2ICs4Niw5IEBAIHY4OjpIYW5kbGU8djg6OlZh bHVlPiBWOEhUTUxDb2xsZWN0aW9uOjpuYW1lZEl0ZW1DYWxsYmFjayhjb25zdCB2ODo6QXJndW1l bnRzJiBhCiAKIHY4OjpIYW5kbGU8djg6OlZhbHVlPiB0b1Y4KEhUTUxDb2xsZWN0aW9uKiBpbXBs LCB2ODo6SGFuZGxlPHY4OjpPYmplY3Q+IGNyZWF0aW9uQ29udGV4dCwgdjg6Oklzb2xhdGUqIGlz b2xhdGUpCiB7CisgICAgaWYgKCFpbXBsKQorICAgICAgICByZXR1cm4gdjhOdWxsV2l0aENoZWNr KGlzb2xhdGUpOworCiAgICAgaWYgKGltcGwtPnR5cGUoKSA9PSBEb2NBbGwpCiAgICAgICAgIHJl dHVybiB0b1Y4KHN0YXRpY19jYXN0PEhUTUxBbGxDb2xsZWN0aW9uKj4oaW1wbCksIGNyZWF0aW9u Q29udGV4dCwgaXNvbGF0ZSk7CiAgICAgcmV0dXJuIFY4SFRNTENvbGxlY3Rpb246OndyYXAoaW1w bCwgY3JlYXRpb25Db250ZXh0LCBpc29sYXRlKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L2JpbmRpbmdzL3Y4L2N1c3RvbS9WOElEQktleUN1c3RvbS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9i aW5kaW5ncy92OC9jdXN0b20vVjhJREJLZXlDdXN0b20uY3BwCmluZGV4IGZjMTE2YTk4NjEzY2Qx YzQwNjA3M2EzNDBkMDc2MDc0MTRjNzI1MzAuLmY2Yzg1NWUwYzhiNmNiOWFhY2JhMWNhNWJiZmQ2 NjlkYTY2MzQyYWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L2N1c3Rv bS9WOElEQktleUN1c3RvbS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvY3Vz dG9tL1Y4SURCS2V5Q3VzdG9tLmNwcApAQCAtNDEsNyArNDEsNyBAQCBuYW1lc3BhY2UgV2ViQ29y ZSB7CiB2ODo6SGFuZGxlPHY4OjpWYWx1ZT4gdG9WOChJREJLZXkqIGtleSwgdjg6OkhhbmRsZTx2 ODo6T2JqZWN0PiBjcmVhdGlvbkNvbnRleHQsIHY4OjpJc29sYXRlKiBpc29sYXRlKQogewogICAg IGlmICgha2V5KQotICAgICAgICByZXR1cm4gdjhVbmRlZmluZWQoKTsKKyAgICAgICAgcmV0dXJu IHY4TnVsbFdpdGhDaGVjayhpc29sYXRlKTsKIAogICAgIHN3aXRjaCAoa2V5LT50eXBlKCkpIHsK ICAgICBjYXNlIElEQktleTo6SW52YWxpZFR5cGU6Cg== 172323 2012-11-05 06:13:48 -0800 2012-11-05 10:41:40 -0800 Patch bug-101206-20121105151202.patch text/plain 1819 haraken U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzNDMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMThiOTEyNGVkZDU1ZTBk NGE0MGI4ZGJhZWE1YjE4MDFhMDc3MzMxMy4uNjAzOGM4MzM0ZjFjNTI5MjBhMGIxZGFlNDEyZTM3 YTc3YmUyZTBhYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIxIEBACiAyMDEyLTExLTA1ICBLZW50 YXJvIEhhcmEgIDxoYXJha2VuQGNocm9taXVtLm9yZz4KIAorICAgICAgICBbVjhdIHRvVjgoaW1w bCkgc2hvdWxkIHJldHVybiBudWxsIGlmIGltcGwgaXMgMAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxMjA2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgdG9WOChpbXBsKSBzaG91bGQgcmV0dXJuIG51bGwg aWYgaW1wbCBpcyAwLiBIb3dldmVyLAorICAgICAgICBWOEhUTUxDb2xsZWN0aW9uOjp0b1Y4KCkg ZG9lcyBub3QgaGF2ZSB0aGUgbnVsbCBjaGVjay4KKyAgICAgICAgQWxsIG90aGVyIHRvVjgoKXMg cmV0dXJuIG51bGwuCisKKyAgICAgICAgTm8gdGVzdHMuIEkgdGhpbmsgdGhlcmUgd2lsbCBiZSBu byBjYWxsIHBhdGggdGhhdCBoaXRzIHRoZSBjaGFuZ2UuCisKKyAgICAgICAgKiBiaW5kaW5ncy92 OC9jdXN0b20vVjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHA6CisgICAgICAgIChXZWJDb3JlOjp0 b1Y4KToKKworMjAxMi0xMS0wNSAgS2VudGFybyBIYXJhICA8aGFyYWtlbkBjaHJvbWl1bS5vcmc+ CisKICAgICAgICAgW1Y4XSBEaXNwb3NlKCkgYW5kIENsZWFyKCkgc2hvdWxkIGJlIGFsd2F5cyBj b3VwbGVkIGZvciBzYWZldHkKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEwMTE5MQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92 OC9jdXN0b20vVjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5k aW5ncy92OC9jdXN0b20vVjhIVE1MQ29sbGVjdGlvbkN1c3RvbS5jcHAKaW5kZXggZGJmZjI5MjQ2 MTkzZGM1MjRlYjI2OTNhOTA3NDgyZGE2MTg2ZjcwMC4uMDQ5MTVkNzkyYjFkYmE4NGVmYjVmOWMz MGExYzgxZTBhNDY4MzM4ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3Mvdjgv Y3VzdG9tL1Y4SFRNTENvbGxlY3Rpb25DdXN0b20uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Jp bmRpbmdzL3Y4L2N1c3RvbS9WOEhUTUxDb2xsZWN0aW9uQ3VzdG9tLmNwcApAQCAtODYsNiArODYs OSBAQCB2ODo6SGFuZGxlPHY4OjpWYWx1ZT4gVjhIVE1MQ29sbGVjdGlvbjo6bmFtZWRJdGVtQ2Fs bGJhY2soY29uc3Qgdjg6OkFyZ3VtZW50cyYgYQogCiB2ODo6SGFuZGxlPHY4OjpWYWx1ZT4gdG9W OChIVE1MQ29sbGVjdGlvbiogaW1wbCwgdjg6OkhhbmRsZTx2ODo6T2JqZWN0PiBjcmVhdGlvbkNv bnRleHQsIHY4OjpJc29sYXRlKiBpc29sYXRlKQogeworICAgIGlmICghaW1wbCkKKyAgICAgICAg cmV0dXJuIHY4TnVsbFdpdGhDaGVjayhpc29sYXRlKTsKKwogICAgIGlmIChpbXBsLT50eXBlKCkg PT0gRG9jQWxsKQogICAgICAgICByZXR1cm4gdG9WOChzdGF0aWNfY2FzdDxIVE1MQWxsQ29sbGVj dGlvbio+KGltcGwpLCBjcmVhdGlvbkNvbnRleHQsIGlzb2xhdGUpOwogICAgIHJldHVybiBWOEhU TUxDb2xsZWN0aW9uOjp3cmFwKGltcGwsIGNyZWF0aW9uQ29udGV4dCwgaXNvbGF0ZSk7Cg==