174055 - deCOM nsIFontMetrics

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Christopher Blizzard (:blizzard)]

After the checkin for bug 126919 none of these apis are used anywhere.  See that
bug for more details.

Comment 1

[Zack Weinberg (:zwol)]

nsIWidget::SetFont seems to be gone, and I have a patch series deCOMming nsIFontMetrics that nukes GetFontHandle as a side effect, so I'm stealing this bug to carry that patch series.  Hope you don't mind.

Comment 2

[Zack Weinberg (:zwol)]

Attachment: part 1: include minimization

Mostly-mechanical patch to remove all unnecessary #includes of ns*FontMetrics.h.

Comment 3

[Zack Weinberg (:zwol)]

Attachment: part 2: merge nsIThebesFontMetrics into nsIFontMetrics

What it says.  Note I did not bump the nsIFontMetrics IID, because the very next patch makes the IID go away.

Comment 4

[Zack Weinberg (:zwol)]

Attachment: part 3: deCOM ns(I|Thebes)FontMetrics. Code outside gfx/ not fixed up.

This is the main deCOM patch.  gfx/ can be compiled at this point, but the mechanical fixups needed for the rest of the tree are in the next patch.

Comment 5

[Zack Weinberg (:zwol)]

Attachment: part 4: mechanical fixups

perl -pi -e 's/\bns(?:I|Thebes)(?=FontMetrics\b)/ns/g;
             s/\bnsCOMPtr(?=<nsFontMetrics>)/nsRefPtr/g' [every file in the tree]

Comment 6

[Zack Weinberg (:zwol)]

Attachment: part 5: prune unused and unimplemented methods.

One interesting thing about this patch is that I deleted GetHeight, which was used but documented as "will be removed once callers have been fixed".  It did the same thing as GetMaxHeight, so I changed all remaining callers to use that.  Also there are a couple of technically-unrelated removals of unused variables in here (I was tired of the warnings).

Comment 7

[Zack Weinberg (:zwol)]

Attachment: part 6: eliminate pointless nsresult return values.

Also rearranges code in nsFontMetrics.cpp a little so we can make use of the anonymous namespace for private classes.  The accessors that now return one nscoord also have the "Get" removed from their names.

Comment 8

[Zack Weinberg (:zwol)]

Attachment: part 7: remove unnecessary members from nsFontMetrics

All other places that call CreateFontGroup put the gfxFontStyle argument on the stack, and nothing else in nsFontMetrics uses the gfxFontStyle, so we don't need to heap-allocate that and keep it around for the lifetime of the font metrics.  mFontGroup is a refptr so it doesn't need to be nulled in the constructor.  And there were *two* booleans for whether text runs were right-to-left, one of which was never set to true (but that was the one that was actually getting *used*; I assumed this was just a mistake).

This is as far as this patch series goes.  I have work past this point, trying to disentangle nsFontMetrics from nsThebesDeviceContext, but it's still got bugs in it, and this is enough for one bug.  This set + the set in bug 266236 has passed the try server.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

GetLanguage could return a raw nsIAtom*.

Comment 10

[Zack Weinberg (:zwol)]

(In reply to comment #9)
> GetLanguage could return a raw nsIAtom*.

Good point; I'll tack on another patch to do that tomorrow morning.

Comment 11

[Zack Weinberg (:zwol)]

Attachment: part 8: GetLanguage() doesn't need to addref

Now I remember why I didn't do this in the first place: the only user is nsFontCache::GetMetricsFor, and I was originally going to have part 8 of this bug delete nsFontCache, so it seemed like unnecessary churn.  But that change turns out to expose a bunch of subtle bugs, so this is a better stopping place in case we wind up not being able to delete nsFontCache.

This series is waiting for one more review in bug 648197 before it can land, by the way.

Comment 12

[Zack Weinberg (:zwol)]

http://hg.mozilla.org/mozilla-central/rev/34e0e1992231
http://hg.mozilla.org/mozilla-central/rev/84c4a1cc3655
http://hg.mozilla.org/mozilla-central/rev/2397bc9933ea
http://hg.mozilla.org/mozilla-central/rev/d06f1282cc6d
http://hg.mozilla.org/mozilla-central/rev/b339ef823c29
http://hg.mozilla.org/mozilla-central/rev/90a2c72237e2
http://hg.mozilla.org/mozilla-central/rev/ea77c7167870
http://hg.mozilla.org/mozilla-central/rev/ee377a1a5e31