648197 - Remove ALERT_MISSING_FONTS code from nsMathMLChar.cpp

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Zack Weinberg (:zwol)]

The #ifdef ALERT_MISSING_FONTS logic in nsMathMLChar.cpp has been disabled for I don't know how long; would not have done anything useful even if enabled since bug 563114 landed; and is the sole remaining user of two nsIDeviceContext methods (GetLocalFontName and CheckFontExistence) that have never actually done what they claim to do in the Thebes implementation.  I propose to remove it, and to remove the surprisingly large amount of code in nsThebesDeviceContext.cpp that implements those methods, in the process causing device context instances to cost quite a bit more to create than necessary.

Comment 1

[Zack Weinberg (:zwol)]

I meant to mention that if this bug is accepted, bug 309090 should be WONTFIXed.

Comment 2

[Zack Weinberg (:zwol)]

Attachment: part 1: remove all #ifdef ALERT_MISSING_FONTS code

Does slightly more than what it says on the tin: the CheckFontExistence() helper function was not under #ifdef ALERT_MISSING_FONTS and was called in one place outside those #ifdefs, but thanks to nsThebesDeviceContext::CheckFontExistence() always returning NS_OK, it did nothing constructive.  Possibly more code in nsGlyphTable::ElementAt can be removed, I don't know.

Comment 3

[Zack Weinberg (:zwol)]

Attachment: part 2: make nsIDeviceContext::FirstExistingFont and ::GetLocalFontName into complete stubs; remove the alias table.

I overstated the value of removing this code -- the alias table is not created unless GetLocalFontName or FirstExistingFont is called, so it's not bulking up *every* device context in memory (well, only by one pointer).  Still, it's a lot of code doing nothing useful.

There is one other caller of FirstExistingFont in the tree: accessible/src/msaa/nsTextAccessibleWrap.cpp.  Thanks to CheckFontExistence already being a stub, this change does not change the actual behavior of FirstExistingFont and therefore should not cause problems for that code.  I suspect there are better ways for that to do what it's trying to do, but I don't know what they are and it's not on the critical path here.

Comment 4

[John Daggett (:jtd)]

Comment on attachment 524346 [details] [diff] [review]
part 2: make nsIDeviceContext::FirstExistingFont and ::GetLocalFontName into complete stubs; remove the alias table.

Switching the review to Jeff...

Comment 5

[Zack Weinberg (:zwol)]

FYI, the other use of FirstExistingFont, that I mentioned in comment 3, was removed in bug 648271.

Comment 6

[Zack Weinberg (:zwol)]

http://hg.mozilla.org/mozilla-central/rev/838238a0df33
http://hg.mozilla.org/mozilla-central/rev/0be26016e06a