limit time spent in font loader RunLoader calls

RESOLVED FIXED in mozilla28

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

(Blocks 1 bug)

unspecified
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

The chrome hang data on bug 859558 shows a significant number of hangs within RunLoader calls.  For some reason I thought we already had an explicit time limit to prevent this but apparently not.

The gfxPlatformFontList::RunLoader method is called on a timer and loads the data for a pref-specified number of font families.  The pref can be tweaked but I think we should explicitly limit each pass to prevent hangs.
Limit the time spent within one pass of RunLoader to 500ms.
Attachment #8343444 - Flags: review?(roc)
Assignee: nobody → jdaggett
Comment on attachment 8343444 [details] [diff] [review]
patch, add time limit to RunLoader

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

::: gfx/thebes/gfxPlatformFontList.cpp
@@ +689,5 @@
>      mStartIndex = 0;
>      mNumFamilies = mFontFamiliesToLoad.Length();
>  }
>  
> +#define FONT_LOADER_MAX_TIMESLICE 500  // max time for one pass through RunLoader = 500ms

How about lowering this to 100ms?
Attachment #8343444 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)

> > +#define FONT_LOADER_MAX_TIMESLICE 500  // max time for one pass through RunLoader = 500ms
> 
> How about lowering this to 100ms?

Sure.
Blocks: 859558
https://hg.mozilla.org/mozilla-central/rev/dba508605c1b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Keywords: verifyme
Please re-add "verifyme" if you want QA to test the fix for this bug (details on how to reproduce it and the used tools would also be necessary in this case).
Keywords: verifyme
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.