Last Comment Bug 752394 - eliminate need to enumerate fonts for localized family names and postscript names
: eliminate need to enumerate fonts for localized family names and postscript n...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla31
Assigned To: John Daggett (:jtd)
:
Mentors:
Depends on: 947812 962440 964613
Blocks: 859558
  Show dependency treegraph
 
Reported: 2012-05-06 19:26 PDT by John Daggett (:jtd)
Modified: 2014-05-01 08:12 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1, use CTFontManager API to enumerate font families in OSX (2.90 KB, patch)
2013-12-17 23:54 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 1, use CTFontManager API to enumerate font families in OSX (2.89 KB, patch)
2014-01-16 00:16 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 1b - create static helper methods for fontinfo loading (15.01 KB, patch)
2014-01-16 00:16 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2a - move the gfxFontInfoLoader class into a separate file (14.44 KB, patch)
2014-01-16 00:18 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2b - set up async loader class (wip) (25.03 KB, patch)
2014-01-16 00:21 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 1b, v2 - create static helper methods for fontinfo loading (17.17 KB, patch)
2014-01-19 23:57 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2b, v3 - set up async loader class with osx implementation (56.69 KB, patch)
2014-01-19 23:59 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2c, v3 - font info loading implementation for DirectWrite (9.49 KB, patch)
2014-01-20 00:00 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2b, v4 - set up async loader class with osx implementation (56.21 KB, patch)
2014-01-20 23:21 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2c, v4 - font info loading implementation for DirectWrite (9.43 KB, patch)
2014-01-20 23:22 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
part 2d, v4 - font info loading for GDI (8.69 KB, patch)
2014-01-20 23:23 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, prevent facename list chrome hang (wip) (14.56 KB, patch)
2014-01-23 23:19 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, prevent facename list chrome hang (wip) (17.18 KB, patch)
2014-04-09 01:17 PDT, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, prevent facename list chrome hang (17.20 KB, patch)
2014-04-09 22:56 PDT, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, prevent chrome hang when searching for localized font family names (9.74 KB, patch)
2014-04-09 23:12 PDT, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
testcase, localized name test (557 bytes, text/html)
2014-04-09 23:16 PDT, John Daggett (:jtd)
no flags Details
testcase, facename lookups that may initially fail at startup (734 bytes, text/html)
2014-04-09 23:43 PDT, John Daggett (:jtd)
no flags Details
patch v3, prevent facename list chrome hang (15.92 KB, patch)
2014-04-16 23:12 PDT, John Daggett (:jtd)
jfkthame: review+
Details | Diff | Splinter Review
testcase, localized name test (829 bytes, text/html)
2014-04-16 23:49 PDT, John Daggett (:jtd)
no flags Details
patch v2, prevent chrome hang when searching for localized font family names (9.61 KB, patch)
2014-04-16 23:51 PDT, John Daggett (:jtd)
jfkthame: review+
Details | Diff | Splinter Review
followup - no need to use an nsString for a single character. (3.12 KB, patch)
2014-04-24 06:34 PDT, Jonathan Kew (:jfkthame)
jd.bugzilla: review+
Details | Diff | Splinter Review

Comment 1 FloRian-R 2013-04-19 21:28:03 PDT
Hello :jtd,

i had trouble recently with fonts and i wonder if this is related to this bug.

Background: I have around 12.000 Files in my Font Folder (Windows 7 64 bit, Firefox 20.0.1). Now, whenever i load a website that has a custom font or a google API font (examples: g2play.net, surrenderat20.net, about:downthemall) my Firefox freezes for 2-3 Minutes while loading the website and does not respond. I temporarely fixed it by setting gfx.downloadable_fonts.enabled to false.

I gladly volunteer as test subject if i can help in any way. Thank you in advance.
Comment 2 John Daggett (:jtd) 2013-11-28 21:56:17 PST
Latest results from Telemetry stats:
http://jsfiddle.net/69QNY/2/

For the past few months, using Firefox 24 release data:

InitFaceNameLists() takes    >5secs 10.5million times
InitOtherFamilyNames() takes >5secs  0.9million times

data: release/24/FONTLIST_INITFACENAMELISTS
Time(ms) spent on reading family names from all fonts

