Closed
Bug 1030829
Opened 11 years ago
Closed 11 years ago
support preloading of "hidden" fonts into the user-font cache with data: URIs
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: jfkthame, Assigned: jfkthame)
References
Details
Attachments
(2 files, 2 obsolete files)
Once we fix bug 1030067, making data-URI entries in the user-font cache independent of the principal from which they were originally loaded, we can use this to provide a "preloaded" version of Gaia-icons (or any other font we want to preload on FirefoxOS without exposing directly to app/web content as an installed font).
General plan here:
We'll have a directory on FxOS such as /system/fonts/hidden/ where Gaia-icons.ttf can be installed. When we initialize the normal system font list, we'll also scan the hidden directory, and create font entries for any fonts found there; but instead of including them in the platform font list, we'll build data: URIs for them and preload them into the user font cache.
The "preloading" of such fonts into the user-font cache will happen before the Nuwa fork, so that the (potentially rather large) data: URI doesn't have to be created separately by each process.
Then any app or page that uses @font-face with the same data: URI will find the already-cached font entry, and use this instead of decoding and sanitizing its own separate copy. This should make it almost equally cheap to use such a font via its data: URI as to use a preinstalled platform font.
If such an app or page is loaded on a system that doesn't have the preloaded font, it'll still work transparently (though not quite as quickly), because the data: URI will be processed as normal.
Assignee | ||
Comment 1•11 years ago
|
||
For these "preloaded" entries, I think we want to avoid ever deleting them from the cache (as this isn't a case of caching in the usual sense, it's just a convenient way to manage these faces). So this patch adds a 'persistent' flag that we can use to ensure they stick around.
Attachment #8446626 -
Flags: review?(roc)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
This seems to work in my testing; with a copy of Gaia-icons.ttf installed in /system/fonts/hidden, pages that use @font-face with the appropriate data URI end up directly using the existing font entry. r?roc
Attachment #8446630 -
Flags: review?(roc)
Assignee | ||
Comment 3•11 years ago
|
||
Instrumenting my Peak build shows that with these patches, the time spent in gfxUserFontSet::LoadNext for a small (8K .ttf) icon font is reduced from around 5ms (roughly; there's quite a bit of noise) to well under 0.5ms.
The benefit should be correspondingly greater for larger fonts, where the sanitization process - which we bypass here - will be more expensive, and for slower devices such as Tarako.
Attachment #8446626 -
Flags: review?(roc) → review+
Comment on attachment 8446630 [details] [diff] [review]
part 2 - preload 'hidden' fonts on FirefoxOS into the user-font cache.
Review of attachment 8446630 [details] [diff] [review]:
-----------------------------------------------------------------
Basically looks good.
There are a lot of boolean parameters which make the code hard to read. I think we should convert them to named flags.
::: gfx/thebes/gfxFT2FontList.cpp
@@ +1395,5 @@
> + if (!f) {
> + continue;
> + }
> + struct stat buf;
> + if (fstat(fileno(f), &buf) != 0 || buf.st_size < 12) {
Wouldn't it be better to mmap these files? I think it would.
Attachment #8446630 -
Flags: review?(roc) → review-
Assignee | ||
Comment 5•11 years ago
|
||
Updated patch 1 to use a named flag instead of a boolean for cache-entry persistence. Carrying over r=roc.
Assignee | ||
Updated•11 years ago
|
Attachment #8446626 -
Attachment is obsolete: true
Assignee | ||
Comment 6•11 years ago
|
||
Replaced bools with named flags in gfxFT2FontList, and use mmap to read the font data.
Attachment #8447214 -
Flags: review?(roc)
Assignee | ||
Updated•11 years ago
|
Attachment #8446630 -
Attachment is obsolete: true
Comment on attachment 8447214 [details] [diff] [review]
part 2 - preload 'hidden' fonts on FirefoxOS into the user-font cache.
Review of attachment 8447214 [details] [diff] [review]:
-----------------------------------------------------------------
Delightful!
::: gfx/thebes/gfxFT2FontList.cpp
@@ +1404,5 @@
> +
> + // XXX Should we move the i/o here off the main thread?
> +
> + // Map the font data in fe->mFilename, so we can generate a data: URI.
> + int fd = open(fe->mFilename.get(), O_RDONLY);
I wonder if this "open" call is actually going to work in the sandbox. Need feedback from Jed.
Attachment #8447214 -
Flags: review?(roc)
Attachment #8447214 -
Flags: review+
Attachment #8447214 -
Flags: feedback?(jld)
Comment 8•11 years ago
|
||
Comment on attachment 8447214 [details] [diff] [review]
part 2 - preload 'hidden' fonts on FirefoxOS into the user-font cache.
Review of attachment 8447214 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/thebes/gfxFT2FontList.cpp
@@ +1404,5 @@
> +
> + // XXX Should we move the i/o here off the main thread?
> +
> + // Map the font data in fe->mFilename, so we can generate a data: URI.
> + int fd = open(fe->mFilename.get(), O_RDONLY);
Where are these files? Content processes already open files under /system/fonts while sandboxed, so if it's there then at least this wouldn't be making things any worse.
Attachment #8447214 -
Flags: feedback?(jld)
Assignee | ||
Comment 9•11 years ago
|
||
They'll be under /system/fonts/hidden. (That's not "hidden" in any filesystem sense, obviously; this just means they're "hidden" from the main font list, which contains only fonts directly in /system/fonts.)
Assignee | ||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/57eebbb377ec
https://hg.mozilla.org/integration/mozilla-inbound/rev/7790d48f1451
Target Milestone: --- → mozilla33
https://hg.mozilla.org/mozilla-central/rev/57eebbb377ec
https://hg.mozilla.org/mozilla-central/rev/7790d48f1451
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•