99556 2012-10-17 00:35:51 -0700 Clean up ContentData operator overloads 2012-10-17 01:18:14 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 99498 1 esprehn esprehn eric inferno jchaffraix kling koivisto webkit.review.bot oldest_to_newest 743957 0 esprehn 2012-10-17 00:35:51 -0700 Clean up ContentData operator overloads 743962 1 169109 esprehn 2012-10-17 00:40:48 -0700 Created attachment 169109 Patch 743969 2 169109 eric 2012-10-17 00:48:55 -0700 Comment on attachment 169109 Patch Seems OK... I worry if this will slow down canShareStyleWithElement (which can be really hot when adding siblings). 743970 3 eric 2012-10-17 00:49:36 -0700 I think that if style sharing is already disabled for elements with content: then this should have no perf impact. 743971 4 esprehn 2012-10-17 00:50:49 -0700 (In reply to comment #2) > (From update of attachment 169109 [details]) > Seems OK... I worry if this will slow down canShareStyleWithElement (which can be really hot when adding siblings). It shouldn't, type() is virtual and the old operator== called it 3 times and then did a switch. With this patch we only make 2 virtual calls and doesn't do a jump for the switch. If anything I feel like this should be (marginally) faster. 743973 5 eric 2012-10-17 00:53:47 -0700 It looks like styles with content actually can be shared in many cases: http://trac.webkit.org/browser/trunk/Source/WebCore/css/StyleResolver.cpp#L2925 Note that only one case sets unique. That said, I'm still not sure when this comparison is used, so this may still be fine? 743977 6 169109 eric 2012-10-17 00:56:34 -0700 Comment on attachment 169109 Patch OK. I'll buy that. Thanks. 743985 7 esprehn 2012-10-17 01:06:40 -0700 (In reply to comment #5) > It looks like styles with content actually can be shared in many cases: > http://trac.webkit.org/browser/trunk/Source/WebCore/css/StyleResolver.cpp#L2925 > > Note that only one case sets unique. > > That said, I'm still not sure when this comparison is used, so this may still be fine? It gets used in Node::diff through StyleRareNonInheritedData::contentDataEquivalent. Your comment made me think to look at where this is used and then I realized our usage is broken: https://bugs.webkit.org/show_bug.cgi?id=99560 :( 743996 8 169109 webkit.review.bot 2012-10-17 01:18:10 -0700 Comment on attachment 169109 Patch Clearing flags on attachment: 169109 Committed r131565: <http://trac.webkit.org/changeset/131565> 743997 9 webkit.review.bot 2012-10-17 01:18:14 -0700 All reviewed patches have been landed. Closing bug. 169109 2012-10-17 00:40:48 -0700 2012-10-17 01:18:10 -0700 Patch bug-99556-20121017003936.patch text/plain 5689 esprehn U3VidmVyc2lvbiBSZXZpc2lvbjogMTMxNDc2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTQ0ZGVkMWU0ZGRmMmM0 NGNjOGNhZGE4NTg2Mzc3ZTM0OGYwNGQ3NS4uNWFjOTE5OTQ0YjQ0NGU2MzkyOTc1NzY0MWFlNGFm YTI0OWViNWY4MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI0IEBACisyMDEyLTEwLTE3ICBFbGxp b3R0IFNwcmVobiAgPGVzcHJlaG5AY2hyb21pdW0ub3JnPgorCisgICAgICAgIENsZWFuIHVwIENv bnRlbnREYXRhIG9wZXJhdG9yIG92ZXJsb2FkcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTk1NTYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBVc2UgdmlydHVhbCBkaXNwYXRjaCBmb3IgY2hlY2tpbmcgQ29u dGVudERhdGEgZXF1YWxpdHkgaW5zdGVhZAorICAgICAgICBvZiBhIHN3aXRjaCBvdmVyIHRoZSB0 eXBlLiBUaGlzIHRoZSBmaXJzdCBzdGVwIGluIGdldHRpbmcgcmlkCisgICAgICAgIG9mIHRoZSBT dHlsZUNvbnRlbnRUeXBlIGVudW0gYW5kIGFsbCB0aGUgc3dpdGNoIHN0YXRlbWVudHMgb3Zlcgor ICAgICAgICB0aGUgdHlwZSgpLgorCisgICAgICAgIE5vIHRlc3RzIG5lZWRlZCwgdGhpcyBpcyBq dXN0IGEgcmVmYWN0b3IuICAgICAgICAKKworICAgICAgICAqIHJlbmRlcmluZy9zdHlsZS9Db250 ZW50RGF0YS5jcHA6CisgICAgICAgICogcmVuZGVyaW5nL3N0eWxlL0NvbnRlbnREYXRhLmg6Cisg ICAgICAgIChDb250ZW50RGF0YSk6CisgICAgICAgIChXZWJDb3JlOjpvcGVyYXRvcj09KToKKyAg ICAgICAgKFdlYkNvcmUpOgorICAgICAgICAoV2ViQ29yZTo6b3BlcmF0b3IhPSk6CisKIDIwMTIt MTAtMTYgIFJhcGhhZWwgS3VibyBkYSBDb3N0YSAgPHJhcGhhZWwua3Viby5kYS5jb3N0YUBpbnRl bC5jb20+CiAKICAgICAgICAgSW1wbGVtZW50IHRlc3RSdW5uZXIuZHVtcFNlbGVjdGlvblJlY3Qo KSBpbiBXZWJLaXRUZXN0UnVubmVyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJp bmcvc3R5bGUvQ29udGVudERhdGEuY3BwIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL3N0eWxl L0NvbnRlbnREYXRhLmNwcAppbmRleCBlOGI0NTM3MzllYjc1MmI5ZGNlOTg2YmQ5OWRkMDNjN2Yy NDdjMzU2Li43Zjg3YTBjMmIwODRhZjgwNTY2NTg5MjQ5YTBiZGQxY2U4YmExMWY0IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvc3R5bGUvQ29udGVudERhdGEuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9zdHlsZS9Db250ZW50RGF0YS5jcHAKQEAgLTIyLDgg KzIyLDYgQEAKICNpbmNsdWRlICJjb25maWcuaCIKICNpbmNsdWRlICJDb250ZW50RGF0YS5oIgog Ci0jaW5jbHVkZSAiU3R5bGVJbWFnZS5oIgotCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKIFBhc3NP d25QdHI8Q29udGVudERhdGE+IENvbnRlbnREYXRhOjpjcmVhdGUoUGFzc1JlZlB0cjxTdHlsZUlt YWdlPiBpbWFnZSkKQEAgLTYwLDMwICs1OCw0IEBAIFBhc3NPd25QdHI8Q29udGVudERhdGE+IENv bnRlbnREYXRhOjpjbG9uZSgpIGNvbnN0CiAgICAgcmV0dXJuIHJlc3VsdC5yZWxlYXNlKCk7CiB9 CiAKLWJvb2wgb3BlcmF0b3I9PShjb25zdCBDb250ZW50RGF0YSYgYSwgY29uc3QgQ29udGVudERh dGEmIGIpCi17Ci0gICAgaWYgKGEudHlwZSgpICE9IGIudHlwZSgpKQotICAgICAgICByZXR1cm4g ZmFsc2U7Ci0KLSAgICBzd2l0Y2ggKGEudHlwZSgpKSB7Ci0gICAgY2FzZSBDT05URU5UX05PTkU6 Ci0gICAgICAgIHJldHVybiB0cnVlOwotICAgIGNhc2UgQ09OVEVOVF9PQkpFQ1Q6Ci0gICAgICAg IHJldHVybiAqc3RhdGljX2Nhc3Q8Y29uc3QgSW1hZ2VDb250ZW50RGF0YSo+KCZhKS0+aW1hZ2Uo KSA9PSAqc3RhdGljX2Nhc3Q8Y29uc3QgSW1hZ2VDb250ZW50RGF0YSo+KCZiKS0+aW1hZ2UoKTsK LSAgICBjYXNlIENPTlRFTlRfVEVYVDoKLSAgICAgICAgcmV0dXJuIHN0YXRpY19jYXN0PGNvbnN0 IFRleHRDb250ZW50RGF0YSo+KCZhKS0+dGV4dCgpID09IHN0YXRpY19jYXN0PGNvbnN0IFRleHRD b250ZW50RGF0YSo+KCZiKS0+dGV4dCgpOwotICAgIGNhc2UgQ09OVEVOVF9DT1VOVEVSOgotICAg ICAgICByZXR1cm4gKnN0YXRpY19jYXN0PGNvbnN0IENvdW50ZXJDb250ZW50RGF0YSo+KCZhKS0+ Y291bnRlcigpID09ICpzdGF0aWNfY2FzdDxjb25zdCBDb3VudGVyQ29udGVudERhdGEqPigmYikt PmNvdW50ZXIoKTsKLSAgICBjYXNlIENPTlRFTlRfUVVPVEU6Ci0gICAgICAgIHJldHVybiBzdGF0 aWNfY2FzdDxjb25zdCBRdW90ZUNvbnRlbnREYXRhKj4oJmEpLT5xdW90ZSgpID09IHN0YXRpY19j YXN0PGNvbnN0IFF1b3RlQ29udGVudERhdGEqPigmYiktPnF1b3RlKCk7Ci0gICAgfQotICAgIEFT U0VSVF9OT1RfUkVBQ0hFRCgpOwotICAgIHJldHVybiBmYWxzZTsKLX0KLQotYm9vbCBvcGVyYXRv ciE9KGNvbnN0IENvbnRlbnREYXRhJiBhLCBjb25zdCBDb250ZW50RGF0YSYgYikKLXsKLSAgICBy ZXR1cm4gIShhID09IGIpOwotfQotCiB9IC8vIG5hbWVzcGFjZSBXZWJDb3JlCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvc3R5bGUvQ29udGVudERhdGEuaCBiL1NvdXJjZS9X ZWJDb3JlL3JlbmRlcmluZy9zdHlsZS9Db250ZW50RGF0YS5oCmluZGV4IDc1MWFmMDViM2FmMTU1 YmRhMjY4OGIwYmYxYjMyNmY2Y2Q4ZGJiMDAuLjg4NzE4ZDg0ZjRhYmIzYjRmOTBmMjE1ZDRhMTEy NmFkOGM1MWEzM2YgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9zdHlsZS9D b250ZW50RGF0YS5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9zdHlsZS9Db250ZW50 RGF0YS5oCkBAIC0yNiw2ICsyNiw3IEBACiAjZGVmaW5lIENvbnRlbnREYXRhX2gKIAogI2luY2x1 ZGUgIkNvdW50ZXJDb250ZW50LmgiCisjaW5jbHVkZSAiU3R5bGVJbWFnZS5oIgogI2luY2x1ZGUg PHd0Zi9Pd25QdHIuaD4KICNpbmNsdWRlIDx3dGYvUGFzc093blB0ci5oPgogCkBAIC00OSwxNSAr NTAsMTMgQEAgcHVibGljOgogICAgIHZpcnR1YWwgYm9vbCBpc1RleHQoKSBjb25zdCB7IHJldHVy biBmYWxzZTsgfQogCiAgICAgdmlydHVhbCBTdHlsZUNvbnRlbnRUeXBlIHR5cGUoKSBjb25zdCA9 IDA7Ci0KLSAgICBmcmllbmQgYm9vbCBvcGVyYXRvcj09KGNvbnN0IENvbnRlbnREYXRhJiwgY29u c3QgQ29udGVudERhdGEmKTsKLSAgICBmcmllbmQgYm9vbCBvcGVyYXRvciE9KGNvbnN0IENvbnRl bnREYXRhJiwgY29uc3QgQ29udGVudERhdGEmKTsKLQogICAgIHZpcnR1YWwgUGFzc093blB0cjxD b250ZW50RGF0YT4gY2xvbmUoKSBjb25zdDsKIAogICAgIENvbnRlbnREYXRhKiBuZXh0KCkgY29u c3QgeyByZXR1cm4gbV9uZXh0LmdldCgpOyB9CiAgICAgdm9pZCBzZXROZXh0KFBhc3NPd25QdHI8 Q29udGVudERhdGE+IG5leHQpIHsgbV9uZXh0ID0gbmV4dDsgfQogCisgICAgdmlydHVhbCBib29s IGVxdWFscyhjb25zdCBDb250ZW50RGF0YSYpIGNvbnN0ID0gMDsKKwogcHJpdmF0ZToKICAgICB2 aXJ0dWFsIFBhc3NPd25QdHI8Q29udGVudERhdGE+IGNsb25lSW50ZXJuYWwoKSBjb25zdCA9IDA7 CiAKQEAgLTcxLDYgKzcwLDEzIEBAIHB1YmxpYzoKICAgICBTdHlsZUltYWdlKiBpbWFnZSgpIHsg cmV0dXJuIG1faW1hZ2UuZ2V0KCk7IH0KICAgICB2b2lkIHNldEltYWdlKFBhc3NSZWZQdHI8U3R5 bGVJbWFnZT4gaW1hZ2UpIHsgbV9pbWFnZSA9IGltYWdlOyB9CiAKKyAgICB2aXJ0dWFsIGJvb2wg ZXF1YWxzKGNvbnN0IENvbnRlbnREYXRhJiBkYXRhKSBjb25zdCBPVkVSUklERQorICAgIHsKKyAg ICAgICAgaWYgKCFkYXRhLmlzSW1hZ2UoKSkKKyAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAg ICAgICAgcmV0dXJuICpzdGF0aWNfY2FzdDxjb25zdCBJbWFnZUNvbnRlbnREYXRhJj4oZGF0YSku aW1hZ2UoKSA9PSAqaW1hZ2UoKTsKKyAgICB9CisKIHByaXZhdGU6CiAgICAgSW1hZ2VDb250ZW50 RGF0YShQYXNzUmVmUHRyPFN0eWxlSW1hZ2U+IGltYWdlKQogICAgICAgICA6IG1faW1hZ2UoaW1h Z2UpCkBAIC05NCw2ICsxMDAsMTMgQEAgcHVibGljOgogICAgIGNvbnN0IFN0cmluZyYgdGV4dCgp IGNvbnN0IHsgcmV0dXJuIG1fdGV4dDsgfQogICAgIHZvaWQgc2V0VGV4dChjb25zdCBTdHJpbmcm IHRleHQpIHsgbV90ZXh0ID0gdGV4dDsgfQogCisgICAgdmlydHVhbCBib29sIGVxdWFscyhjb25z dCBDb250ZW50RGF0YSYgZGF0YSkgY29uc3QgT1ZFUlJJREUKKyAgICB7CisgICAgICAgIGlmICgh ZGF0YS5pc1RleHQoKSkKKyAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgcmV0dXJu IHN0YXRpY19jYXN0PGNvbnN0IFRleHRDb250ZW50RGF0YSY+KGRhdGEpLnRleHQoKSA9PSB0ZXh0 KCk7CisgICAgfQorCiBwcml2YXRlOgogICAgIFRleHRDb250ZW50RGF0YShjb25zdCBTdHJpbmcm IHRleHQpCiAgICAgICAgIDogbV90ZXh0KHRleHQpCkBAIC0xMjcsNiArMTQwLDEzIEBAIHByaXZh dGU6CiAgICAgICAgIHJldHVybiBjcmVhdGUoY291bnRlckRhdGEucmVsZWFzZSgpKTsKICAgICB9 CiAKKyAgICB2aXJ0dWFsIGJvb2wgZXF1YWxzKGNvbnN0IENvbnRlbnREYXRhJiBkYXRhKSBjb25z dCBPVkVSUklERQorICAgIHsKKyAgICAgICAgaWYgKCFkYXRhLmlzQ291bnRlcigpKQorICAgICAg ICAgICAgcmV0dXJuIGZhbHNlOworICAgICAgICByZXR1cm4gKnN0YXRpY19jYXN0PGNvbnN0IENv dW50ZXJDb250ZW50RGF0YSY+KGRhdGEpLmNvdW50ZXIoKSA9PSAqY291bnRlcigpOworICAgIH0K KwogICAgIE93blB0cjxDb3VudGVyQ29udGVudD4gbV9jb3VudGVyOwogfTsKIApAQCAtMTM2LDYg KzE1NiwxMyBAQCBwdWJsaWM6CiAgICAgUXVvdGVUeXBlIHF1b3RlKCkgY29uc3QgeyByZXR1cm4g bV9xdW90ZTsgfQogICAgIHZvaWQgc2V0UXVvdGUoUXVvdGVUeXBlIHF1b3RlKSB7IG1fcXVvdGUg PSBxdW90ZTsgfQogCisgICAgdmlydHVhbCBib29sIGVxdWFscyhjb25zdCBDb250ZW50RGF0YSYg ZGF0YSkgY29uc3QgT1ZFUlJJREUKKyAgICB7CisgICAgICAgIGlmICghZGF0YS5pc1F1b3RlKCkp CisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgIHJldHVybiBzdGF0aWNfY2FzdDxj b25zdCBRdW90ZUNvbnRlbnREYXRhJj4oZGF0YSkucXVvdGUoKSA9PSBxdW90ZSgpOworICAgIH0K KwogcHJpdmF0ZToKICAgICBRdW90ZUNvbnRlbnREYXRhKFF1b3RlVHlwZSBxdW90ZSkKICAgICAg ICAgOiBtX3F1b3RlKHF1b3RlKQpAQCAtMTQ5LDYgKzE3NiwxNiBAQCBwcml2YXRlOgogICAgIFF1 b3RlVHlwZSBtX3F1b3RlOwogfTsKIAoraW5saW5lIGJvb2wgb3BlcmF0b3I9PShjb25zdCBDb250 ZW50RGF0YSYgYSwgY29uc3QgQ29udGVudERhdGEmIGIpCit7CisgICAgcmV0dXJuIGEuZXF1YWxz KGIpOworfQorCitpbmxpbmUgYm9vbCBvcGVyYXRvciE9KGNvbnN0IENvbnRlbnREYXRhJiBhLCBj b25zdCBDb250ZW50RGF0YSYgYikKK3sKKyAgICByZXR1cm4gIShhID09IGIpOworfQorCiB9IC8v IG5hbWVzcGFjZSBXZWJDb3JlCiAKICNlbmRpZiAvLyBDb250ZW50RGF0YV9oCg==