Benoit Jacob [:bjacob] (mostly away) Reporter
	Description • 10 years ago

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

	Jonathan Kew [:jfkthame] Assignee
	Comment 1 • 10 years ago

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.

	Jonathan Kew [:jfkthame] Assignee
	Comment 2 • 10 years ago

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.

	Jonathan Kew [:jfkthame] Assignee
	Comment 3 • 10 years ago

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.

	Andrew McCreight [:mccr8]
	Comment 4 • 10 years ago

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

	John Daggett (:jtd)
	Comment 5 • 10 years ago

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/

	Andrew McCreight [:mccr8]
	Comment 6 • 10 years ago

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

	Jonathan Kew [:jfkthame] Assignee
	Comment 7 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 10 years ago

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