628917 - (we_suck) Message handling code on Windows calls InitFontList multiple times

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

We suck, we really do.

Apparently, the event handling code on Windows handles a change in the font folder.  But somehow that message is sent out multiple times.  With 4 tabs open in a single window, the message is handled *18* times which causes InitFontList to get called each time.  I went back and tested with 3.6, same behavior.  Somehow we never caught this...

Note the bug is probably *not* in graphics but in Windows event handling code.  The code in nsWindows::ProcessMessage handles the WM_FONTCHANGE event here:

http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#4780

The proper title for this bug should be "message handling code on Windows calls InitFontList multiple times".  I'll change it later when I'm less depressed and am properly medicated...

Steps to reproduce:

1. Attach a debugger and set a breakpoint at InitFontList
2. Add a new font to the font folder

Result: InitFontList is called repeatedly

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(sucking or not aside, and while this is bad, adding a font to the font folder isn't exactly a common operation :)

Comment 2

[John Daggett (:jtd)]

(In reply to comment #1)
> (sucking or not aside, and while this is bad, adding a font to the font folder
> isn't exactly a common operation :)

Right. But I think that this message is also sent on dynamic font loads.  Bug 628152 is somehow a top crasher and it appears the only way to cause that is via a font load of some sort.  Simple pages with downloadable fonts don't seem to cause this message but I'm wondering if font loads via something like the JVM might be responsible.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: avoid rebuilding the font list repeatedly, by only handling WM_FONTCHANGE for the hidden window.

Seems like the simple fix here, as Windows is sending the font-change message to all our windows, is to make sure that only one of them actually calls InitFontList in response. If we let the hidden window do this, then all other windows can just ignore the message.

Comment 4

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb92322700ca

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/eb92322700ca