Bug 220969 – Ensure FontGenericFamilies uses AtomString where possible

• Status: NEW
• Component: WebCore Misc.

[Chris Lord]

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.

[Chris Lord]

Created attachment 418387 [details]
Patch

[Darin Adler]

Comment on attachment 418387 [details]
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.

[Radar WebKit Bug Importer]

<rdar://problem/73872757>