Closed Bug 735504 Opened 13 years ago Closed 13 years ago

Unmark ChromeWindow held by live nsGlobalWindow

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 736563

People

(Reporter: mccr8, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

The mInnerWindowHolder field of an nsGlobalWindow seems to contain an XPCWrappedNative that is doubly linked with a JS Object (ChromeWindow). If the nsGlobalWindow is necessarily alive, we should unmark gray the JS Object. I'm not 100% sure if the XPCWN necessarily keeps alive the JS Object, so I should figure that out. In a log I have here, there are two of these XPCWNs that each add almost 100 JS objects to the CC graph.
Blocks: 716598
Attached patch may not be okay (obsolete) (deleted) — Splinter Review
Patch doesn't seem to successfully unmark the JS Object (ChromeWindow).
So, I think the two ChromeWindows that aren't getting marked are associated with about:blank documents. I'm not sure where those come from.
These are about:blank windows are apparently those that are created when the window is opened, then you navigate away and they are put into the bf cache. Or something like that.
Are we really bfcaching about:blank? That seems silly.
Just open about:memory verbose and look for about:blank...
So the windows aren't chrome windows or what? chrome docshells don't have session history nor bfcache.
I think they are Chrome windows. I don't know why they are around, that was just jst's theory. I see things like this in my about:memory which I suspect are similar: │ ├───2,272,104 B (00.45%) -- top(chrome://browser/content/browser.xul, id=1) │ │ └──2,272,104 B (00.45%) -- active │ │ ├──2,262,632 B (00.45%) ++ window(chrome://browser/content/browser.xul) │ │ └──────9,472 B (00.00%) -- window(about:blank) │ │ └──9,472 B (00.00%) ── dom [4] For browser.xul, they are tiny like that. For console.xul, they are about 180k, because they have some kind of layout data in there.
This seems to work. jst pointed me at the memory reporter code for windows, and I just copied the logic there.
Attachment #605583 - Attachment is obsolete: true
This is pretty redundant with the existing MarkWindowList code, so I should probably fold those into this loop.
If we're just going to do this once per GC, which probably makes sense, then we might as well just do this by making the GC mark black instead of gray. We'll have some minor slowness from CC windows that open in between GCs, but it shouldn't matter much.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: