As part of implementing OffscreenCanvas, bug 202793 changed FontCache from a singleton to have an instance on the main thread and one on every worker thread. In this model, Font objects are also created and used on a single thread. I think we should add some assertions that Font objects are touched only on the thread that created them.
Created attachment 446287 [details]
Here's an incomplete patch that shows one approach, using the bits I'm adding in bug 233967. Unfortunately it's pretty noisy since the WTF_GUARDED_BY_LOCK attribute needs to be added on every member variable. An alternative would be to bundle up all the data members into a struct and have a single member variable with WTF_GUARDED_BY_LOCK on it, but that would cause corresponding noise on all member variable references. Sadly wrapping all the data members in an anonymous struct prevents WTF_GUARDED_BY_LOCK from being used on it in a single place. (It would be nice if a single annotation could be added on the class itself.)
I think the title is wrong?
This seems a bit overkill to me vs just guarding on the functions that provide Font objects - there's no point where we transfer Fonts between threads or access Fonts from different threads from the FontCache that provided them are there?
Guarding on the places that provide the Font objects might be a better balance between safety (and ability to rely on the static analysis attributes) and noisiness.
And you're right, I don't think we do explicitly transfer Fonts between threads. I guess I just want to protect against future code changes that might provide a different way to get access to Font objects.
A different approach could be to have some handle class that wraps a raw Font*, asserts in its operator->, and doesn't provide a way to get at the raw pointer. The static constructor functions are the only way to create Font instances and would return Ref<FontHandle>.