Bug 233971 – add assertions to check Font objects are used only on a single thread

• Status: NEW
• Component: Layout and Rendering

[Cameron McCormack (:heycam)]

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.

[Cameron McCormack (:heycam)]

Created attachment 446287 [details]
WIP

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.)

[Simon Fraser (smfr)]

I think the title is wrong?

[Chris Lord]

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?

[Cameron McCormack (:heycam)]

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>.

[Radar WebKit Bug Importer]

<rdar://problem/86504870>