678813 - Outparamdel nsLayoutUtils::GetFontMetricsForFrame

Status: NEW

(Core :: Layout, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

We've got a chain of nsresult-returning functions

nsLayoutUtils::GetFontMetricsForFrame
nsLayoutUtils::GetFontMetricsForStyleContext
nsDeviceContext::GetMetricsFor
nsFontCache::GetMetricsFor
nsFontMetrics::Init

that only fails if gfxPlatform::CreateFontGroup returns a gfxFontGroup whose FontListLength() returns zero. However, gfxPlatform::CreateFontGroup calls the gfxFontGroup constructor, which, on Windows, OSX, and Android, calls gfxFontGroup::BuildFontList, which triggers an NS_RUNTIMEABORT in that case.

gfxPangoFontGroup calls mFonts.AppendElements(1) (which is infallible); gfxFT2FontGroup goes looking for a default font, and asserts it finds one; and gfxOS2FontGroup picks "Helv".

Furthermore, barely any callers of these functions actually check the return value.

Hence, I think it would be useful to have all these functions return already_AddRefed<nsFontMetrics> (like nsPresContext::GetMetricsFor does), and document that they won't return null.

Dbaron, is there anything here that doesn't sound right to you?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Sounds fine to me; at least wait until after bug 678671 lands.  Though, actually, part of the reason I just did bug 678671 is that I'm about to make a bunch of other changes to GetFontMetricsFor(Frame|StyleContext).  But if this is done with an automated tool it should be either (a) quick or (b) easy to rerun on top of that.