Martijn Wargers (dead) Reporter
	Description • 11 years ago

See url testcase, a window should quickly open and then close again.
When I have the preference browser.tabs.remote=true, then the popup window doesn't close by itself.
Instead, I'm seeing this assertion in the console:
 0:52.72 [Child 8552] ###!!! ASSERTION: TabChild::SetWebBrowser not supported in TabChild: 'Not Reached', file /Users/mwargers/mozilla-central/dom/ipc/TabChild.cpp, line 823

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 1 • 11 years ago

Related to bug 919878?

	Martijn Wargers (dead) Reporter
	Comment 2 • 11 years ago

Yeah, probably.

	Bob Owen (:bobowen)
	Comment 3 • 10 years ago

I've just hit a similar problem in tests that open and close new tabs.

I also noticed that it is actually TabChild::DestroyBrowserWindow that is being called, the error message is wrong.
I've raised bug 982555, with a patch.

	Bob Owen (:bobowen)
	Comment 4 • 10 years ago

Looks like billm has got a fix for the actual closing of the tab in bug 978610.
It certainly seems to work for my test and for the URL test case.

You still get the ASSERTION though.

TabChild::DestroyBrowserWindow is being called by nsDocShellTreeOwner::Destroy, which is in turn called by nsGlobalWindow::ReallyCloseWindow.

bz - I assume that this is your comment about removing this in ReallyCloseWindow:
http://dxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#8294-8326

Do you think we can safely get rid of this now?

All nsDocShellTreeOwner::Destroy does is conditionally call DestroyBrowserWindow and all of the implementations of DestroyBrowserWindow seem to error anyway (apart from the one in embedding/tests/winEmbed/WebBrowserChrome.cpp).
I'm not sure if it might be used by other people embedding the code.

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 10 years ago

The comment about mHavePendingClose was just about the need for the IsTabContentWindow() checks and the like, I think.

> All nsDocShellTreeOwner::Destroy does is conditionally call DestroyBrowserWindow

nsDocShellTreeOwner is used by embedding and e10s.  Desktop firefox in-process bits use nsContentTreeOWner, and nsContentTreeOwner::Destroy does useful things like close the window.

	Bob Owen (:bobowen)
	Comment 6 • 10 years ago

(In reply to Boris Zbarsky [:bz] from comment #5)
> The comment about mHavePendingClose was just about the need for the
> IsTabContentWindow() checks and the like, I think.

Ah, OK.
 
> > All nsDocShellTreeOwner::Destroy does is conditionally call DestroyBrowserWindow
> 
> nsDocShellTreeOwner is used by embedding and e10s.  Desktop firefox
> in-process bits use nsContentTreeOWner, and nsContentTreeOwner::Destroy does
> useful things like close the window.

Thanks bz.

In my initial debugging (without e10s) nsContentTreeOwner::Destroy didn't seem to be getting called.
This was because nsGlobalWindow::ReallyCloseWindow wasn't getting called as |DispatchCustomEvent("DOMWindowClose")| in sGlobalWindow::Close was returning false and so it wasn't calling FinalClose().

The tab was still closing though, so I need to do some more investigation.

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 10 years ago

Presumably the Firefox UI sees the DOMWindowClose event and does ... something.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 8 • 10 years ago

I guess this was actually a bug in bug 978610. When the child calls window.close(), we always send a message to the parent asking it to close the tab or the window. However, that happens asynchronously. In the meantime, we just want to act like we're not planning to close anything. In particular, we shouldn't try to destroy the TabChild, since we don't have code to handle that.

	Bob Owen (:bobowen)
	Comment 9 • 10 years ago

(In reply to Bill McCloskey (:billm) from comment #8)
> Created attachment 8393754 [details] [diff] [review]
> close-fix

Thanks ... that's saved me a lot of head scratching. :)

This definitely gets rid of the assertion I was getting, by the way.

	Bill McCloskey [inactive unless it's an emergency] (:billm) Assignee
	Comment 10 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/266a7c5085ce

	Carsten Book [:Tomcat]
	Comment 11 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/266a7c5085ce

	Paul Silaghi, QA [:pauly]
	Comment 12 • 10 years ago

Reproduced in Nightly 2013-10-14-mozilla-central-debug.
Verified fixed Nightly 2014-03-24-mozilla-central-debug, Win 7 x64.