1028134 - Remove dangerous public destructor of gfxContext

Status: RESOLVED FIXED

(Core :: Graphics, 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: gfxContext

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Remove dangerous public destructor of gfxContext.

AFAICS, there's no need for this destructor to be public; we can simply move it to private and drop it from the whitelist.

Comment 2

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

Comment on attachment 8443683 [details] [diff] [review]
Remove dangerous public destructor of gfxContext.

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

Great, I somehow got confused into thinking that this would not be so trivial, but if this builds on tryserver then I was wrong. (FWIW when I tried locally that was on Linux).

Comment 3

[Jonathan Kew [:jfkthame]]

It builds for me on OS X; I've sent it to tryserver to see what the other platforms think: https://tbpl.mozilla.org/?tree=Try&rev=3054fc5aeaee.

Comment 4

[Jonathan Kew [:jfkthame]]

Turns out you're right, Linux doesn't like it. :(

Comment 5

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

IIRC BenWa might have ideas about why gfxContexts are managed not-by-refcounting.

Comment 6

[Jonathan Kew [:jfkthame]]

The gfxPlatformGtk issue, at least, looks like an easy fix. But I'll wait to hear from the other platforms before posting a new patch.

Comment 7

[Benoit Girard (:BenWa)]

No, I was saying it would be nice if classes like gfxContext didn't require a heap allocation but ATM they do. They are -not- safe to be used on the stack. Please land this patch and fix all the stack usage of this. It would be nice to land this with the DISALLOW_STACK_ALLOCATION something macro.

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: Remove dangerous public destructor of gfxContext.

This version seems to fare better: https://tbpl.mozilla.org/?tree=Try&rev=11f8404a2c6e.

Comment 9

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93cd05e483cd

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/93cd05e483cd