1542773 - Testcase from bug 1393259 could suffer from racy failures

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Jonathan Kew [:jfkthame]]

I think the testcase from bug 1393259, for access to a font installed in a non-default directory on macOS, may not always be reliable. The testcase tries to execute a sequence of operations:

(a) activate a font file in a non-standard directory
(b) wait for the browser to detect the change
(c) get the list of fonts used to render some content
(d) check that the expected (newly-activated) font is used

In particular, (b) is detected by observing the font.internaluseonly.changed pref, which gets modified when the font configuration changes and we build a new font list.

The trouble is that in a multi-process world, I don't think we have any guarantee that the content process rendering the test content will have fully handled the font configuration change by the time the parent process handles it, and bumps the internaluseonly.changed pref. So the testcase might proceed to (c) query for the fonts before the content process has caught up with the change.

Currently, we seem to be getting away with it; I guess this is because the font configuration change notification gets sent more-or-less simultaneously to all processes that have registered for it, and so it will most likely have been received by the content process before the parent finishes handling it and the testcase proceeds. I can't see that this is guaranteed, however, so there may be a potential intermittent failure hiding here; perhaps it could manifest if the system is particularly busy and dispatch of the system's font-change notification to some processes gets delayed.

The immediate problem, though, is that with the cross-process shared font list work in bug 1514869 this will become a perma-failure. That's because in the new model, only the parent process listens directly for font configuration changes; then, once it has processed the change, it notifies content processes. But this means there's more latency between the notification being handled by the parent, which then allows the testcase (as currently written) to proceed, and the update being received and handled fully by content processes.

To fix this, I propose modifying the testcase such that it doesn't rely on watching the internaluseonly.changed pref, which the parent will modify as soon as it handles the change, and instead watch the actual test content for a change in rendered width. This will happen once the updated font list has been noticed by the content process and the content reflowed accordingly.

Comment 1

[Jonathan Kew [:jfkthame]]

Ah, so it's been a bit intermittent already - I hadn't noticed that. It's not totally clear to me if the intermittents there are the same as this issue, or if they're more generally an intermittent failure to register/unregister the test font successfully. But anyhow, I don't think it can hurt to make the adjustment here.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1542773 - Make the mac font activation/sandbox test (browser_bug1393259.js) more robust. r=haik

When the test font is activated, the notification of the font configuration change
may be handled asynchronously by content processes, so that it's possible the content
process has not yet handled the update at the point when registerFont() detects the
change and returns to the test script.

(This issue becomes more acute with the upcoming shared-font-list system, where the
OS notification is not handled by the content process at all; it's only handled by
the parent process, which then notifies content processes after it has updated the
font list. So there's an inherent latency between the update being recognized by the
chrome process - and therefore "ready" as far as the test script is concerned - and
content processes receiving and handling the change.)

To handle this, we can explicitly wait for the width of the rendered content to change,
which will indicate that the font configuration change has been handled by the content
process.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c62f06c8c6a
Make the mac font activation/sandbox test (browser_bug1393259.js) more robust. r=haik

Comment 4

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9c62f06c8c6a