Bug 628917 (we_suck)

Message handling code on Windows calls InitFontList multiple times

RESOLVED FIXED in mozilla20

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: jtd, Assigned: jfkthame)

Tracking

Trunk
mozilla20
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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

Updated

7 years ago
Alias: we_suck
Summary: we suck → Message handling code on Windows calls InitFontList multiple times
(sucking or not aside, and while this is bad, adding a font to the font folder isn't exactly a common operation :)
(Reporter)

Comment 2

7 years ago
(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.
(Assignee)

Comment 3

5 years ago
Created attachment 694059 [details] [diff] [review]
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.
Attachment #694059 - Flags: review?(jdaggett)
(Assignee)

Updated

5 years ago
Assignee: jdaggett → jfkthame
(Reporter)

Updated

5 years ago
Attachment #694059 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb92322700ca
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/eb92322700ca
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.