After the checkin for bug 126919 none of these apis are used anywhere. See that
bug for more details.
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.
Created attachment 525578 [details] [diff] [review]
part 1: include minimization
Mostly-mechanical patch to remove all unnecessary #includes of ns*FontMetrics.h.
Created attachment 525579 [details] [diff] [review]
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.
Created attachment 525580 [details] [diff] [review]
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.
Created attachment 525581 [details] [diff] [review]
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]
Created attachment 525582 [details] [diff] [review]
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).
Created attachment 525583 [details] [diff] [review]
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.
Created attachment 525584 [details] [diff] [review]
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.
GetLanguage could return a raw nsIAtom*.
(In reply to comment #9)
> GetLanguage could return a raw nsIAtom*.
Good point; I'll tack on another patch to do that tomorrow morning.
Created attachment 526019 [details] [diff] [review]
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.