1028136 - Remove dangerous public destructor of FontFamilyList

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

In bug 1027251 we removed all dangerous public destructors of XPCOM-refcounted classes outside of a finite whitelist, see HasDangerousPublicDestructor. Now we are going over the entries in this whitelist.

One of them is: FontFamilyList

Comment 1

[Jonathan Kew [:jfkthame]]

This was introduced recently in bug 280443.

It gets a bit messy because FontFamilyList is used as a direct member (field) within nsFont, where it clearly does NOT want to be refcounted; but it's also used by the CSS parsing code, which DOES want to refcount it.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Remove dangerous public destructor of FontFamilyList.

Here's a possible solution: remove the refcounting from the FontFamilyList that nsFont uses, and then provide an additional, refcountable subclass (mozilla::css::RCFontFamilyList) for use within the CSS parsing code.

Comment 3

[Jonathan Kew [:jfkthame]]

Comment on attachment 8443568 [details] [diff] [review]
Remove dangerous public destructor of FontFamilyList.

Review of attachment 8443568 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/nsCSSValue.h
@@ +196,5 @@
>    GridTemplateAreasValue&
>    operator=(const GridTemplateAreasValue& aOther) MOZ_DELETE;
>  };
>  
> +class RCFontFamilyList : public FontFamilyList {

Oops, forgot to mark this MOZ_FINAL. Consider it added.

Comment 4

[Andrew McCreight [:mccr8]]

See bug 1028132 comment 3 and 4 for an explanation of how to fix this kind of bug.

Comment 5

[John Daggett (:jtd)]

Comment on attachment 8443568 [details] [diff] [review]
Remove dangerous public destructor of FontFamilyList.

Review of attachment 8443568 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with s/RCFontFamilyList/FontFamilyListRefCnt/

Comment 6

[Andrew McCreight [:mccr8]]

Woah, I totally missed that there was activity in this bug somehow when I was looking at it earlier today.

Comment 7

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bef8505f71e4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bef8505f71e4