220969 2021-01-26 03:05:47 -0800 Ensure FontGenericFamilies uses AtomString where possible 2021-02-02 03:06:32 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=220801 InRadar P2 Normal --- 1 clord clord darin esprehn+autocc ews-watchlist ggaren glenn gyuyoung.kim koivisto macpherson menard sam simon.fraser thorton webkit-bug-importer oldest_to_newest 1722439 0 clord 2021-01-26 03:05:47 -0800 Bug 220801 changed the function prototypes and storage on FontGenericFamilies from AtomString to String, to allow for easier cross-thread copies. We should still ensure that the actual storage is AtomString where possible though, to avoid string duplication. 1722441 1 418387 clord 2021-01-26 03:10:12 -0800 Created attachment 418387 Patch 1722549 2 418387 darin 2021-01-26 09:33:28 -0800 Comment on attachment 418387 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418387&action=review > Source/WebCore/page/SettingsBase.cpp:98 > + bool changes = fontGenericFamilies().setStandardFontFamily(AtomString(family), script); I think we need a why comment explaining why we convert a string to an AtomString even though we are then calling a function that takes a String. Otherwise it just seems like a mistake to be undone. Obviously we don’t want to repeat that comment over and over again. And we should make sure we are thinking clearly about the benefit here; writing the comment helps us be explicit about the benefit we think accrues. We’re thinking that converting to AtomString lets us save memory because we think the font family names will be repeated and so it’s good to share the storage. That works across multiple Settings objects, each of which has its own FontGenericFamilies object. And the code goes here because here we know it’s running on the main thread, whereas other uses of FontGenericFamilies can be across threads? Writing this out somewhere is needed; otherwise the code is too subtle to keep working. 1724599 3 webkit-bug-importer 2021-02-02 03:06:32 -0800 <rdar://problem/73872757> 418387 2021-01-26 03:10:12 -0800 2021-01-26 09:33:28 -0800 Patch bug-220969-20210126111011.patch text/plain 7232 clord U3VidmVyc2lvbiBSZXZpc2lvbjogMjcxMjg5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjRlZjljYTNmNTk3MjA0 ZTllN2JlNGU5MmQ3YThmYmEyNjhiOGVhZi4uNWVlMTE0NDg5NzJlNDc1YzEzMTVmZmJkOTJhZGM0 N2E2NDhkOGY0MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2IEBACisyMDIxLTAxLTI2ICBDaHJp cyBMb3JkICA8Y2xvcmRAaWdhbGlhLmNvbT4KKworICAgICAgICBFbnN1cmUgRm9udEdlbmVyaWNG YW1pbGllcyB1c2VzIEF0b21TdHJpbmcgd2hlcmUgcG9zc2libGUKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyMDk2OQorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvbnZlcnQgdG8gQXRvbVN0cmluZyB3aGVu IHNldHRpbmcgZm9udCBnZW5lcmljIGZhbWlsaWVzIGluIFNldHRpbmdzQmFzZS4KKworICAgICAg ICBObyBuZXcgdGVzdHMsIG5vIGJlaGF2aW9yIGNoYW5nZS4KKworICAgICAgICAqIGNzcy9DU1NG b250U2VsZWN0b3IuY3BwOgorICAgICAgICAoV2ViQ29yZTo6cmVzb2x2ZUdlbmVyaWNGYW1pbHkp OgorICAgICAgICAqIHBhZ2UvU2V0dGluZ3NCYXNlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNl dHRpbmdzQmFzZTo6U2V0dGluZ3NCYXNlKToKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzQmFz ZTo6c2V0U3RhbmRhcmRGb250RmFtaWx5KToKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzQmFz ZTo6c2V0Rml4ZWRGb250RmFtaWx5KToKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzQmFzZTo6 c2V0U2VyaWZGb250RmFtaWx5KToKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzQmFzZTo6c2V0 U2Fuc1NlcmlmRm9udEZhbWlseSk6CisgICAgICAgIChXZWJDb3JlOjpTZXR0aW5nc0Jhc2U6OnNl dEN1cnNpdmVGb250RmFtaWx5KToKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzQmFzZTo6c2V0 RmFudGFzeUZvbnRGYW1pbHkpOgorICAgICAgICAoV2ViQ29yZTo6U2V0dGluZ3NCYXNlOjpzZXRQ aWN0b2dyYXBoRm9udEZhbWlseSk6CisKIDIwMjEtMDEtMjIgIENocmlzIExvcmQgIDxjbG9yZEBp Z2FsaWEuY29tPgogCiAgICAgICAgIFJlbW92ZSBpbmRpdmlkdWFsIHNldHRpbmdzIGFjY2Vzc29y cyBmcm9tIFdvcmtlckdsb2JhbFNjb3BlIGFuZCBhZGQgU2V0dGluZ3M6OlZhbHVlcyBhY2Nlc3Nv cgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvY3NzL0NTU0ZvbnRTZWxlY3Rvci5jcHAgYi9T b3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udFNlbGVjdG9yLmNwcAppbmRleCBjMGI5YmRiNjQ3MWI1 MTliMmM2ODMzMDMzYmUzYWFmYThmM2FkMTYxLi41MmZmY2JlMWJiYjNjZTk2OWU0MTYyNzRkYzU5 NTAwMzVkMTMwZGE1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udFNlbGVj dG9yLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTRm9udFNlbGVjdG9yLmNwcApAQCAt MjY0LDExICsyNjQsMTEgQEAgdm9pZCBDU1NGb250U2VsZWN0b3I6OmZvbnRDYWNoZUludmFsaWRh dGVkKCkKICAgICBkaXNwYXRjaEludmFsaWRhdGlvbkNhbGxiYWNrcygpOwogfQogCi1zdGF0aWMg T3B0aW9uYWw8QXRvbVN0cmluZz4gcmVzb2x2ZUdlbmVyaWNGYW1pbHkoRG9jdW1lbnQqIGRvY3Vt ZW50LCBjb25zdCBGb250RGVzY3JpcHRpb24mIGZvbnREZXNjcmlwdGlvbiwgY29uc3QgQXRvbVN0 cmluZyYgZmFtaWx5TmFtZSkKK3N0YXRpYyBPcHRpb25hbDxTdHJpbmc+IHJlc29sdmVHZW5lcmlj RmFtaWx5KERvY3VtZW50KiBkb2N1bWVudCwgY29uc3QgRm9udERlc2NyaXB0aW9uJiBmb250RGVz Y3JpcHRpb24sIGNvbnN0IEF0b21TdHJpbmcmIGZhbWlseU5hbWUpCiB7CiAgICAgYXV0byBwbGF0 Zm9ybVJlc3VsdCA9IEZvbnREZXNjcmlwdGlvbjo6cGxhdGZvcm1SZXNvbHZlR2VuZXJpY0ZhbWls eShmb250RGVzY3JpcHRpb24uc2NyaXB0KCksIGZvbnREZXNjcmlwdGlvbi5jb21wdXRlZExvY2Fs ZSgpLCBmYW1pbHlOYW1lKTsKICAgICBpZiAoIXBsYXRmb3JtUmVzdWx0LmlzTnVsbCgpKQotICAg ICAgICByZXR1cm4gcGxhdGZvcm1SZXN1bHQ7CisgICAgICAgIHJldHVybiBwbGF0Zm9ybVJlc3Vs dC5zdHJpbmcoKTsKIAogICAgIGlmICghZG9jdW1lbnQpCiAgICAgICAgIHJldHVybiBXVEY6Om51 bGxvcHQ7CkBAIC0yNzcsMTkgKzI3NywxOSBAQCBzdGF0aWMgT3B0aW9uYWw8QXRvbVN0cmluZz4g cmVzb2x2ZUdlbmVyaWNGYW1pbHkoRG9jdW1lbnQqIGRvY3VtZW50LCBjb25zdCBGb250RAogCiAg ICAgVVNjcmlwdENvZGUgc2NyaXB0ID0gZm9udERlc2NyaXB0aW9uLnNjcmlwdCgpOwogICAgIGlm IChmYW1pbHlOYW1lID09IHNlcmlmRmFtaWx5KQotICAgICAgICByZXR1cm4gQXRvbVN0cmluZyhz ZXR0aW5ncy5zZXJpZkZvbnRGYW1pbHkoc2NyaXB0KSk7CisgICAgICAgIHJldHVybiBzZXR0aW5n cy5zZXJpZkZvbnRGYW1pbHkoc2NyaXB0KTsKICAgICBpZiAoZmFtaWx5TmFtZSA9PSBzYW5zU2Vy aWZGYW1pbHkpCi0gICAgICAgIHJldHVybiBBdG9tU3RyaW5nKHNldHRpbmdzLnNhbnNTZXJpZkZv bnRGYW1pbHkoc2NyaXB0KSk7CisgICAgICAgIHJldHVybiBzZXR0aW5ncy5zYW5zU2VyaWZGb250 RmFtaWx5KHNjcmlwdCk7CiAgICAgaWYgKGZhbWlseU5hbWUgPT0gY3Vyc2l2ZUZhbWlseSkKLSAg ICAgICAgcmV0dXJuIEF0b21TdHJpbmcoc2V0dGluZ3MuY3Vyc2l2ZUZvbnRGYW1pbHkoc2NyaXB0 KSk7CisgICAgICAgIHJldHVybiBzZXR0aW5ncy5jdXJzaXZlRm9udEZhbWlseShzY3JpcHQpOwog ICAgIGlmIChmYW1pbHlOYW1lID09IGZhbnRhc3lGYW1pbHkpCi0gICAgICAgIHJldHVybiBBdG9t U3RyaW5nKHNldHRpbmdzLmZhbnRhc3lGb250RmFtaWx5KHNjcmlwdCkpOworICAgICAgICByZXR1 cm4gc2V0dGluZ3MuZmFudGFzeUZvbnRGYW1pbHkoc2NyaXB0KTsKICAgICBpZiAoZmFtaWx5TmFt ZSA9PSBtb25vc3BhY2VGYW1pbHkpCi0gICAgICAgIHJldHVybiBBdG9tU3RyaW5nKHNldHRpbmdz LmZpeGVkRm9udEZhbWlseShzY3JpcHQpKTsKKyAgICAgICAgcmV0dXJuIHNldHRpbmdzLmZpeGVk Rm9udEZhbWlseShzY3JpcHQpOwogICAgIGlmIChmYW1pbHlOYW1lID09IHBpY3RvZ3JhcGhGYW1p bHkpCi0gICAgICAgIHJldHVybiBBdG9tU3RyaW5nKHNldHRpbmdzLnBpY3RvZ3JhcGhGb250RmFt aWx5KHNjcmlwdCkpOworICAgICAgICByZXR1cm4gc2V0dGluZ3MucGljdG9ncmFwaEZvbnRGYW1p bHkoc2NyaXB0KTsKICAgICBpZiAoZmFtaWx5TmFtZSA9PSBzdGFuZGFyZEZhbWlseSkKLSAgICAg ICAgcmV0dXJuIEF0b21TdHJpbmcoc2V0dGluZ3Muc3RhbmRhcmRGb250RmFtaWx5KHNjcmlwdCkp OworICAgICAgICByZXR1cm4gc2V0dGluZ3Muc3RhbmRhcmRGb250RmFtaWx5KHNjcmlwdCk7CiAK ICAgICByZXR1cm4gV1RGOjpudWxsb3B0OwogfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGFnZS9TZXR0aW5nc0Jhc2UuY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9TZXR0aW5nc0Jhc2Uu Y3BwCmluZGV4IDZhZjc4NDM1YmU4NzkzNmYyOTA2ODI4N2M0YTFiNWVkMTBkZTZlNDYuLjk3ZmM2 ODQzYzRhMWZkYTcyMWUyZDdkMzE3MDc0MDg5YWNiNzdiNjMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL3BhZ2UvU2V0dGluZ3NCYXNlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL1Nl dHRpbmdzQmFzZS5jcHAKQEAgLTY1LDYgKzY1LDkgQEAgU2V0dGluZ3NCYXNlOjpTZXR0aW5nc0Jh c2UoUGFnZSogcGFnZSkKICAgICAsIG1fbWluaW11bURPTVRpbWVySW50ZXJ2YWwoRE9NVGltZXI6 OmRlZmF1bHRNaW5pbXVtSW50ZXJ2YWwoKSkKICAgICAsIG1fc2V0SW1hZ2VMb2FkaW5nU2V0dGlu Z3NUaW1lcigqdGhpcywgJlNldHRpbmdzQmFzZTo6aW1hZ2VMb2FkaW5nU2V0dGluZ3NUaW1lckZp cmVkKQogeworICAgIC8vIEEgRnJhbWUgbWF5IG5vdCBoYXZlIGJlZW4gY3JlYXRlZCB5ZXQsIHNv IHdlIGluaXRpYWxpemUgdGhlIEF0b21TdHJpbmcKKyAgICAvLyBoYXNoIGJlZm9yZSB0cnlpbmcg dG8gdXNlIGl0LgorICAgIEF0b21TdHJpbmc6OmluaXQoKTsKIH0KIAogU2V0dGluZ3NCYXNlOjp+ U2V0dGluZ3NCYXNlKCkgPSBkZWZhdWx0OwpAQCAtOTIsNyArOTUsNyBAQCBjb25zdCBTdHJpbmcm IFNldHRpbmdzQmFzZTo6c3RhbmRhcmRGb250RmFtaWx5KFVTY3JpcHRDb2RlIHNjcmlwdCkgY29u c3QKIAogdm9pZCBTZXR0aW5nc0Jhc2U6OnNldFN0YW5kYXJkRm9udEZhbWlseShjb25zdCBTdHJp bmcmIGZhbWlseSwgVVNjcmlwdENvZGUgc2NyaXB0KQogewotICAgIGJvb2wgY2hhbmdlcyA9IGZv bnRHZW5lcmljRmFtaWxpZXMoKS5zZXRTdGFuZGFyZEZvbnRGYW1pbHkoZmFtaWx5LCBzY3JpcHQp OworICAgIGJvb2wgY2hhbmdlcyA9IGZvbnRHZW5lcmljRmFtaWxpZXMoKS5zZXRTdGFuZGFyZEZv bnRGYW1pbHkoQXRvbVN0cmluZyhmYW1pbHkpLCBzY3JpcHQpOwogICAgIGlmIChjaGFuZ2VzKQog ICAgICAgICBpbnZhbGlkYXRlQWZ0ZXJHZW5lcmljRmFtaWx5Q2hhbmdlKG1fcGFnZSk7CiB9CkBA IC0xMDQsNyArMTA3LDcgQEAgY29uc3QgU3RyaW5nJiBTZXR0aW5nc0Jhc2U6OmZpeGVkRm9udEZh bWlseShVU2NyaXB0Q29kZSBzY3JpcHQpIGNvbnN0CiAKIHZvaWQgU2V0dGluZ3NCYXNlOjpzZXRG aXhlZEZvbnRGYW1pbHkoY29uc3QgU3RyaW5nJiBmYW1pbHksIFVTY3JpcHRDb2RlIHNjcmlwdCkK IHsKLSAgICBib29sIGNoYW5nZXMgPSBmb250R2VuZXJpY0ZhbWlsaWVzKCkuc2V0Rml4ZWRGb250 RmFtaWx5KGZhbWlseSwgc2NyaXB0KTsKKyAgICBib29sIGNoYW5nZXMgPSBmb250R2VuZXJpY0Zh bWlsaWVzKCkuc2V0Rml4ZWRGb250RmFtaWx5KEF0b21TdHJpbmcoZmFtaWx5KSwgc2NyaXB0KTsK ICAgICBpZiAoY2hhbmdlcykKICAgICAgICAgaW52YWxpZGF0ZUFmdGVyR2VuZXJpY0ZhbWlseUNo YW5nZShtX3BhZ2UpOwogfQpAQCAtMTE2LDcgKzExOSw3IEBAIGNvbnN0IFN0cmluZyYgU2V0dGlu Z3NCYXNlOjpzZXJpZkZvbnRGYW1pbHkoVVNjcmlwdENvZGUgc2NyaXB0KSBjb25zdAogCiB2b2lk IFNldHRpbmdzQmFzZTo6c2V0U2VyaWZGb250RmFtaWx5KGNvbnN0IFN0cmluZyYgZmFtaWx5LCBV U2NyaXB0Q29kZSBzY3JpcHQpCiB7Ci0gICAgYm9vbCBjaGFuZ2VzID0gZm9udEdlbmVyaWNGYW1p bGllcygpLnNldFNlcmlmRm9udEZhbWlseShmYW1pbHksIHNjcmlwdCk7CisgICAgYm9vbCBjaGFu Z2VzID0gZm9udEdlbmVyaWNGYW1pbGllcygpLnNldFNlcmlmRm9udEZhbWlseShBdG9tU3RyaW5n KGZhbWlseSksIHNjcmlwdCk7CiAgICAgaWYgKGNoYW5nZXMpCiAgICAgICAgIGludmFsaWRhdGVB ZnRlckdlbmVyaWNGYW1pbHlDaGFuZ2UobV9wYWdlKTsKIH0KQEAgLTEyOCw3ICsxMzEsNyBAQCBj b25zdCBTdHJpbmcmIFNldHRpbmdzQmFzZTo6c2Fuc1NlcmlmRm9udEZhbWlseShVU2NyaXB0Q29k ZSBzY3JpcHQpIGNvbnN0CiAKIHZvaWQgU2V0dGluZ3NCYXNlOjpzZXRTYW5zU2VyaWZGb250RmFt aWx5KGNvbnN0IFN0cmluZyYgZmFtaWx5LCBVU2NyaXB0Q29kZSBzY3JpcHQpCiB7Ci0gICAgYm9v bCBjaGFuZ2VzID0gZm9udEdlbmVyaWNGYW1pbGllcygpLnNldFNhbnNTZXJpZkZvbnRGYW1pbHko ZmFtaWx5LCBzY3JpcHQpOworICAgIGJvb2wgY2hhbmdlcyA9IGZvbnRHZW5lcmljRmFtaWxpZXMo KS5zZXRTYW5zU2VyaWZGb250RmFtaWx5KEF0b21TdHJpbmcoZmFtaWx5KSwgc2NyaXB0KTsKICAg ICBpZiAoY2hhbmdlcykKICAgICAgICAgaW52YWxpZGF0ZUFmdGVyR2VuZXJpY0ZhbWlseUNoYW5n ZShtX3BhZ2UpOwogfQpAQCAtMTQwLDcgKzE0Myw3IEBAIGNvbnN0IFN0cmluZyYgU2V0dGluZ3NC YXNlOjpjdXJzaXZlRm9udEZhbWlseShVU2NyaXB0Q29kZSBzY3JpcHQpIGNvbnN0CiAKIHZvaWQg U2V0dGluZ3NCYXNlOjpzZXRDdXJzaXZlRm9udEZhbWlseShjb25zdCBTdHJpbmcmIGZhbWlseSwg VVNjcmlwdENvZGUgc2NyaXB0KQogewotICAgIGJvb2wgY2hhbmdlcyA9IGZvbnRHZW5lcmljRmFt aWxpZXMoKS5zZXRDdXJzaXZlRm9udEZhbWlseShmYW1pbHksIHNjcmlwdCk7CisgICAgYm9vbCBj aGFuZ2VzID0gZm9udEdlbmVyaWNGYW1pbGllcygpLnNldEN1cnNpdmVGb250RmFtaWx5KEF0b21T dHJpbmcoZmFtaWx5KSwgc2NyaXB0KTsKICAgICBpZiAoY2hhbmdlcykKICAgICAgICAgaW52YWxp ZGF0ZUFmdGVyR2VuZXJpY0ZhbWlseUNoYW5nZShtX3BhZ2UpOwogfQpAQCAtMTUyLDcgKzE1NSw3 IEBAIGNvbnN0IFN0cmluZyYgU2V0dGluZ3NCYXNlOjpmYW50YXN5Rm9udEZhbWlseShVU2NyaXB0 Q29kZSBzY3JpcHQpIGNvbnN0CiAKIHZvaWQgU2V0dGluZ3NCYXNlOjpzZXRGYW50YXN5Rm9udEZh bWlseShjb25zdCBTdHJpbmcmIGZhbWlseSwgVVNjcmlwdENvZGUgc2NyaXB0KQogewotICAgIGJv b2wgY2hhbmdlcyA9IGZvbnRHZW5lcmljRmFtaWxpZXMoKS5zZXRGYW50YXN5Rm9udEZhbWlseShm YW1pbHksIHNjcmlwdCk7CisgICAgYm9vbCBjaGFuZ2VzID0gZm9udEdlbmVyaWNGYW1pbGllcygp LnNldEZhbnRhc3lGb250RmFtaWx5KEF0b21TdHJpbmcoZmFtaWx5KSwgc2NyaXB0KTsKICAgICBp ZiAoY2hhbmdlcykKICAgICAgICAgaW52YWxpZGF0ZUFmdGVyR2VuZXJpY0ZhbWlseUNoYW5nZSht X3BhZ2UpOwogfQpAQCAtMTY0LDcgKzE2Nyw3IEBAIGNvbnN0IFN0cmluZyYgU2V0dGluZ3NCYXNl OjpwaWN0b2dyYXBoRm9udEZhbWlseShVU2NyaXB0Q29kZSBzY3JpcHQpIGNvbnN0CiAKIHZvaWQg U2V0dGluZ3NCYXNlOjpzZXRQaWN0b2dyYXBoRm9udEZhbWlseShjb25zdCBTdHJpbmcmIGZhbWls eSwgVVNjcmlwdENvZGUgc2NyaXB0KQogewotICAgIGJvb2wgY2hhbmdlcyA9IGZvbnRHZW5lcmlj RmFtaWxpZXMoKS5zZXRQaWN0b2dyYXBoRm9udEZhbWlseShmYW1pbHksIHNjcmlwdCk7CisgICAg Ym9vbCBjaGFuZ2VzID0gZm9udEdlbmVyaWNGYW1pbGllcygpLnNldFBpY3RvZ3JhcGhGb250RmFt aWx5KEF0b21TdHJpbmcoZmFtaWx5KSwgc2NyaXB0KTsKICAgICBpZiAoY2hhbmdlcykKICAgICAg ICAgaW52YWxpZGF0ZUFmdGVyR2VuZXJpY0ZhbWlseUNoYW5nZShtX3BhZ2UpOwogfQo=