149759 2015-10-02 12:30:43 -0700 JSBuiltinConstructor must always add builtin header 2015-10-06 03:17:20 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 calvaris calvaris benjamin cdumez commit-queue darin ggaren sam youennf oldest_to_newest 1130276 0 calvaris 2015-10-02 12:30:43 -0700 JSBuiltinConstructor must always add builtin header 1130277 1 262345 calvaris 2015-10-02 12:32:53 -0700 Created attachment 262345 Patch If you compile the previously generated file with the initialization function ready in the .js file, compilation was failing because the header file couldn't be found. It is actually needed because the builtins include already the initialization function. 1130562 2 youennf 2015-10-04 23:40:07 -0700 LGTM 1130571 3 262345 youennf 2015-10-05 01:02:27 -0700 Comment on attachment 262345 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262345&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5120 > + return 1; Just for style, returning 1 is not needed anymore. It might be better to refactor the whole routine accordingly. 1130573 4 calvaris 2015-10-05 01:24:36 -0700 (In reply to comment #3) > Comment on attachment 262345 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262345&action=review > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5120 > > + return 1; > > Just for style, returning 1 is not needed anymore. > It might be better to refactor the whole routine accordingly. I could rewrite to have a if-then and avoid that. I did it like this to pullute the patch less though it is not big deal, of course. What I can't avoid is the return inside the loop because it is supposed to bail out the first time it adds the header. 1130650 5 calvaris 2015-10-05 10:19:44 -0700 (In reply to comment #4) > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5120 > > > + return 1; > > > > Just for style, returning 1 is not needed anymore. > > It might be better to refactor the whole routine accordingly. > > I could rewrite to have a if-then and avoid that. I did it like this to > pollute the patch less though it is not big deal, of course. What I can't > avoid is the return inside the loop because it is supposed to bail out the > first time it adds the header. Darin, wdyt about this? Should I change this when landing or do you like it as it is now? 1130736 6 youennf 2015-10-05 14:24:46 -0700 One possibility for the refactoring would be to transform AddIncludesForJSBuiltinMethods into something like NeedsJSBuiltinsInclude. If you prefer to land this one directly, I can handle it in a follow-up patch. 1130909 7 262345 calvaris 2015-10-06 00:16:27 -0700 Comment on attachment 262345 Patch (In reply to comment #6) > One possibility for the refactoring would be to transform > AddIncludesForJSBuiltinMethods into something like NeedsJSBuiltinsInclude. > > If you prefer to land this one directly, I can handle it in a follow-up > patch. I can't do anything else than landing this since I'd need Darin's blessing for any other thing. 1130914 8 262345 commit-queue 2015-10-06 01:02:31 -0700 Comment on attachment 262345 Patch Clearing flags on attachment: 262345 Committed r190610: <http://trac.webkit.org/changeset/190610> 1130915 9 commit-queue 2015-10-06 01:02:38 -0700 All reviewed patches have been landed. Closing bug. 1130927 10 youennf 2015-10-06 03:17:20 -0700 Filed https://bugs.webkit.org/show_bug.cgi?id=149837 as a follow-up 262345 2015-10-02 12:32:53 -0700 2015-10-06 01:02:31 -0700 Patch bug-149759-20151002213226.patch text/plain 2288 calvaris U3VidmVyc2lvbiBSZXZpc2lvbjogMTkwNDU2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGJkODNiZDMxMmY2MTQ4 ZGVmYzNlNGVkOWE3NGQyNTQ5MjUxN2Y4NS4uMDcwYTBhZDQwNGQwOWI5Yzc0NjQ1MzljYmZkNjY2 MGFjNzI5ZDM2ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE1LTEwLTAyICBYYWJp ZXIgUm9kcmlndWV6IENhbHZhciAgPGNhbHZhcmlzQGlnYWxpYS5jb20+CisKKyAgICAgICAgSlNC dWlsdGluQ29uc3RydWN0b3IgbXVzdCBhbHdheXMgYWRkIGJ1aWx0aW4gaGVhZGVyCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDk3NTkKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDb3ZlcmVkIGJ5IFRlc3RK U0J1aWx0aW5Db25zdHJ1Y3Rvci5pZGwuCisKKyAgICAgICAgKiBiaW5kaW5ncy9zY3JpcHRzL0Nv ZGVHZW5lcmF0b3JKUy5wbToKKyAgICAgICAgKEFkZEluY2x1ZGVzRm9ySlNCdWlsdGluTWV0aG9k cyk6IEZvcmNlcyBhZGRpbmcgdGhlIGJ1aWx0aW4gaGVhZGVyIHdoZW4gdGhlIEpTQnVpbHRpbkNv bnN0cnVjdG9yIGlzIHByZXNlbnQuCisgICAgICAgICogYmluZGluZ3Mvc2NyaXB0cy90ZXN0L0pT L0pTVGVzdEpTQnVpbHRpbkNvbnN0cnVjdG9yLmNwcDogRXhwZWN0YXRpb24uCisKIDIwMTUtMTAt MDEgIENzYWJhIE9zenRyb2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9yZz4KIAogICAgICAgICBGaXgg dGhlICNpZiBndWFyZCBpbiBQb2ludGVyTG9ja0NvbnRyb2xsZXIuY3BwCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9zY3JpcHRzL0NvZGVHZW5lcmF0b3JKUy5wbSBiL1NvdXJj ZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCmluZGV4IDVkZmFj MzVkYjQ3ODI3YzVmOWRmZDcyOWU5ZWM5Yjg2YzBjOTU0NDYuLmRkYjM4NDA1OTI4MTAyNWY2MjIx ODlkYjk0NWY4MjkwNzk5NzFiMzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdz L3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCisrKyBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdz L3NjcmlwdHMvQ29kZUdlbmVyYXRvckpTLnBtCkBAIC01MTE1LDYgKzUxMTUsMTEgQEAgc3ViIEFk ZEluY2x1ZGVzRm9ySlNCdWlsdGluTWV0aG9kcygpCiB7CiAgICAgbXkgJGludGVyZmFjZSA9IHNo aWZ0OwogCisgICAgaWYgKCRpbnRlcmZhY2UtPmV4dGVuZGVkQXR0cmlidXRlcy0+eyJKU0J1aWx0 aW5Db25zdHJ1Y3RvciJ9KSB7CisgICAgICAgIEFkZFRvSW1wbEluY2x1ZGVzKCRpbnRlcmZhY2Ut Pm5hbWUgLiAiQnVpbHRpbnMuaCIpOworICAgICAgICByZXR1cm4gMTsKKyAgICB9CisKICAgICBm b3JlYWNoIG15ICRmdW5jdGlvbiAoQHskaW50ZXJmYWNlLT5mdW5jdGlvbnN9KSB7CiAgICAgICAg IG5leHQgdW5sZXNzICgkZnVuY3Rpb24tPnNpZ25hdHVyZS0+ZXh0ZW5kZWRBdHRyaWJ1dGVzLT57 IkpTQnVpbHRpbiJ9KTsKICAgICAgICAgbXkgJHNjb3BlTmFtZSA9ICRmdW5jdGlvbi0+c2lnbmF0 dXJlLT5leHRlbmRlZEF0dHJpYnV0ZXMtPnsiSW1wbGVtZW50ZWRCeSJ9OwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy90ZXN0L0pTL0pTVGVzdEpTQnVpbHRpbkNv bnN0cnVjdG9yLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3NjcmlwdHMvdGVzdC9KUy9K U1Rlc3RKU0J1aWx0aW5Db25zdHJ1Y3Rvci5jcHAKaW5kZXggMzY1OWIyNGY3N2MxZTZiYTQyZWVk YTIwNTYyNDFiY2NjZTFkZDA4Ni4uMWI3YWYyZGFjMWY4MmQ3YTk2NGQyZmY2NDAwMjlkZTUyNzc0 OThlNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3Mvc2NyaXB0cy90ZXN0L0pT L0pTVGVzdEpTQnVpbHRpbkNvbnN0cnVjdG9yLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9iaW5k aW5ncy9zY3JpcHRzL3Rlc3QvSlMvSlNUZXN0SlNCdWlsdGluQ29uc3RydWN0b3IuY3BwCkBAIC0y Miw2ICsyMiw3IEBACiAjaW5jbHVkZSAiSlNUZXN0SlNCdWlsdGluQ29uc3RydWN0b3IuaCIKIAog I2luY2x1ZGUgIkpTRE9NQmluZGluZy5oIgorI2luY2x1ZGUgIlRlc3RKU0J1aWx0aW5Db25zdHJ1 Y3RvckJ1aWx0aW5zLmgiCiAjaW5jbHVkZSA8d3RmL0dldFB0ci5oPgogCiB1c2luZyBuYW1lc3Bh Y2UgSlNDOwo=