[webkit-reviews] review granted: [Bug 73688] REGRESSION (r91738): didFinishLoad is called before custom fonts have finished loading : [Attachment 117684] Ensure that font loads that are waiting to begin prevent didFinishLoad from being called

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 73688: REGRESSION (r91738): didFinishLoad is called before custom fonts
have finished loading
https://bugs.webkit.org/show_bug.cgi?id=73688

Attachment 117684: Ensure that font loads that are waiting to begin prevent
didFinishLoad from being called
https://bugs.webkit.org/attachment.cgi?id=117684&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117684&action=review


> Source/WebCore/css/CSSFontSelector.cpp:583
> +    if (!m_fontsToBeginLoading.isEmpty()) {

Do we really need this optimization? It seems to me that the code inside this
if would do nothing relatively quickly.

> Source/WebCore/css/CSSFontSelector.cpp:607
> +    // Increment the request count now, in order to prevent didFinishLoad
from being dispatched
> +    // after this font has been requested but before it began loading.
Balanced by
> +    // decrementRequestCount() in beginLoadTimerFired() and in
clearDocument().
> +    m_document->cachedResourceLoader()->incrementRequestCount(font);

Another way to do this is to create a type that does the increment/decrement
and holds a CachedResourceHandle<CachedFont> inside it, then use that type in
the vector. A little more foolproof than having the explicit code in three
places, but not sure it’s better. It would completely eliminate the loop in
CSSFontSelector::clearDocument.