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

In bug 1028588 we removed 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: nsComputedDOMStyle

	Nathan Froyd [:froydnj]
	Comment 1 • 10 years ago

Anuj, does this bug look like something you'd be interested in?

	Andrew McCreight [:mccr8] Assignee
	Comment 2 • 10 years ago

Where is the destructor called?  Bug 1029157 will at least fix the uses of the public destructor in the nsComputedDOMStyle.cpp itself.  That needs some perf testing.

	Andrew McCreight [:mccr8] Assignee
	Comment 3 • 10 years ago

I'll just do this.  Sorry Anuj that I keep taking your bugs, I'm just on a big cleanup pass of this style code. :)

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

try run: https://tbpl.mozilla.org/?tree=Try&rev=ff1ece740d55

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 10 years ago

Comment on attachment 8446042 [details] [diff] [review]
Make the destructor of nsComputedDOMStyle private.

r=me

	Daniel Holbert [:dholbert]
	Comment 6 • 10 years ago

Comment on attachment 8446042 [details] [diff] [review]
Make the destructor of nsComputedDOMStyle private.

>+++ b/layout/style/nsComputedDOMStyle.h
>-  virtual ~nsComputedDOMStyle();
[...]
> private:
>+  virtual ~nsComputedDOMStyle();
>+

Incidentally, do we know of any reason this needs to be virtual? (particularly now that we're Pretty Sure that the only thing deleting our instances should be our own ::Release method, which know our exact type.)

(We inherit from nsDOMCSSDeclaration, which has a (empty) virtual destructor, so to de-virtualize, we'd need to remove the annotation there as well.)

	Andrew McCreight [:mccr8] Assignee
	Comment 7 • 10 years ago

> Incidentally, do we know of any reason this needs to be virtual?
Yeah, I noticed that, but I didn't want to unravel that sweater...

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b42688bc9e9

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

https://hg.mozilla.org/mozilla-central/rev/5b42688bc9e9