Outparamdel nsLayoutUtils::GetFontMetricsForFrame

NEW
Unassigned

Status

()

defect
8 years ago
8 years ago

People

(Reporter: Ms2ger, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
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?
Flags: in-testsuite-
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.
You need to log in before you can comment on or make changes to this bug.