script: Re-initialize fulfilled document.fonts.ready when a web font starts loading - #45382
Merged
Merged
Conversation
…t starts loading This change adds an initial implementation of the "switch the FontFaceSet to loading" steps from the CSS Font Loading specification. The only thing it does is to reset `document.fonts.ready` so that it can be waited on again. In addition, this is called when the addition of a stylesheet or the DOM modification of a `FontFace` or `FontFaceSet` triggers more font loading. The goal here is to reduce flakiness in the CSS font loading tests. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
force-pushed
the
recreate-document-fonts-ready
branch
from
June 4, 2026 17:05
3900339 to
0d1e7dc
Compare
|
🔨 Triggering try run (#26967146250) for Linux (WPT) |
Member
Author
|
This change is quite large, but most of it is just threading |
|
Test results for linux-wpt from try job (#26967146250): Flaky unexpected result (60)
Stable unexpected results that are known to be intermittent (23)
|
|
✨ Try run (#26967146250) succeeded. |
mrobinson
marked this pull request as ready for review
June 4, 2026 17:42
simonwuelker
reviewed
Jun 4, 2026
Comment on lines
+649
to
+655
| // See <https://github.com/w3c/csswg-drafts/issues/13235>: | ||
| // All browsers switch the FontFaceSet to loading, but this is currently missing | ||
| // from the specification. | ||
| if let Some(font_face_set) = self.font_face_set.get() { | ||
| font_face_set.handle_font_face_status_changed(cx, self); | ||
| } | ||
|
|
| /// Fulfill the font ready promise, returning true if it was not already fulfilled beforehand. | ||
| pub(crate) fn fulfill_ready_promise_if_needed(&self, cx: &mut JSContext) -> bool { | ||
| if self.promise.is_fulfilled() { | ||
| let promise = self.promise.borrow().clone(); |
Member
There was a problem hiding this comment.
Why do we need to clone the promise here?
Member
Author
There was a problem hiding this comment.
I cloned the promise here to defensively avoid a potential borrow hazard during the promise fulfillment that follows. It's hard to reason about re-entrancy here, but I wanted to be sure.
simonwuelker
approved these changes
Jun 4, 2026
Member
Author
|
@simonwuelker Thanks for the review! |
niyabits
pushed a commit
to niyabits/servo
that referenced
this pull request
Jun 8, 2026
…t starts loading (servo#45382) This change adds an initial implementation of the "switch the FontFaceSet to loading" steps from the CSS Font Loading specification. The only thing it does is to reset `document.fonts.ready` so that it can be waited on again. In addition, this is called when the addition of a stylesheet or the DOM modification of a `FontFace` or `FontFaceSet` triggers more font loading. The goal here is to reduce flakiness in the CSS font loading tests. Testing: This causes some tests to pass and one to fail (due to missing preload support -- so this is progression actually). The main goal here though is to remove a lot of flakiness. Fixes: servo#37467. Fixes: servo#40929. Fixes: servo#32732. Fixes: servo#34624. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds an initial implementation of the "switch the
FontFaceSet to loading" steps from the CSS Font Loading specification.
The only thing it does is to reset
document.fonts.readyso that it canbe waited on again. In addition, this is called when the addition of a
stylesheet or the DOM modification of a
FontFaceorFontFaceSettriggers more font loading. The goal here is to reduce flakiness in the
CSS font loading tests.
Testing: This causes some tests to pass and one to fail (due to missing preload support -- so this is progression actually). The main goal here though is to remove a lot of flakiness.
Fixes: #37467.
Fixes: #40929.
Fixes: #32732.
Fixes: #34624.