652754 - pages containing korean cause font tables to be read at startup

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

Code was added for FF4 to try and avoid having to enumerate localized font names if at all possible.  Unfortunately, there's still a scenario under which those fonts will be enumerated: when using GDI font rendering, if a page contains Korean with no font specified, we look up pref fonts for Korean.  Our pref font setting for Korean uses the localized name, which in turns kicks off the enumeration process.  The fix is to use the en-us name and make sure our UI code is provided with the localized name.

Steps to reproduce:

1. Start FF4 and enter about:config
2. Set 'gfx.directwrite.disableD2D' to true to assure GDI
3. Set www.facebook.com as the default home page
4. Quit Firefox
5. Run ProcessMonitor
6. Start Firefox
7. After the page loads, switch to ProcessMonitor

Result: ProcessMonitor shows that all fonts on the system have been enumerated (at cold startup this will include a read but on warm startup only a stat call will occur).

Note: this is currently blocked by 651498, gfx code is not currently handling pref settings correctly at startup (Nightly builds on 4/20 onwards).

Comment 1

[John Daggett (:jtd)]

Attachment: patch, eliminate unnecessary font data enumerations at startup

Part of this patch is cleanup, trimming out unused member variables.  mUnicodeFont and mSymbolFont were initialized in the GDI font entry case by reading the cmap.  I trimmed mUnicodeFont and made IsSymbolFont() call to check to make sure the cmap is loaded when needed.  This eliminates unneeded cmap loads when enumerating the faces in a given family.

The other part was to switch over to using the en-us names of pref fonts.  This eliminates the "miss on localized name, load localized names, lookup on localized name" that was causing name table reads to occur at startup.  The testcase was hitting this path by containing "Language" UI strings in Korean.  This would then fallback to the pref fonts for Korean resulting in a localized name lookup miss.

Without the patch, the GDI path loads 19MB of font data at cold startup for the testcase, 2.3MB with the patch.

Testing the fix requires a fix for the gfx prefs init problem (bug 651498) but that should be fixed in the next couple days.

Comment 2

[John Daggett (:jtd)]

We will mourn the passing of the 'IsCrappyFont' method...

Comment 3

[John Daggett (:jtd)]

Attachment: patch v2, eliminate unnecessary font data enumerations at startup

Note that some of the names in all.js were spelled incorrectly (i.e. didn't match the actual font names!).  A couple others were mislabeled in the comments.  I went through and verified with a testcase that the English names match the identical font matched using the localized name.  Also trimmed out a bunch of trailing white space.

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 531242 [details] [diff] [review]
patch v2, eliminate unnecessary font data enumerations at startup

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

Looks fine to me.

::: modules/libpref/src/init/all.js
@@ +1434,5 @@
>  pref("font.name-list.cursive.he", "Guttman Yad, Ktav, Arial");
>  
> +pref("font.name.serif.ja", "MS PMincho"); // "MS PMincho"
> +pref("font.name.sans-serif.ja", "MS PGothic"); // "MS PGothic"
> +pref("font.name.monospace.ja", "MS Gothic"); // "MS Gothic"

These comments (and similar ones later) are now completely redundant; I'd suggest stripping them.

Comment 5

[John Daggett (:jtd)]

Pushed to trunk
http://hg.mozilla.org/mozilla-central/rev/2e0e36b0feae

Comment 6

[Jonathan Kew [:jfkthame]]

Re-opening; backed out (by jtd) because of Linux64 build redness:
http://hg.mozilla.org/mozilla-central/rev/03a2ba248791

Log of the failed job:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1305871997.1305874164.17473.gz

obj-firefox/dist/bin/leakstats starting at Thu May 19 16:31:29 2011
Unknown event type 0x1
obj-firefox/dist/bin/leakstats: log file incomplete
program finished with exit code 1

That looks to me like an intermittent build or infra failure rather than a code issue. So I re-triggered that job, and it's green on the second attempt:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1305880622.1305883030.22415.gz

Comment 7

[Jonathan Kew [:jfkthame]]

Re-landed the original patch, as I don't believe it was at fault:
http://hg.mozilla.org/mozilla-central/rev/f2cac6e5a444