1145506 - uninitialized FontFace objects added to the FontFaceSet do not start loading due to font matching

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

STR:

1. Create a new FontFace object and immediately add it to the FontFaceSet.
2. Add some text to the document that should cause the FontFace to load.
3. Flush layout.
4. Expect the FontFace to have status "loading", but it is "unloaded".

Because we haven't gone around the event loop yet, the FontFace object is still uninitialized (which means we haven't parsed its src).  So when we flush layout, we skip creating and adding a user font entry for the FontFace.

I think to simplify things, we should create user font entries even for FontFace objects that aren't initialized yet.  This will need some tracking in the gfxUserFontEntry to ensure that we update the src list once the FontFace is initialized, but that shouldn't be too hard.  This will also let us remove the mShouldLoadWhenInitialized handling in FontFace, since calling Load() on an uninitialized FontFace will now have a user font entry to start loading.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Create user font entries for uninitialized FontFaces and update their src lists once they are initialized.

Comment 2

[John Daggett (:jtd)]

Comment on attachment 8581105 [details] [diff] [review]
Create user font entries for uninitialized FontFaces and update their src lists once they are initialized.

I think the complicated uninitialized/pending/invalid state changes for source types is unnecessary. But I think they follow from the spec wording, so the spec wording should probably change.

I posted that src parsing for strings should follow the behavior of family/descriptor parsing -- if invalid, set status to "error" and simply reject with a syntax error. No need to queue tasks and deal with odd intermediate states.

https://lists.w3.org/Archives/Public/www-style/2015Mar/0493.html

I should also note that what I'm suggesting here is exactly what the current Chrome implementation does. Yay, interop...

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Make FontFace constructor fail on invalid src strings but otherwise create user font entries immediately.

This simplifies a lot!

Comment 4

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30ecd5a0f32e

Comment 5

[John Daggett (:jtd)]

Comment on attachment 8584285 [details] [diff] [review]
Make FontFace constructor fail on invalid src strings but otherwise create user font entries immediately.

Review of attachment 8584285 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!

Comment 6

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1df07e80e3c7

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1df07e80e3c7