Closed Bug 1028136 Opened 10 years ago Closed 10 years ago

Remove dangerous public destructor of FontFamilyList

Categories

(Core :: Graphics: Text, defect)

Product:
Core
Component:
Graphics: Text
Version:
Other Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34

People

(Reporter: bjacob, Assigned: jfkthame)

References

Details

(Whiteboard: [lang=c++])

Attachments

(1 file)

Remove dangerous public destructor of FontFamilyList.
(deleted), patch
jtd
: review+
Details | Diff | Splinter Review
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
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)
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)
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
Whiteboard: [lang=c++]
See bug 1028132 comment 3 and 4 for an explanation of how to fix this kind of bug.
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
https://hg.mozilla.org/mozilla-central/rev/bef8505f71e4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: