307777 - Accessing properties on an already-closed window asserts

Status: RESOLVED WORKSFORME

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

Description

[Boris Zbarsky [:bzbarsky]]

Testcase coming up, assertion is:

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components'
property!

This is pretty much always bad. It usually means that native code is
making a callback to an interface implemented in JavaScript, but the
document where the JS object was created has already been cleared and the
global properties of that document's window are *gone*. Generally this
indicates a problem that should be addressed in the design and use of the
callback code.
: 'Error', file
../../../../../mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 564

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

jst and I talked about this some. The problem is that when the window is going
away (and we call SetDocShell(nsnull)) we ClearScope on the inner window. We
probably asserted even before the split window landing. The fix for this might
be to not clear scope (which jst says we shouldn't do anyway) and instead come
up with a better way of breaking the window -> event listener -> window leak.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I think what should happen is that event listeners should hold a weak ref to the
window and the window should hold a strong ref to the listeners.  When the
window goes away it should notify the listeners so they drop their weak refs...

Comment 4

[Asa Dotzler [:asa]]

it's not clear to me why we should block the release on this. please renominate
with an explanation if this is really a necessity for the release.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So do listeners still hold a strong ref to the window now that dbaron's patches for addEventListener have landed?

If they hold weak refs now, should we try not clearing the scope and seeing how things look?

I do think we should try to fix this for 1.9...

Comment 6

[Ryan VanderMeulen [:RyanVM]]

This assertion is gone.