Remove dangerous public destructor of FontFamilyList

RESOLVED FIXED in mozilla34

Status

()

Core
Graphics: Text
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bjacob, Assigned: jfkthame)

Tracking

Other Branch
mozilla34
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=c++])

Attachments

(1 attachment)

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
Flags: needinfo?(jfkthame)
(Assignee)

Comment 1

3 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.
Blocks: 280443
Flags: needinfo?(jfkthame)
(Assignee)

Comment 2

3 years ago
Created attachment 8443568 [details] [diff] [review]
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.
Attachment #8443568 - Flags: review?(jdaggett)
(Assignee)

Comment 3

3 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.
Mentor: continuation@gmail.com
Whiteboard: [lang=c++]
See bug 1028132 comment 3 and 4 for an explanation of how to fix this kind of bug.

Comment 5

3 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/
Attachment #8443568 - Flags: review?(jdaggett) → review+
Woah, I totally missed that there was activity in this bug somehow when I was looking at it earlier today.
Assignee: nobody → jfkthame
Mentor: continuation@gmail.com
(Assignee)

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bef8505f71e4
Target Milestone: --- → mozilla34
https://hg.mozilla.org/mozilla-central/rev/bef8505f71e4
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.