759919 - XUL prototype cache causes compartment for extension's XUL window to live forever

Status: RESOLVED WORKSFORME

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

Description

[Girish Sharma [:Optimizer]]

Attachment: xpi to test

1) Install the addon from the xpi provided.
2) Play with it a little, disable it, uninstall it but don't open the Options window by any means, and the about:memory memory listing is perfectly fine and the addon clears up all the memory used by the bootstrap.js
3) But if you open the options window while the addon is enabled, you get a compartment in about:memory under JS.

│  ├───────55,944 B (00.03%) -- compartment([System Principal], chrome://uienhancer/content/options.xul)
│  │       ├──40,960 B (00.02%) -- gc-heap
│  │       │  ├──29,280 B (00.01%) ── arena/unused
│  │       │  └──11,680 B (00.01%) ── sundries
│  │       ├──10,472 B (00.01%) ── script-data
│  │       └───4,512 B (00.00%) ── other-sundries

This compartment is always like this, and remains even after disabling the add-on. It only gets removed on uninstalling the add-on.

The problem is due to the options.js file that is used by the options.xul . I have tested that If I do not use the options.js file, then the compartment is never formed.

Please have a look at the options.js file and options.xul file from the github repo link : https://github.com/scrapmac/UIEnhancer/tree/master/content

Comment 1

[Justin Lebar (not reading bugmail)]

Since you know the problem is in optimizer.js, why don't you try taking out pieces of that file until you've narrowed things down further?

Comment 2

[Girish Sharma [:Optimizer]]

optimizer.js ?
Hmm, I am doing that as we speak, this bug is for the fact that it might not be my add-ons fault after all :D

Comment 3

[Girish Sharma [:Optimizer]]

Keeping the Options.xul the same, I stripped down the Options.js to this blank file:


let optionsWindow = {
  openFile: function OW_openFile() {},
  onLoad: function OW_onLoad() {},
  saveChanges: function OW_saveChanges() {},
  handleShortcutChange: function OW_handleShortcutChange(event) {},
  resetStyleSheet: function OW_resetStyleSheet() {},
  toggleStylesSettings: function OW_toggleStylesSettings(check) {},
  toggleURLBarSettings: function OW_toggleURLBarSettings(check) {},
  toggleStatusSettings: function OW_toggleStatusSettings(check, leftoverSpaceIndicator) {},
  toggleProgressSettings: function OW_toggleProgressSettings(arrowIndicator) {},
  changeColor: function OW_changeColor(index) {},
  onColorChange: function OW_onColorChange() {},
  toggleBookmarksSettings: function OW_toggleBookmarksSettings(check) {},
  notifyChange: function OW_notifyChange(val) {},
  onUnload: function OW_onUnload() { optionsWindow = null; },
};

I am keeping the function declaration so that there is no error as these functions are assigned to elements of Options.xul via oncommand, or onkeyup attributes.

Still I am getting the compartment, its size is less, but it is still there.

│  ├───────37,104 B (00.01%) -- compartment([System Principal], chrome://uienhancer/content/options.xul)
│  │       ├──32,768 B (00.01%) -- gc-heap
│  │       │  ├──27,128 B (00.01%) ── arena/unused
│  │       │  └───5,640 B (00.00%) ── sundries
│  │       └───4,336 B (00.00%) ── other-sundries

I am beginning to think that the reason might not be leaking.

Comment 4

[Girish Sharma [:Optimizer]]

Attachment: blank options.js

Comment 5

[Girish Sharma [:Optimizer]]

Attachment: options.xul

Comment 6

[Girish Sharma [:Optimizer]]

So as to narrow things down, I stripped down both options.js and options.xul to be almost blank.
Here is the options.xul

<prefwindow id="UIEnhancerOptionsWindow" 
  minwidth="600" minheight="375" maxheight="500" buttons="accept,cancel"
  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
  <script type="application/javascript" src="options.js"/>
 
</prefwindow>

and my options.js is totally blank.

now if I open the options window, through any method (be it hotkey, toolbar button, addon manager), a compartment is still remains even after closing the options window, or disabling the addon :

│   ├──────27,080 B (00.02%) -- compartment([System Principal], chrome://uienhancer/content/options.xul)
│   │      ├──24,576 B (00.02%) -- gc-heap
│   │      │  ├──23,864 B (00.01%) ── arena/unused
│   │      │  └─────712 B (00.00%) ── sundries
│   │      └───2,504 B (00.00%) ── other-sundries

Now I am totally sure that this is not due to my add-on, its a fault in Firefox.
As soon as I remove the <script type="application/javascript" src="options.js"/> line from options.xul, the compartment is not formed.

The point is, the options.js file is totally empty, so importing it in an xul should not create any left out compartment.

Comment 7

[Justin Lebar (not reading bugmail)]

> As soon as I remove the <script type="application/javascript" src="options.js"/> line from 
> options.xul, the compartment is not formed.

Well, yes.  "compartment" is short for "javascript compartment."  If you have no JS, it's not unsurprising that you don't get a compartment.

But the bug isn't about *getting* a compartment.  It's about the fact that the compartment never goes away, even after you close the options window.

Comment 8

[Girish Sharma [:Optimizer]]

My point is, the compartment was still made, even when the options.js file was totally empty and options.xul the minimum. With that configuration, there is no code to blame the leaking to.

Comment 9

[Girish Sharma [:Optimizer]]

One clarification, whenever I meant the compartment was made, I actually was referring to the point after the closing of options window, that the compartment still remains after closing of options window.

Comment 10

[Justin Lebar (not reading bugmail)]

Do you see the zombie compartment when you open about:compartments?

Comment 11

[Girish Sharma [:Optimizer]]

I see an entry under System Compartments as
[System Principal], chrome://uienhancer/content/options.xul

IMO, this leaking cannot be called zombie compartment as it goes away on add-on uninstall.

Comment 12

[Justin Lebar (not reading bugmail)]

> IMO, this leaking cannot be called zombie compartment as it goes away on add-on uninstall.

Well, that's not how we've defined "zombie compartment".  :)

I think you're right, this probably is a bug in Firefox.  Looking through your code, I don't see anybody taking a reference to the options window after you open it.

It would be helpful, in terms of getting this bug fixed in a timely manner, if you could produce a new add-on which is a minimal testcase for this bug.

If this bug is keeping your add-on from being approved for AMO, please point the reviewer here.  I think this add-on is fine to approve as-is.

Comment 13

[Girish Sharma [:Optimizer]]

I will shortly attach a minimal add-on which will reproduce this.

I have made a comment and pointed to this page already.

Comment 14

[Girish Sharma [:Optimizer]]

Attachment: Test case Add-on

1) Install the add-on, it has nothing and will affect nothing.
2) Go to add-on manager, open the options window. The options window is also empty and the javascript attached to the options window is also empty.
3) Close the options window, now open about:memory and see that their is still a compartment for testaddon's options.xul under JS.
4) Try minimizing the memory, GC, or CC, but the compartment will still be there, it will not remove even if you disable the add-on.
5) Uninstall the add-on and the compartment is finally gone.

