81466 2012-03-17 22:36:13 -0700 Do not copy the script source in the SourceProvider, just reference the existing string 2012-05-07 12:14:59 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 benjamin benjamin barraclough ggaren jberlin yong.li.webkit oldest_to_newest 581418 0 benjamin 2012-03-17 22:36:13 -0700 When executing scripts, we copy giant strings in the SourceProvider. That is not good and very simple to fix. 581419 1 132483 benjamin 2012-03-17 22:40:07 -0700 Created attachment 132483 Patch 581595 2 132483 ap 2012-03-18 20:26:15 -0700 Comment on attachment 132483 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132483&action=review > Source/JavaScriptCore/parser/SourceProvider.h:89 > + UString getRange(int start, int end) const OVERRIDE If it's OVERRIDE, it also needs to be virtual. Although it can be implicit in C++, we prefer to spell it out. > Source/WebCore/bindings/js/StringSourceProvider.h:45 > + JSC::UString getRange(int start, int end) const OVERRIDE Ditto. 582054 3 132483 buildbot 2012-03-19 12:29:56 -0700 Comment on attachment 132483 Patch Attachment 132483 did not pass win-ews (win): Output: http://queues.webkit.org/results/11986329 582165 4 132483 ggaren 2012-03-19 13:40:54 -0700 Comment on attachment 132483 Patch r=me Please add "virtual" to these declarations, as Alexey suggested. 582758 5 benjamin 2012-03-19 22:27:02 -0700 Committed r111358: <http://trac.webkit.org/changeset/111358> 582783 6 jberlin 2012-03-19 23:00:38 -0700 (In reply to comment #5) > Committed r111358: <http://trac.webkit.org/changeset/111358> This broke the Mac build: http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29/builds/7932/steps/compile-webkit/logs/stdio 582790 7 benjamin 2012-03-19 23:18:51 -0700 > This broke the Mac build: > > http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29/builds/7932/steps/compile-webkit/logs/stdio Damn, that is dumb. Give me a minute, I'll fix that. 617788 8 yong.li.webkit 2012-05-07 11:50:24 -0700 *** Bug 79125 has been marked as a duplicate of this bug. *** 617799 9 yong.li.webkit 2012-05-07 12:03:05 -0700 There is no test, neither can I find a reason why there is no test in here or change log. 617804 10 benjamin 2012-05-07 12:08:32 -0700 > There is no test, neither can I find a reason why there is no test in here or change log. Are you kidding me? How would you test this? 617813 11 yong.li.webkit 2012-05-07 12:11:48 -0700 (In reply to comment #10) > > There is no test, neither can I find a reason why there is no test in here or change log. > > Are you kidding me? How would you test this? I don't have a quick solution for testing it, either. But I would mention that in change log or bugzilla, as I've been told by many reviewers. 617816 12 benjamin 2012-05-07 12:14:59 -0700 > I don't have a quick solution for testing it, either. But I would mention that in change log or bugzilla, as I've been told by many reviewers. We also use common sense in reviews. 132483 2012-03-17 22:40:07 -0700 2012-03-19 13:40:54 -0700 Patch bug-81466-20120317224006.patch text/plain 5362 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTExMTI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBl OGE5YTNhODMwMDFiODc5Yzc5ODI2OTc1MGMyNDU1NTA3YmJkNjQ2Li5iNzhmMWYyYTFiNTkxMWI4 NTVhZWZhNzEyODdkMjA0ODE5MjJiNDZkIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAxMi0wMy0xNyAgQmVuamFtaW4gUG91bGFpbiAgPGJlbmphbWluQHdlYmtp dC5vcmc+CisKKyAgICAgICAgRG8gbm90IGNvcHkgdGhlIHNjcmlwdCBzb3VyY2UgaW4gdGhlIFNv dXJjZVByb3ZpZGVyLCBqdXN0IHJlZmVyZW5jZSB0aGUgZXhpc3Rpbmcgc3RyaW5nCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04MTQ2NgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcGFyc2VyL1NvdXJjZUNv ZGUuaDogUmVtb3ZlIHRoZSB1bnVzZWQsIGFuZCBpbmNvcnJlY3QsIGZ1bmN0aW9uIGRhdGEoKS4K KyAgICAgICAgKiBwYXJzZXIvU291cmNlUHJvdmlkZXIuaDogQWRkIE9WRVJSSURFIGZvciBjbGFy aXR5LgorCiAyMDEyLTAzLTE2ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgog CiAgICAgICAgIEltcHJvdmUgZGlhZ25vc3RpYyBiZW5lZml0IG9mIEpTR2xvYmFsRGF0YTo6bV9p c0luaXRpYWxpemluZ09iamVjdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9n IGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IDg3OTBhYTYzNzhiN2YxNTQwNGUwOWY0 NWNkZDkyYjA3MmY4MTI3NjkuLmU3MmEyZjRhMzI3Y2U3NDgxODljOGU1ZjE0YTg1YjBmZDIxMzBl NjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMi0wMy0xNyAgQmVuamFtaW4gUG91 bGFpbiAgPGJlbmphbWluQHdlYmtpdC5vcmc+CisKKyAgICAgICAgRG8gbm90IGNvcHkgdGhlIHNj cmlwdCBzb3VyY2UgaW4gdGhlIFNvdXJjZVByb3ZpZGVyLCBqdXN0IHJlZmVyZW5jZSB0aGUgZXhp c3Rpbmcgc3RyaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD04MTQ2NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEluc3RlYWQgb2YgY3JlYXRpbmcgYSBjb3B5IG9mIHRoZSBzdHJpbmcgd2l0aCAxNmJpdHMg Y2hhcmFjdGVycywgd2UganVzdCByZXR1cm4KKyAgICAgICAgYSBzaGFyZWQgU3RyaW5nSW1wbC4K KworICAgICAgICAqIGJpbmRpbmdzL2pzL0NhY2hlZFNjcmlwdFNvdXJjZVByb3ZpZGVyLmg6Cisg ICAgICAgICogYmluZGluZ3MvanMvU3RyaW5nU291cmNlUHJvdmlkZXIuaDoKKwogMjAxMi0wMy0x NyAgSm9lIFRob21hcyAgPGpvZXRob21hc0Btb3Rvcm9sYS5jb20+CiAKICAgICAgICAgbW92ZSBj YWxjKlZhbHVlIGZ1bmN0aW9ucyBvdXQgZnJvbSBMZW5ndGggKGFuZCBwbGF0Zm9ybSkKZGlmZiAt LWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvU291cmNlQ29kZS5oIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL3BhcnNlci9Tb3VyY2VDb2RlLmgKaW5kZXggMmExNjJiOTBmYjEzN2Fh ZTk3YThiOGY0NzcwYTRhMDdkMmRlMjMxOC4uYTMzZmZlYTcyZjYzZjZlNWYzZTljMzM2ZmU2ZmQw MDE1NWNjNDU1ZiAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9Tb3Vy Y2VDb2RlLmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9Tb3VyY2VDb2RlLmgK QEAgLTcyLDExICs3Miw2IEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAgICBpbnQgZmlyc3RMaW5l KCkgY29uc3QgeyByZXR1cm4gbV9maXJzdExpbmU7IH0KICAgICAgICAgaW50IHN0YXJ0T2Zmc2V0 KCkgY29uc3QgeyByZXR1cm4gbV9zdGFydENoYXI7IH0KICAgICAgICAgaW50IGVuZE9mZnNldCgp IGNvbnN0IHsgcmV0dXJuIG1fZW5kQ2hhcjsgfQotICAgICAgICBjb25zdCBVQ2hhciogZGF0YSgp IGNvbnN0Ci0gICAgICAgIHsKLSAgICAgICAgICAgIEFTU0VSVChtX3Byb3ZpZGVyLT5kYXRhKCkp OwotICAgICAgICAgICAgcmV0dXJuIG1fcHJvdmlkZXItPmRhdGEoKS0+Y2hhcmFjdGVyczE2KCkg KyBtX3N0YXJ0Q2hhcjsKLSAgICAgICAgfQogICAgICAgICBpbnQgbGVuZ3RoKCkgY29uc3QgeyBy ZXR1cm4gbV9lbmRDaGFyIC0gbV9zdGFydENoYXI7IH0KICAgICAgICAgCiAgICAgICAgIFNvdXJj ZUNvZGUgc3ViRXhwcmVzc2lvbih1bnNpZ25lZCBvcGVuQnJhY2UsIHVuc2lnbmVkIGNsb3NlQnJh Y2UsIGludCBmaXJzdExpbmUpOwpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3Bh cnNlci9Tb3VyY2VQcm92aWRlci5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9Tb3Vy Y2VQcm92aWRlci5oCmluZGV4IGY2NmE5MjFmNTJiYzBjOWVjYWFjNTQzNmU3MjA5NWNkMjlkY2Zi NmEuLmQxOTdhY2ZlN2VmZTRmMDQ5NjM1YTNiNTMyYTcwZGNhZWVhNjRjZDAgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvU291cmNlUHJvdmlkZXIuaAorKysgYi9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvcGFyc2VyL1NvdXJjZVByb3ZpZGVyLmgKQEAgLTg2LDcgKzg2LDcg QEAgbmFtZXNwYWNlIEpTQyB7CiAgICAgICAgICAgICByZXR1cm4gYWRvcHRSZWYobmV3IFVTdHJp bmdTb3VyY2VQcm92aWRlcihzb3VyY2UsIHVybCwgc3RhcnRQb3NpdGlvbikpOwogICAgICAgICB9 CiAKLSAgICAgICAgVVN0cmluZyBnZXRSYW5nZShpbnQgc3RhcnQsIGludCBlbmQpIGNvbnN0Cisg ICAgICAgIFVTdHJpbmcgZ2V0UmFuZ2UoaW50IHN0YXJ0LCBpbnQgZW5kKSBjb25zdCBPVkVSUklE RQogICAgICAgICB7CiAgICAgICAgICAgICByZXR1cm4gbV9zb3VyY2Uuc3Vic3RyaW5nU2hhcmlu Z0ltcGwoc3RhcnQsIGVuZCAtIHN0YXJ0KTsKICAgICAgICAgfQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvYmluZGluZ3MvanMvQ2FjaGVkU2NyaXB0U291cmNlUHJvdmlkZXIuaCBiL1NvdXJj ZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0NhY2hlZFNjcmlwdFNvdXJjZVByb3ZpZGVyLmgKaW5kZXgg YjBiNmFjOTMyMWY5NjhjNzQwYmU2NWMxNTBkZWM5OGQzOTU1Njk4Yi4uYWJiOTU4N2U4ZWI0ZTcx NzMzNmY4YjliMWIzOTdmYTU3MjE3MDE1YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmlu ZGluZ3MvanMvQ2FjaGVkU2NyaXB0U291cmNlUHJvdmlkZXIuaAorKysgYi9Tb3VyY2UvV2ViQ29y ZS9iaW5kaW5ncy9qcy9DYWNoZWRTY3JpcHRTb3VyY2VQcm92aWRlci5oCkBAIC00NSw3ICs0NSwx NiBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgICAgICBtX2NhY2hlZFNjcmlwdC0+cmVt b3ZlQ2xpZW50KHRoaXMpOwogICAgICAgICB9CiAKLSAgICAgICAgSlNDOjpVU3RyaW5nIGdldFJh bmdlKGludCBzdGFydCwgaW50IGVuZCkgY29uc3QgeyByZXR1cm4gSlNDOjpVU3RyaW5nKG1fY2Fj aGVkU2NyaXB0LT5zY3JpcHQoKS5jaGFyYWN0ZXJzKCkgKyBzdGFydCwgZW5kIC0gc3RhcnQpOyB9 CisgICAgICAgIEpTQzo6VVN0cmluZyBnZXRSYW5nZShpbnQgc3RhcnQsIGludCBlbmQpIGNvbnN0 IE9WRVJSSURFCisgICAgICAgIHsKKyAgICAgICAgICAgIGludCBsZW5ndGggPSBlbmQgLSBzdGFy dDsKKyAgICAgICAgICAgIEFTU0VSVChsZW5ndGggPj0gMCk7CisgICAgICAgICAgICBBU1NFUlQo c3RhcnQgKyBsZW5ndGggPD0gbGVuZ3RoKCkpOworCisgICAgICAgICAgICBTdHJpbmcgc2NyaXB0 ID0gbV9jYWNoZWRTY3JpcHQtPnNjcmlwdCgpOworICAgICAgICAgICAgcmV0dXJuIEpTQzo6VVN0 cmluZyhTdHJpbmdJbXBsOjpjcmVhdGUoc2NyaXB0LmltcGwoKSwgc3RhcnQsIGxlbmd0aCkpOwor ICAgICAgICB9CisKICAgICAgICAgY29uc3QgU3RyaW5nSW1wbCogZGF0YSgpIGNvbnN0IHsgcmV0 dXJuIG1fY2FjaGVkU2NyaXB0LT5zY3JpcHQoKS5pbXBsKCk7IH0KICAgICAgICAgaW50IGxlbmd0 aCgpIGNvbnN0IHsgcmV0dXJuIG1fY2FjaGVkU2NyaXB0LT5zY3JpcHQoKS5sZW5ndGgoKTsgfQog ICAgICAgICBjb25zdCBTdHJpbmcmIHNvdXJjZSgpIGNvbnN0IHsgcmV0dXJuIG1fY2FjaGVkU2Ny aXB0LT5zY3JpcHQoKTsgfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMv U3RyaW5nU291cmNlUHJvdmlkZXIuaCBiL1NvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL1N0cmlu Z1NvdXJjZVByb3ZpZGVyLmgKaW5kZXggZWQzMDJmOGRkNmJmZDVmYTZjMDJlYjYwZGFhNmQxM2Ez NjcwMzgwOS4uZWNiZGE5MjNhOTQzNGQzMzY3ZDc2ZjE0OGRkNWZhODlmNmMxZGRmNSAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvU3RyaW5nU291cmNlUHJvdmlkZXIuaAor KysgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9TdHJpbmdTb3VyY2VQcm92aWRlci5oCkBA IC00Miw3ICs0MiwxNSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgICAgICByZXR1cm4g YWRvcHRSZWYobmV3IFN0cmluZ1NvdXJjZVByb3ZpZGVyKHNvdXJjZSwgdXJsLCBzdGFydFBvc2l0 aW9uKSk7CiAgICAgICAgIH0KIAotICAgICAgICBKU0M6OlVTdHJpbmcgZ2V0UmFuZ2UoaW50IHN0 YXJ0LCBpbnQgZW5kKSBjb25zdCB7IHJldHVybiBKU0M6OlVTdHJpbmcobV9zb3VyY2UuY2hhcmFj dGVycygpICsgc3RhcnQsIGVuZCAtIHN0YXJ0KTsgfQorICAgICAgICBKU0M6OlVTdHJpbmcgZ2V0 UmFuZ2UoaW50IHN0YXJ0LCBpbnQgZW5kKSBjb25zdCBPVkVSUklERQorICAgICAgICB7CisgICAg ICAgICAgICBpbnQgbGVuZ3RoID0gZW5kIC0gc3RhcnQ7CisgICAgICAgICAgICBBU1NFUlQobGVu Z3RoID49IDApOworICAgICAgICAgICAgQVNTRVJUKHN0YXJ0ICsgbGVuZ3RoIDw9IGxlbmd0aCgp KTsKKworICAgICAgICAgICAgcmV0dXJuIEpTQzo6VVN0cmluZyhTdHJpbmdJbXBsOjpjcmVhdGUo bV9zb3VyY2UuaW1wbCgpLCBzdGFydCwgbGVuZ3RoKSk7CisgICAgICAgIH0KKwogICAgICAgICBj b25zdCBTdHJpbmdJbXBsKiBkYXRhKCkgY29uc3QgeyByZXR1cm4gbV9zb3VyY2UuaW1wbCgpOyB9 CiAgICAgICAgIGludCBsZW5ndGgoKSBjb25zdCB7IHJldHVybiBtX3NvdXJjZS5sZW5ndGgoKTsg fQogICAgICAgICBjb25zdCBTdHJpbmcmIHNvdXJjZSgpIGNvbnN0IHsgcmV0dXJuIG1fc291cmNl OyB9Cg==