Closed Bug 1439954 Opened 2 years ago Closed 2 years ago

Intermittent run-by-manitest OSX debug assertion from various tests in layout/reftests/font-matching/reftest.list


(Core :: Layout: Text and Fonts, enhancement)

Not set



Tracking Status
firefox60 --- fixed


(Reporter: ahal, Assigned: jfkthame)




(1 file)

In bug 1353461 we are in the process of switching reftest to "run-by-manifest" mode. This means that between every manifest of tests we will restart Firefox. In general this leads to improved stability, though in a few cases existing tests that happened to depend on side-effects of previous tests can start failing. This is one of those cases.

This is very likely related to bug 1439937. Basically there appears to be an intermittent assertion (which happens more often than not), somewhere in or after the layout/reftests/font-family/localized-family-names-00{1-4}.html tests. So I think which test it happens in is just down to timing and luck.

The further we get from localized-family-names-001.html, the less likely it is to show up. I've seen it in weightmapping-45.html, but never weightmapping-458.html (though I'm sure it's possible).

Here is a push demonstrating this:

Example log:

We'll need to either fix this before we can land run-by-manifest, or land with the 'asserts-if' annotations in place.
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
Jonathan, can you please take a look?
Flags: needinfo?(jfkthame)
Looks like the problem is that when the gfxPlatformFontList code finishes loading font info (done in the background shortly after startup), it calls ForceGlobalReflow() to ensure that all content is reflowed in order to take account of font info that may not have been available when initially reflowed.

But ForceGlobalReflow() works by touching a preference that all presContext's observe for changes; and we can't do that from a content process. So it asserts.

Really, however, ForceGlobalReflow() is primarily meant to be used by the parent process when something in the system environment has changed (e.g. the user installed new fonts on their OS), such that everything needs to be reflowed (in all content processes, as well as the chrome process). That's not the case here: when a child process finishes loading font info, it should force a reflow only for its own content, not globally for all processes.

So what we need is an alternative that works in a content process and forces all its presContexts (but not other processes) to reflow.
Flags: needinfo?(jfkthame)
This fixes the assertion here (see, by making gfxPlatform::ForceGlobalReflow safely usable in the content process. Note that it doesn't prevent these font-matching tests failing with a mismatch, however; that's a related issue, to be considered in bug 1439937.
Attachment #8952880 - Flags: review?(jmuizelaar)
Assignee: nobody → jfkthame
Attachment #8952880 - Flags: review?(jmuizelaar) → review+
Pushed by
Make gfxPlatform::ForceGlobalReflow when called in a content process trigger reflows only within that process; it should not try (and fail, with an assertion) to affect the parent or other content processes. r=jrmuizel
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.