window.close not working with browser.tabs.remote=true - ASSERTION: TabChild::DestroyBrowserWindow not supported in TabChild

VERIFIED FIXED in Firefox 31

Status

()

Core
IPC
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: billm)

Tracking

({assertion, testcase})

Trunk
mozilla31
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox31 verified)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 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

Updated

5 years ago
Blocks: 516752
OS: Mac OS X → All
Hardware: x86 → All
(Reporter)

Comment 2

5 years ago
Yeah, probably.
Depends on: 919878

Comment 3

4 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.

Comment 4

4 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.
Depends on: 978610
Flags: needinfo?(bzbarsky)
Summary: window.close not working with browser.tabs.remote=true - ASSERTION: TabChild::SetWebBrowser not supported in TabChild → window.close not working with browser.tabs.remote=true - ASSERTION: TabChild::DestroyBrowserWindow not supported in TabChild
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.
Flags: needinfo?(bzbarsky)

Comment 6

4 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.
Assignee: nobody → bobowencode
Presumably the Firefox UI sees the DOMWindowClose event and does ... something.
Created attachment 8393754 [details] [diff] [review]
close-fix

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.
Assignee: bobowencode → wmccloskey
Status: NEW → ASSIGNED
Attachment #8393754 - Flags: review?(felipc)

Comment 9

4 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.
Attachment #8393754 - Flags: review?(felipc) → review+
https://hg.mozilla.org/mozilla-central/rev/266a7c5085ce
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Reproduced in Nightly 2013-10-14-mozilla-central-debug.
Verified fixed Nightly 2014-03-24-mozilla-central-debug, Win 7 x64.
Status: RESOLVED → VERIFIED
status-firefox31: --- → verified
Depends on: 1196159
You need to log in before you can comment on or make changes to this bug.