Closed Bug 648197 Opened 9 years ago Closed 9 years ago

Remove ALERT_MISSING_FONTS code from nsMathMLChar.cpp

Categories

(Core :: MathML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: zwol, Assigned: zwol)

References

Details

Attachments

(2 files)

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.
I meant to mention that if this bug is accepted, bug 309090 should be WONTFIXed.
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.
Assignee: nobody → zackw
Status: NEW → ASSIGNED
Attachment #524344 - Flags: review?(karlt)
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.
Attachment #524346 - Flags: review?(jdaggett)
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...
Attachment #524346 - Flags: review?(jdaggett) → review?(bugzmuiz)
Blocks: 174055
FYI, the other use of FirstExistingFont, that I mentioned in comment 3, was removed in bug 648271.
Depends on: 648271
Attachment #524344 - Flags: review?(karlt) → review+
http://hg.mozilla.org/mozilla-central/rev/838238a0df33
http://hg.mozilla.org/mozilla-central/rev/0be26016e06a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.