1247759 - Remove support for addons leaking chrome windows

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

I was talking to billm today, and he pointed out that because we have compartment-per-addon now, we could potentially cut references from addon compartments to chrome windows, when we close the windows. This would likely break less stuff than cutting all references to chrome windows.

The drawback is that it might break addons and we don't really have any way to know how common this is until it lands in Nightly. It also isn't clear how many leaks this will actually fix. Bug 873163 is an example of an addon leaking chrome windows, which this would help, but unfortunately just cutting addon to chrome references is not sufficient to fix the leak in that case, because there's also some reference through editor stuff.

Comment 1

[Andrew McCreight [:mccr8]]

(The argument for why it is fair to allow these references for Firefox chrome code but not addons is that for chrome code we do have automated testing that should detect windows that are being held alive indefinitely, while we do not have any such testing for addons, so we can't trust addon code as much.)

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Cut references from addon chrome compartments to the compartment of chrome windows being closed. WIP

Here's a hacky mostly-untested patch to do this.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Andrew McCreight [:mccr8] from comment #0)
> The drawback is that it might break addons and we don't really have any way
> to know how common this is until it lands in Nightly. It also isn't clear
> how many leaks this will actually fix. Bug 873163 is an example of an addon
> leaking chrome windows, which this would help, but unfortunately just
> cutting addon to chrome references is not sufficient to fix the leak in that
> case, because there's also some reference through editor stuff.

All of these objections were true of the original wrapper cutting fix too, so I see no reason to not at least try it and see what breaks.

Comment 4

[Andrew McCreight [:mccr8]]

Addon architecture is very different now.