630242 - WARNING: No outer window available!: file dom/base/nsGlobalWindow.cpp, line 8451

Status: RESOLVED FIXED

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

Description

[Honza Bambas (:mayhemer)]

Attachment: test case

I'm getting this warning repeatedly with the attached test case when using a debug build, win7.  It seems to come from the storage event invocation.

WARNING: No outer window available!: file D:/mozilla/mozilla-central/_obj-browser-debug/dom/base/../../../dom/base/nsGlobalWindow.cpp, line 8451
WARNING: Need a principal to compare this to!: file d:/mozilla/mozilla-central/_obj-browser-debug/caps/src/../../../caps/src/nsPrincipal.cpp, line 318


Callstack:

>	xul.dll!nsGlobalWindow::GetParentInternal()  Line 8466	
 	xul.dll!nsGlobalWindow::GetPrincipal()  Line 2813 + 0xb bytes	
 	xul.dll!nsGlobalWindow::Observe(nsISupports * aSubject=0x0a753b6c, const char * aTopic=0x0a753e28, const wchar_t * aData=0x00000000)  Line 8280 + 0x17 bytes	
 	xul.dll!nsGlobalWindowObserver::Observe(nsISupports * aSubject=0x0a753b6c, const char * aTopic=0x0a753e28, const wchar_t * aData=0x00000000)  Line 696	
 	xul.dll!nsObserverList::NotifyObservers(nsISupports * aSubject=0x0a753b6c, const char * aTopic=0x0a753e28, const wchar_t * someData=0x00000000)  Line 131	
 	xul.dll!nsObserverService::NotifyObservers(nsISupports * aSubject=0x0a753b6c, const char * aTopic=0x0a753e28, const wchar_t * someData=0x00000000)  Line 185	
 	xul.dll!NS_InvokeByIndex_P(nsISupports * that=0x007c73f8, unsigned int methodIndex=5, unsigned int paramCount=3, nsXPTCVariant * params=0x0a753d50)  Line 103	

The topic is "dom-storage2-changed"



Steps to reproduce:

- open the attachment
- let it finish the performance test
- refresh it with F5
- soon the console gets flooded with the warnings

Comment 1

[Honza Bambas (:mayhemer)]

I found out that a window open for about:blank is soon released by cycle collector but still receives notifications from the observer service.

After patch from bug 630193 lands, should we also call CleanUp() as part of unlinking the window by CC?  Or have a separate method to release the observer?

Comment 2

[Nathan Froyd [:froydnj]]

This seems to be happening quite a bit more frequently during tests due to ghost window checking.  We have an inner window that doesn't have an outer window, isn't modal content, and isn't chrome...what sort of window is it at this point?  Kyle?

Comment 3

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

I don't know what "ghost window checking" is, but if we have an inner window with no outer it's either awaiting GC/CC or it's being held alive by JS somewhere (and may be considered "leaked").

If "ghost window checking" refers to jlebar's concept of ghost windows, I think that's a waste of time, because almost all mochitests are loaded from the same domain, and a window can't be a ghost window if there are non-ghost-windows from the same origin, iirc.

Comment 4

[Justin Lebar (not reading bugmail)]

> This seems to be happening quite a bit more frequently during tests due to ghost window checking.

Can you give me a stack?

> If "ghost window checking" refers to jlebar's concept of ghost windows, I think that's a waste of 
> time, because almost all mochitests are loaded from the same domain, and a window can't be a ghost 
> window if there are non-ghost-windows from the same origin, iirc.

Sure, but we have to iterate over all windows in order to get their URIs.

We already have one branch in the code which attempts to avoid spewing warnings, so maybe we just need to add another.

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: stack during mochitest-chrome testing

(In reply to Justin Lebar [:jlebar] from comment #4)
> > This seems to be happening quite a bit more frequently during tests due to ghost window checking.
> 
> Can you give me a stack?

Attached.  I manually inlined FORWARD_TO_OUTER so I would have a convenient place to set a breakpoint.  You ought to be able to see numerous examples of this sort of stack by running:

TEST_PATH=content/base/testchrome/ make mochitest-chrome

in a suitable objdir.

Comment 6

[Justin Lebar (not reading bugmail)]

Okay, so this is just because we do window->scriptObjectPrincipal().

I can hack around this by checking GetOuterWindow (as I do elsewhere), but I wonder if we should just get rid of the warnings in FORWARD_TO_OUTER(), as they're clearly getting in the way, at least in this case.

bz, smaug, do you have an opinion?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Not offhand... jst and mrbkap might.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Looks like a useful warning to me. And the fact that CheckForGhostWindows may cause the
warning just indicates that we have odd situation where inner window is kept alive too long.

Comment 9

[Andrew McCreight [:mccr8]]

This warning also appears 1634 times in a Mochitest-4 log I have sitting around. (0.45% of the entire log's size)

Comment 10

[Andrew McCreight [:mccr8]]

There are about 13000 WARNING lines in total, so it is more than 10% of the total warnings during a M4 run, though slightly less than 10% of the total size of WARNING lines in the file. (1.6MB for all WARNING lines, 147KB for the window warning.)

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Well, I assume the stack isn't always the same.
Is there some common stack where we should explicitly check whether there is outer window.

Tough, I haven't personally found this particular warning very useful when debugging, so
I don't object removing the warning.

Comment 12

[Justin Lebar (not reading bugmail)]

Is it possible that the warning we're seeing in the mochitest logs is indicative of bugs in any code?  For something like GetPrincipal(), the warning is probably not indicating a bug, because when we get the warning, we return NULL for the principal, and if I didn't check that, we'd crash.  But maybe elsewhere...

Sounds like what we should do is work around the warning here and then see how many warnings go away.  If it's not the vast majority of them (I suspect it will be), then we can consider removing the warning altogether...

Comment 13

[Justin Lebar (not reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=e6c60fc053c3

Comment 14

[Justin Lebar (not reading bugmail)]

(In reply to Justin Lebar [:jlebar] from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=e6c60fc053c3

This seems to get rid of all of the "No outer window available!" warnings in mochitest-4.  So I guess we can just take this as-is.

Comment 15

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 16

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d58233470ee5

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d58233470ee5