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

Status: RESOLVED FIXED

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

Description

[Andrew Halberstadt [:ahal]]

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:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e140b0227133502411e5437db9815f529d1436b2

Example log:
https://treeherder.mozilla.org/logviewer.html#?job_id=163440603&repo=try&lineNumber=48080

We'll need to either fix this before we can land run-by-manifest, or land with the 'asserts-if' annotations in place.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Jonathan, can you please take a look?

Comment 2

[Jonathan Kew [: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.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: 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

This fixes the assertion here (see https://treeherder.mozilla.org/#/jobs?repo=try&revision=0841c72497af6132d2badf97eb97d099280f71a0&group_state=expanded&selectedJob=163528027), 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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b85566707940
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

Comment 5

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b85566707940