index	start	end	count	pct	cum pct
39	4111	5015	2030923	1.91	90.13
40	5015	6118	2018149	1.90	92.03
41	6118	7463	1897272	1.79	93.82
42	7463	9104	1568345	1.48	95.30
43	9104	11106	1217968	1.15	96.44
44	11106	13548	954160	0.90	97.34
45	13548	16527	733772	0.69	98.03
46	16527	20161	575310	0.54	98.57
47	20161	24593	448556	0.42	98.99
48	24593	30000	348238	0.33	99.32
49	>30000		719780	0.68	100.00

total counts: 106242667

data: release/24/FONTLIST_INITOTHERFAMILYNAMES
Time(ms) spent on reading other family names from all fonts

index	start	end	count	pct	cum pct
39	4111	5015	228836	1.76	92.93
40	5015	6118	192489	1.48	94.41
41	6118	7463	161659	1.24	95.65
42	7463	9104	134220	1.03	96.68
43	9104	11106	109671	0.84	97.53
44	11106	13548	88879	0.68	98.21
45	13548	16527	68463	0.53	98.73
46	16527	20161	51638	0.40	99.13
47	20161	24593	37183	0.29	99.42
48	24593	30000	26250	0.20	99.62
49	>30000		49772	0.38	100.00

total counts: 13018817
Comment 3 John Daggett (:jtd) 2013-11-28 21:57:37 PST
(In reply to FloRian-R from comment #1)
> Background: I have around 12.000 Files in my Font Folder (Windows 7 64 bit,
> Firefox 20.0.1). Now, whenever i load a website that has a custom font or a
> google API font (examples: g2play.net, surrenderat20.net, about:downthemall)
> my Firefox freezes for 2-3 Minutes while loading the website and does not
> respond. I temporarely fixed it by setting gfx.downloadable_fonts.enabled to
> false.

I think this is unrelated but if you're still seeing this problem with downloadable fonts enabled, we should probably log another bug and track down what the problem is.
Comment 4 FloRian-R 2013-11-29 02:25:22 PST
(In reply to John Daggett (:jtd) from comment #3)
> (In reply to FloRian-R from comment #1)
> > Background: I have around 12.000 Files in my Font Folder (Windows 7 64 bit,
> > Firefox 20.0.1). Now, whenever i load a website that has a custom font or a
> > google API font (examples: g2play.net, surrenderat20.net, about:downthemall)
> > my Firefox freezes for 2-3 Minutes while loading the website and does not
> > respond. I temporarely fixed it by setting gfx.downloadable_fonts.enabled to
> > false.
> 
> I think this is unrelated but if you're still seeing this problem with
> downloadable fonts enabled, we should probably log another bug and track
> down what the problem is.

I deleted around 80% of my fonts so it works without hangs at the moment, if it helps i can reinstall them and we can track it down.
Comment 5 John Daggett (:jtd) 2013-12-04 21:17:42 PST
Font hang data from bug 859558
https://bugzilla.mozilla.org/show_bug.cgi?id=859558#c5

InitFaceNameLists hangs: 51047
DirectWrite: 34436
GDI: 16611

Almost all of these are within a call to gfxUserFontSet::LoadNext, handling the loading of src: local(xxx) descriptors in @font-face rules.
Comment 6 John Daggett (:jtd) 2013-12-04 21:20:05 PST
Right now OSX provides OS functions for looking up <fullname, psname>.  Other platforms do not, so the font loader that runs on a timer after startup reads the name table of all fonts to pick up the <fullname, psname>.  The LookupLocalFont methods access a hashtable mExtras to map a <fullname, psname> to a font entry.

I think as a first pass here, the smart thing to do would be to:

1. Create a runnable that iterates over all font families on the system and reads the name table and comes up with a <fullname> ==> <family> mapping.  In the completion event this would be passed back to the platform font list.  This list would be strictly limited to string pairs (i.e. no underlying gfxFontEntry/gfxFontFamily usage).  I'm proposing something that would be platform-specific and *not* something that uses the methods associated with gfxFontEntry::GetTable.

2. When looking up local names, use the <fullname> ==> <family> mapping.  Then look up the family and make sure the individual faces have been read.  This will populate the local name lookup table (mExtras) and the existing method of looking up the font entry in the local name lookup table can be used.

3. When a local lookup occurs before the name lookup thread completes, record the name that failed and treat it as "not found" but note that it failed.  When the thread completes, restyle any content containing names that were found.

With these changes, the logic in LookupLocalFont methods would be more complicated but we could eliminate calls to InitFaceNameLists that cause chrome hangs.

There's a clear inefficiency here, namely that by iterating over the tables once, then reading those same tables again for a given family, we've duplicated reading the name table.  But that will only occur for the font families referenced via src local lookups, so the impact should be very limited.  Keeping this simple allows the off-main-thread name lookup in (1) to be independent of other table handling code and avoid the *very* painful effort of trying to make gfxFontEntry and gfxFontFamily and subclasses thread-safe.

In the DirectWrite case, we'll be able to eliminate the use of the font loader and only ever run through all fonts only when cmap font fallback is explicitly enabled.

One optimization would be to store the string list in the startup cache and update it when the fontlist changes (stat all font files listed in the registry and store the checksum for the moddates with the fontlist).  Last time I looked at this it wasn't really feasible, the startup cache is made for storing one-time startup data, not for data that changes over time.
Comment 7 John Daggett (:jtd) 2013-12-17 23:54:39 PST
Created attachment 8349235 [details] [diff] [review]
part 1, use CTFontManager API to enumerate font families in OSX

Instead of using the NSFontManager API, use the CoreText CTFontManager API. In general, I think we need to avoid AppKit API's within off-main-thread code.

I fiddled a bit with using the CTFontDescriptor API's to switch over FindStyleVariations but the "trait" info is a mess and mapping it to CSS values is harder than using the NSFontManager API.  This is basically the same problem as bug 931426.  So I'll probably leave this as is for now.
Comment 8 John Daggett (:jtd) 2014-01-16 00:16:02 PST
Created attachment 8360918 [details] [diff] [review]
part 1, use CTFontManager API to enumerate font families in OSX
Comment 9 John Daggett (:jtd) 2014-01-16 00:16:43 PST
Created attachment 8360919 [details] [diff] [review]
part 1b - create static helper methods for fontinfo loading
Comment 10 John Daggett (:jtd) 2014-01-16 00:18:07 PST
Created attachment 8360921 [details] [diff] [review]
part 2a - move the gfxFontInfoLoader class into a separate file

Move the gfxFontInfoLoader class over to a separate file so that later diffs are clearer
Comment 11 John Daggett (:jtd) 2014-01-16 00:21:56 PST
Created attachment 8360925 [details] [diff] [review]
part 2b - set up async loader class (wip)

Set up async loader thread and added loading code for fonts under OSX.  Next step is to fix up the finalize process to load in from the data provided by the async loader.  After that write the subclasses for FontInfoData for GDI, DirectWrite, Android.
Comment 12 John Daggett (:jtd) 2014-01-16 00:24:06 PST
Note that the code here is intended to implement async loading of localized names, fullnames/psnames and cmap data so it should also resolve the other "font hang" related bugs.
Comment 13 John Daggett (:jtd) 2014-01-19 23:57:59 PST
Created attachment 8362387 [details] [diff] [review]
part 1b, v2 - create static helper methods for fontinfo loading
Comment 14 John Daggett (:jtd) 2014-01-19 23:59:21 PST
Created attachment 8362388 [details] [diff] [review]
part 2b, v3 - set up async loader class with osx implementation
Comment 15 John Daggett (:jtd) 2014-01-20 00:00:16 PST
Created attachment 8362389 [details] [diff] [review]
part 2c, v3 - font info loading implementation for DirectWrite
Comment 16 John Daggett (:jtd) 2014-01-20 23:21:59 PST
Created attachment 8362824 [details] [diff] [review]
part 2b, v4 - set up async loader class with osx implementation
Comment 17 John Daggett (:jtd) 2014-01-20 23:22:37 PST
Created attachment 8362825 [details] [diff] [review]
part 2c, v4 - font info loading implementation for DirectWrite
Comment 18 John Daggett (:jtd) 2014-01-20 23:23:40 PST
Created attachment 8362826 [details] [diff] [review]
part 2d, v4 - font info loading for GDI
Comment 19 John Daggett (:jtd) 2014-01-20 23:38:24 PST
With the currently attached patches, the font loader that runs on desktop platforms has now been completely replaced by an async loader thread.  After the async loader has run, the finalize method calls a method to move the loaded data into the gfxFontFamily/gfxFontEntry data. This is done on the main thread in intervals if needed (the OSX cmap loading code has some complexity to it that's hard to completely push into an async thread but this will only affect users with a non-default pref setting).

Each one of the OSX/DirectWrite/GDI implementations implements a platform-specific LoadFontFamilyData method that loads all the needed data for a given font family without touching any other data structures in gfx (i.e. it calls only static helper methods and uses only thread-safe platform API's).  Upon loader completion, the gfxPlatformFontList::LoadFontInfo method updates gfx data structures with the loaded data.

Need to do some more testing/tryserver runs, etc.  The final piece for this bug is to implement a timeout in the code for looking up fullname/postscript names and and after the loader completes do a global reflow if and only if a font was found that was looked up previously.
Comment 20 John Daggett (:jtd) 2014-01-21 22:58:17 PST
Moved async font loader implementation over to bug 962440.
Comment 21 John Daggett (:jtd) 2014-01-23 23:19:19 PST
Created attachment 8364908 [details] [diff] [review]
patch, prevent facename list chrome hang (wip)

Adds a timeout to the InitFaceNameLists method. The missed name that sparked the initialization is saved. After the font loader thread completes, that name is checked again.  If that name can now be found, global reflow is triggered.

This doesn't quite work yet, global reflow isn't enough, we need to also get the userfont set to redo the specific face that triggered the local lookup.  In a similar vein, I suspect we don't behave correctly when faces are disabled -- the user font set will hold on to outdated font entries.  If a user types into an element that uses a local font specified via src: local(blah), then I think we will fail if 'blah' is explicitly disabled and the user then types in the same element.  Need to experiment more...
Comment 22 John Daggett (:jtd) 2014-04-09 01:17:56 PDT
Created attachment 8403833 [details] [diff] [review]
patch, prevent facename list chrome hang (wip)

Revised on top of the fix for bug 964613.

When looking up font face names before the lists have been fully initialized, first try to load all families starting with the same letter.  So "Georgia Bold" will cause all families beginning with 'G' to have their facenames loaded.  If that fails, all fonts are enumerated but only for a fixed time slot. If that times out, the lookup fails and the name is added to a set of missing face names. Once the font loader completes, these names are then checked again.  If any of those now match, userfont sets are checked (using the code from bug 964613) and pages reflowed.

This code will only be used on DirectWrite and GDI backends. Recent versions of DirectWrite provide cached access to the Postscript/fullname for fonts so given the logic above, a timeout that would affect display should almost never occur.
Comment 23 John Daggett (:jtd) 2014-04-09 22:56:18 PDT
Created attachment 8404447 [details] [diff] [review]
patch, prevent facename list chrome hang

Revised to latest trunk code.
Comment 24 John Daggett (:jtd) 2014-04-09 23:12:23 PDT
Created attachment 8404454 [details] [diff] [review]
patch, prevent chrome hang when searching for localized font family names

Similar approach here to the face name lists. When a localized name lookup needs to happen, try to enumerate font family names within the time allowed. If that doesn't work, fail.  After the async font loader completes, reflow to pick up the font that will now match (if such a font exists).

Unlike in the facename case, there's no quick and dirty way to target names that might be loaded.  If we want to do that we should cache the localized names from last time and use that.  But that seems like too much effort for a relatively rarely used feature.

This code is limited to XP and OSX.  Some users in Japan use the Japanese name for common Japanese fonts, so while we want to support these users to be compatible with IE, don't cause a chrome hang to do it.
Comment 25 John Daggett (:jtd) 2014-04-09 23:16:55 PDT
Created attachment 8404455 [details]
testcase, localized name test

Testcase for localized names.

Steps to test:

1. Settings > Preferences > General, select 'Show my windows from last time'
2. Load the test
3. Quit and restart the browser

Expected behavior: one of the lines will show initially in a serif face, then snap to a sans-serif face once the async font loader completes.

Note: existing trunk behavior will always show the same because we synchronously read all localized font names.
Comment 26 John Daggett (:jtd) 2014-04-09 23:43:48 PDT
Created attachment 8404471 [details]
testcase, facename lookups that may initially fail at startup

Same steps as the previous test. One of the lines may show in the "wrong" face (sans-serif or serif) depending upon the platform. Under OSX the name will probably always fail, on Windows it may often succeed depending upon how quickly the OS reads through fonts starting with 'c' in the name.
Comment 27 John Daggett (:jtd) 2014-04-10 17:02:19 PDT
tryserver build:
https://tbpl.mozilla.org/?tree=Try&rev=47b0f04eb17b
Comment 28 Jonathan Kew (:jfkthame) 2014-04-14 07:30:38 PDT
Comment on attachment 8404447 [details] [diff] [review]
patch, prevent facename list chrome hang

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

I found some of the method names here a bit confusing (see below); I'd like us to find ways to make things clearer before we land this.

::: gfx/thebes/gfxPlatformFontList.cpp
@@ +224,5 @@
> +    bool mFilter;
> +
> +    // if mFilter is true, only load facenames for families that
> +    // start with the string below
> +    nsString mFilterStr;

IIUC, you only filter on the initial letter. So just store that as a single char16_t. No need for the mFilter bool above, either; use mFilterChar==0 to indicate no filtering.

@@ +228,5 @@
> +    nsString mFilterStr;
> +};
> +
> +gfxFontEntry*
> +gfxPlatformFontList::InitFaceNameLists(const nsAString& aFontName)

This method isn't (just) an Init... method, it also does a lookup. As such, I think it should be named accordingly, including Lookup.

@@ +316,5 @@
> +    }
> +
> +    // lookup in name lookup tables, return null if not found
> +    if (!(lookup = LookupLocalFaceNameLists(aFontName))) {
> +        // names not completely initialized, so keep track of lookup misses

Either move the comment inside the if() below, or make the comment text itself express the condition: "if names not...then keep track..."

@@ +320,5 @@
> +        // names not completely initialized, so keep track of lookup misses
> +        if (!mFaceNameListsInitialized) {
> +            mFaceNamesMissed.PutEntry(aFontName);
> +        }
> +        return nullptr;

Redundant, |lookup| is null anyway if we're here.

@@ +959,5 @@
>      mFontFamiliesToLoad.Clear();
>      mNumFamilies = 0;
>  
> +    // if had missed face names that are now available, force reflow all
> +    bool checkUserFontSets = false;

This is redundant...

@@ +971,5 @@
> +        mFaceNamesMissed.Clear();
> +    }
> +
> +    if (checkUserFontSets) {
> +        CheckUserFontSetLocalFonts();

...if you simply do this inside the if()-block above in the case where mFoundName is true.

::: gfx/thebes/gfxPlatformFontList.h
@@ +246,5 @@
>                                                   nsRefPtr<gfxFontFamily>& aFamilyEntry,
>                                                   void* userArg);
>  
> +    // helper method for fullname/postscript name lookup
> +    gfxFontEntry* LookupLocalFaceNameLists(const nsAString& aFontName) {

I don't think this needs to be inlined in the header; move it to the .cpp file. Also, does it need to be public?

@@ +261,5 @@
> +    }
> +
> +    // look up a font by name, for cases where platform font list
> +    // maintains explicit mappings of fullname/psname ==> font
> +    virtual gfxFontEntry* LookupInFaceNameLists(const nsAString& aFontName);

It's not clear from either the method names or comments here why we have two methods that both take aFontName as a string and return a gfxFontEntry. Reading the interface here, it looks like they should be equivalent - but they're not. Could we clarify things here somehow, please?

@@ +329,5 @@
>      };
>      nsAutoPtr<ExtraNames> mExtraNames;
>  
> +    // face names missed when face name loading takes a long time
> +    nsTHashtable<nsStringHashKey> mFaceNamesMissed;

I suspect that in most cases, for most users, this will never be used (do you have any insight into how commonly it'll be hit?). As such, maybe we should make it a pointer-to-hashtable and only create it on demand.
Comment 29 John Daggett (:jtd) 2014-04-16 23:12:09 PDT
Created attachment 8408006 [details] [diff] [review]
patch v3, prevent facename list chrome hang

Revised based on review comments.

I cleaned up the names of the various methods and simplified the functionality at bit. Until face name list initialization has completed, only font families with the same first letter as the face name will be enumerated when searching for a face name.  On Windows, all standard fonts fit this pattern.  This will avoid a delay enumerating all font families looking for a font that's not available on the system.  If it is available and it doesn't fit the first letter pattern, the font will get picked up after the font loader runs.
Comment 30 John Daggett (:jtd) 2014-04-16 23:49:12 PDT
Created attachment 8408027 [details]
testcase, localized name test

Revised to clearly label Windows vs. OSX fonts.

Use steps described in comment 25.

Expected results: under given OS, should see serif face initially, then after the font loader completes (10-15 secs after startup), serif face should show. On Windows, this will only affect Meiryo, MS PGothic will always show correctly.
Comment 31 John Daggett (:jtd) 2014-04-16 23:51:06 PDT
Created attachment 8408028 [details] [diff] [review]
patch v2, prevent chrome hang when searching for localized font family names

Updated based on changes in facename handling patch.
Comment 32 John Daggett (:jtd) 2014-04-21 01:32:53 PDT
review ping...
Comment 33 Jonathan Kew (:jfkthame) 2014-04-22 13:46:43 PDT
Comment on attachment 8408006 [details] [diff] [review]
patch v3, prevent facename list chrome hang

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

r=me, with a couple minor fixes as noted.

::: gfx/thebes/gfxPlatformFontList.cpp
@@ +236,3 @@
>  
> +    // iterate over familes starting with the same letter
> +    faceNameListsData.mFirstChar = NS_ToLower(aFaceName.CharAt(0));

Shouldn't we use ToLowerCase (from nsUnicharUtils.h) here? NS_ToLower only takes a char argument.

@@ +266,5 @@
>  {
> +    ReadFaceNamesData *data = static_cast<ReadFaceNamesData*>(userArg);
> +    gfxPlatformFontList *fc = data->mFontList;
> +
> +    // when filtering, skip names that don't start with the filter string

s/string/character/

@@ +267,5 @@
> +    ReadFaceNamesData *data = static_cast<ReadFaceNamesData*>(userArg);
> +    gfxPlatformFontList *fc = data->mFontList;
> +
> +    // when filtering, skip names that don't start with the filter string
> +    if (data->mFirstChar && (NS_ToLower(aKey.CharAt(0)) != data->mFirstChar)) {

ToLowerCase()

@@ +931,5 @@
>      mNumFamilies = 0;
> +    bool rebuilt = false;
> +
> +    // if had missed face names that are now available, force reflow all
> +    if (NeedFullnamePostscriptNames() &&

This is redundant - if it's not true, there's no way mFaceNamesMissed will have been set.
Comment 34 Jonathan Kew (:jfkthame) 2014-04-22 14:02:08 PDT
Comment on attachment 8408028 [details] [diff] [review]
patch v2, prevent chrome hang when searching for localized font family names

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

r=me, with mOtherNamesMissed as a pointer rather than a direct member of gfxPlatformFontList.

::: gfx/thebes/gfxPlatformFontList.h
@@ +317,5 @@
>      // face names missed when face name loading takes a long time
>      nsAutoPtr<nsTHashtable<nsStringHashKey> > mFaceNamesMissed;
>  
> +    // localized family names missed when face name loading takes a long time
> +    nsTHashtable<nsStringHashKey> mOtherNamesMissed;

Make this an nsAutoPtr<...etc...>, like mFaceNamesMissed. In most sessions, I'm assuming it won't actually be used.
Comment 35 John Daggett (:jtd) 2014-04-22 22:22:11 PDT
Pushed to inbound, with changes based on review comments
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c4cbcc72ae4
https://hg.mozilla.org/integration/mozilla-inbound/rev/555db81b5607
Comment 37 Jonathan Kew (:jfkthame) 2014-04-24 06:34:54 PDT
Created attachment 8411799 [details] [diff] [review]
followup - no need to use an nsString for a single character.

Uhhh... looking at 0c4cbcc72ae4 as landed, it seems mFirstChar has become a string again. That wasn't what I meant at all - it seems quite unnecessary.
Comment 38 Jonathan Kew (:jfkthame) 2014-05-01 01:05:31 PDT
Pushed followup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7118f282f343
Comment 39 Ed Morley [:emorley] 2014-05-01 08:12:06 PDT
https://hg.mozilla.org/mozilla-central/rev/7118f282f343

Note You need to log in before you can comment on or make changes to this bug.