Comment 15

[Nils Maier [:nmaier]]

Not sure if this is a bug or a feature:
It seems the startup cache or any associated component is holding refs to the compartment.
When you invalidate it, like XPIProvider.jsm will do when uninstalling an add-on, but not when just disabling it, then those compartments will go away.

STR:
- Install addon from comment #14
- Open the addon preference window
- Close the addon preference window
- Check about:compartmets
-> Still a chrome://testaddon/content/options.xul compartment around

- Without disabling or uninstalling the add-on, issue the following in a privileged context (e.g. about:compartments WebConsole):
Components.utils.import("resource://gre/modules/Services.jsm", {}).Services.obs.notifyObservers(null, "startupcache-invalidate", null);
- Refresh about:compartments
-> chrome://testaddon/content/options.xul compartment is gone now

Who is knowledgeable enough about the startup cache and can shed some light on this?

Comment 16

[Girish Sharma [:Optimizer]]

Thank God I Cc'ed you Nils, now its confirmed that its a Firefox's behavior and not due to my add-on, Can you change the title to a more suitable one ?

Comment 17

[Justin Lebar (not reading bugmail)]

> Who is knowledgeable enough about the startup cache and can shed some light on this?

Maybe mwu?

Comment 18

[Michael Wu [:mwu]]

Can you try setting the pref nglayout.debug.disable_xul_cache to true? If that "fixes" it, this is a problem in the xul prototype cache.

Comment 19

[Nicholas Nethercote [inactive]]

Girish, have you had a chance to try the suggestion in comment 18?

Comment 20

[Girish Sharma [:Optimizer]]

Sorry for the late reply. Yes, following comment 18 fixes the problem and the options.xul is not present in the about:memory page after the close of options window.

Comment 21

[Nils Maier [:nmaier]]

I think the Add-on Manager, more specifically the XPIProvider, is to blame here because it does not attempt to clear the cache(s).
Having the XPIProvider push a "chrome-flush-caches" notification when an add-on is disabled/uninstalled should do the trick. This would immediately kill the xul/xbl cache, image cache, string bundle cache, style sheet cache and jar cache at once (in all of which the add-on may have still parts floating around).
However, this approach isn't very fine grained and will kill the cache for all non-addon stuff as well.

Blair, what do you think about that approach?

Comment 22

[Kris Maglione [:kmag]]

Extensions don't have XUL windows anymore.