121656 2013-09-19 21:31:49 -0700 Avoid calling AtomicString::lower() in makeFontGlyphsCacheKey 2013-09-20 00:07:54 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 andersca andersca ossy oldest_to_newest 931180 0 andersca 2013-09-19 21:31:49 -0700 Avoid calling AtomicString::lower() in makeFontGlyphsCacheKey 931181 1 212122 andersca 2013-09-19 21:34:17 -0700 Created attachment 212122 Patch 931185 2 212122 darin 2013-09-19 21:37:24 -0700 Comment on attachment 212122 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212122&action=review > Source/WebCore/platform/graphics/Font.cpp:171 > + if (!equalIgnoringCase(a.families[i].impl(), b.families[i].impl())) Why is the impl() needed here? Shouldn't we have functions overloaded so that isn’t needed? > Source/WebCore/platform/graphics/Font.cpp:206 > + Vector<unsigned, 7> hashCodes; > + hashCodes.reserveInitialCapacity(4 + key.families.size()); Need a comment explaining why 7. Why not, say, more like 16 to cover virtually all non-ridiculous cases without allocation? > Source/WebCore/platform/graphics/Font.cpp:215 > + return StringHasher::hashMemory(hashCodes.data(), hashCodes.size() * sizeof(unsigned)); The proper comment for this line is, "Yo dawg! I heard you like hashes, so I hashed your hash." 931187 3 212122 rniwa 2013-09-19 21:37:37 -0700 Comment on attachment 212122 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212122&action=review > Source/WebCore/platform/graphics/Font.cpp:170 > + for (unsigned i = 0; i < a.families.size(); ++i) { Why don't we cache the size in a local variable or traverse backwards? 931189 4 andersca 2013-09-19 21:46:22 -0700 Committed r156139: <http://trac.webkit.org/changeset/156139> 931200 5 ossy 2013-09-19 22:38:20 -0700 (In reply to comment #4) > Committed r156139: <http://trac.webkit.org/changeset/156139> FYI:It made all test crash everywhere. 931207 6 ossy 2013-09-20 00:07:54 -0700 Fix landed in http://trac.webkit.org/changeset/156142. Thanks. 212122 2013-09-19 21:34:17 -0700 2013-09-19 21:37:37 -0700 Patch bug-121656-20130919213422.patch text/plain 3727 andersca U3VidmVyc2lvbiBSZXZpc2lvbjogMTU2MTM2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTQyNTU2YmNmMTk5NWVi MjZkMDBkMGVjOTk2MmU4MzBkNDJhY2I3Mi4uN2IyNGYxNDA5NWY1YmFlNDUzYWM1NDA5NWNhODMx YTUyYTc2ZDlhOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTA5LTE5ICBBbmRl cnMgQ2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CisKKyAgICAgICAgQXZvaWQgY2FsbGlu ZyBBdG9taWNTdHJpbmc6Omxvd2VyKCkgaW4gbWFrZUZvbnRHbHlwaHNDYWNoZUtleQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIxNjU2CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW5zdGVhZCBvZiBsb3dl ciBjYXNpbmcgQXRvbWljU3RyaW5ncywgc3RvcmUgdGhlIHN0cmluZ3MgYXMgaXMgYW5kIHVzZSB0 aGUgY2FzZSBmb2xkaW5nCisgICAgICAgIGhhc2ggYW5kIGNhc2UgaW5zZW5zaXRpdmUgY29tcGFy ZSB0byBkZXRlcm1pbmUgZXF1YWxpdHkuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9G b250LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Om9wZXJhdG9yPT0pOgorICAgICAgICAoV2ViQ29y ZTo6bWFrZUZvbnRHbHlwaHNDYWNoZUtleSk6CisgICAgICAgIChXZWJDb3JlOjpjb21wdXRlRm9u dEdseXBoc0NhY2hlSGFzaCk6CisKIDIwMTMtMDktMTkgIEVyaWMgQ2FybHNvbiAgPGVyaWMuY2Fy bHNvbkBhcHBsZS5jb20+CiAKICAgICAgICAgTWVkaWFTdHJlYW0gQVBJOiB1cGRhdGUgTWVkaWFT dHJlYW1UcmFja0V2ZW50IG9iamVjdCB0byBtYXRjaCBzcGVjCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9Gb250LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL0ZvbnQuY3BwCmluZGV4IGMzY2NhNmVlOTNmMTlkNjUwZDRmMGJlMjZkMThl MTM0NWM4NzE0Y2YuLjI0M2IyZDNhMzQzMTg3OTM4NjgwMzI4MDViNWMwNjMzYTRlZWNmZGMgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ZvbnQuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ZvbnQuY3BwCkBAIC0xNjMsMTAgKzE2 MywxNCBAQCBzdGF0aWMgYm9vbCBvcGVyYXRvcj09KGNvbnN0IEZvbnRHbHlwaHNDYWNoZUtleSYg YSwgY29uc3QgRm9udEdseXBoc0NhY2hlS2V5JiBiKQogewogICAgIGlmIChhLmZvbnREZXNjcmlw dGlvbkNhY2hlS2V5ICE9IGIuZm9udERlc2NyaXB0aW9uQ2FjaGVLZXkpCiAgICAgICAgIHJldHVy biBmYWxzZTsKLSAgICBpZiAoYS5mYW1pbGllcyAhPSBiLmZhbWlsaWVzKQotICAgICAgICByZXR1 cm4gZmFsc2U7CiAgICAgaWYgKGEuZm9udFNlbGVjdG9ySWQgIT0gYi5mb250U2VsZWN0b3JJZCB8 fCBhLmZvbnRTZWxlY3RvclZlcnNpb24gIT0gYi5mb250U2VsZWN0b3JWZXJzaW9uIHx8IGEuZm9u dFNlbGVjdG9yRmxhZ3MgIT0gYi5mb250U2VsZWN0b3JGbGFncykKICAgICAgICAgcmV0dXJuIGZh bHNlOworICAgIGlmIChhLmZhbWlsaWVzLnNpemUoKSAhPSBiLmZhbWlsaWVzLnNpemUoKSkKKyAg ICAgICAgcmV0dXJuIGZhbHNlOworICAgIGZvciAodW5zaWduZWQgaSA9IDA7IGkgPCBhLmZhbWls aWVzLnNpemUoKTsgKytpKSB7CisgICAgICAgIGlmICghZXF1YWxJZ25vcmluZ0Nhc2UoYS5mYW1p bGllc1tpXS5pbXBsKCksIGIuZmFtaWxpZXNbaV0uaW1wbCgpKSkKKyAgICAgICAgICAgIHJldHVy biBmYWxzZTsKKyAgICB9CiAgICAgcmV0dXJuIHRydWU7CiB9CiAKQEAgLTE5MCw3ICsxOTQsNyBA QCBzdGF0aWMgdm9pZCBtYWtlRm9udEdseXBoc0NhY2hlS2V5KEZvbnRHbHlwaHNDYWNoZUtleSYg a2V5LCBjb25zdCBGb250RGVzY3JpcHRpbwogewogICAgIGtleS5mb250RGVzY3JpcHRpb25DYWNo ZUtleSA9IEZvbnREZXNjcmlwdGlvbkZvbnREYXRhQ2FjaGVLZXkoZGVzY3JpcHRpb24pOwogICAg IGZvciAodW5zaWduZWQgaSA9IDA7IGkgPCBkZXNjcmlwdGlvbi5mYW1pbHlDb3VudCgpOyArK2kp Ci0gICAgICAgIGtleS5mYW1pbGllcy5hcHBlbmQoZGVzY3JpcHRpb24uZmFtaWx5QXQoaSkubG93 ZXIoKSk7CisgICAgICAgIGtleS5mYW1pbGllcy5hcHBlbmQoZGVzY3JpcHRpb24uZmFtaWx5QXQo aSkpOwogICAgIGtleS5mb250U2VsZWN0b3JJZCA9IGZvbnRTZWxlY3RvciA/IGZvbnRTZWxlY3Rv ci0+dW5pcXVlSWQoKSA6IDA7CiAgICAga2V5LmZvbnRTZWxlY3RvclZlcnNpb24gPSBmb250U2Vs ZWN0b3IgPyBmb250U2VsZWN0b3ItPnZlcnNpb24oKSA6IDA7CiAgICAga2V5LmZvbnRTZWxlY3Rv ckZsYWdzID0gZm9udFNlbGVjdG9yICYmIGZvbnRTZWxlY3Rvci0+cmVzb2x2ZXNGYW1pbHlGb3Io ZGVzY3JpcHRpb24pID8gbWFrZUZvbnRTZWxlY3RvckZsYWdzKGRlc2NyaXB0aW9uKSA6IDA7CkBA IC0xOTgsMTQgKzIwMiwxNyBAQCBzdGF0aWMgdm9pZCBtYWtlRm9udEdseXBoc0NhY2hlS2V5KEZv bnRHbHlwaHNDYWNoZUtleSYga2V5LCBjb25zdCBGb250RGVzY3JpcHRpbwogCiBzdGF0aWMgdW5z aWduZWQgY29tcHV0ZUZvbnRHbHlwaHNDYWNoZUhhc2goY29uc3QgRm9udEdseXBoc0NhY2hlS2V5 JiBrZXkpCiB7Ci0gICAgdW5zaWduZWQgaGFzaENvZGVzWzVdID0gewotICAgICAgICBTdHJpbmdI YXNoZXI6Omhhc2hNZW1vcnkoa2V5LmZhbWlsaWVzLmRhdGEoKSwga2V5LmZhbWlsaWVzLnNpemUo KSAqIHNpemVvZihrZXkuZmFtaWxpZXNbMF0pKSwKLSAgICAgICAga2V5LmZvbnREZXNjcmlwdGlv bkNhY2hlS2V5LmNvbXB1dGVIYXNoKCksCi0gICAgICAgIGtleS5mb250U2VsZWN0b3JJZCwKLSAg ICAgICAga2V5LmZvbnRTZWxlY3RvclZlcnNpb24sCi0gICAgICAgIGtleS5mb250U2VsZWN0b3JG bGFncwotICAgIH07Ci0gICAgcmV0dXJuIFN0cmluZ0hhc2hlcjo6aGFzaE1lbW9yeTxzaXplb2Yo aGFzaENvZGVzKT4oaGFzaENvZGVzKTsKKyAgICBWZWN0b3I8dW5zaWduZWQsIDc+IGhhc2hDb2Rl czsKKyAgICBoYXNoQ29kZXMucmVzZXJ2ZUluaXRpYWxDYXBhY2l0eSg0ICsga2V5LmZhbWlsaWVz LnNpemUoKSk7CisKKyAgICBoYXNoQ29kZXMudW5jaGVja2VkQXBwZW5kKGtleS5mb250RGVzY3Jp cHRpb25DYWNoZUtleS5jb21wdXRlSGFzaCgpKTsKKyAgICBoYXNoQ29kZXMudW5jaGVja2VkQXBw ZW5kKGtleS5mb250U2VsZWN0b3JJZCk7CisgICAgaGFzaENvZGVzLnVuY2hlY2tlZEFwcGVuZChr ZXkuZm9udFNlbGVjdG9yVmVyc2lvbik7CisgICAgaGFzaENvZGVzLnVuY2hlY2tlZEFwcGVuZChr ZXkuZm9udFNlbGVjdG9yRmxhZ3MpOworICAgIGZvciAodW5zaWduZWQgaSA9IDA7IGkgPCBrZXku ZmFtaWxpZXMuc2l6ZSgpOyArK2kpCisgICAgICAgIGhhc2hDb2Rlcy51bmNoZWNrZWRBcHBlbmQo Q2FzZUZvbGRpbmdIYXNoOjpoYXNoKGtleS5mYW1pbGllc1tpXSkpOworCisgICAgcmV0dXJuIFN0 cmluZ0hhc2hlcjo6aGFzaE1lbW9yeShoYXNoQ29kZXMuZGF0YSgpLCBoYXNoQ29kZXMuc2l6ZSgp ICogc2l6ZW9mKHVuc2lnbmVkKSk7CiB9CiAKIHZvaWQgcHJ1bmVVbnJlZmVyZW5jZWRFbnRyaWVz RnJvbUZvbnRHbHlwaHNDYWNoZSgpCg==