1563381 - Fix race in test_font_whitelist and remove workarounds/skip-if

Status: NEW

(Core :: Graphics, defect, P3)

Description

[Kris Maglione [:kmag]]

There's a race in the various bits of IPC involved in the whitelist preference changes in test_font_whitelist which started causing problems after we removed the synchronous IPC from the SpecialPowers preference APIs.

I haven't worked out all of the details yet, but essentially the problem is this:

In the working cases, we rely on getting calls to Document::MarkUserFontSetDirty(), gfxPlatform::UpdateFontList(), and Document::FlushUserFontSet() in that order.

The MarkUserFontSetDirty call comes from nsPresContext::RebuildAllStyleData when it's called from nsPresContext::UpdateAfterPreferencesChanged(). The UpdateFontList() call comes from IPC. I haven't checked exactly what triggers the FlushUserFontSet() call, but it's presumably triggered by a delayed restyle triggered by the pref change flush.

In the bad case, though, we get Document::MarkUserFontSetDirty() followed by Document::FlushUserFontSet() and then gfxPlatform::UpdateFontList(). The gfxPlatform::UpdateFontList() call itself does not end up marking the document fonts dirty or triggering a restyle.

It looks like this is supposed to happen via gfxPlatformFontList::RebuildLocalFonts() called from `gfxPlatformFontList::UpdateFontList()](https://searchfox.org/mozilla-central/rev/11712bd3ce7454923e5931fa92eaf9c01ef35a0a/gfx/thebes/gfxPlatformFontList.cpp#1880), but for some reason it does not.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a defect, but please change it back in case of error.

Comment 2

[Kris Maglione [:kmag]]

Jonathan, can you please look at this and figure out what needs to be done, or find someone else to? I've worked around the test failures, but this race probably shows up after preference changes in the real world too, and I'm not sure how important it is.

Comment 3

[Jonathan Kew [:jfkthame]]

(In reply to Kris Maglione [:kmag] from comment #2)

Jonathan, can you please look at this and figure out what needs to be done, or find someone else to? I've worked around the test failures, but this race probably shows up after preference changes in the real world too, and I'm not sure how important it is.

Sorry, I haven't had cycles to really look into this yet; leaving the needinfo? for now to remind me about it.

In practice this is unlikely to matter much, even if there's a possible race that could result in temporarily inconsistent behavior. This isn't a pref people are going to be changing on the fly (except for experimentation purposes); it exists mostly so that packages like Tor can set it once for their distribution, and leave it alone. Users tweaking it would defeat the whole point of having a standardized list to reduce fingerprinting entropy.