1516343 - BrowsingContext::Close should only set mClosed after sending the DOMWindowClose event

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, task, P3)

Comment 1

[u608768]

Attachment: Bug 1516343 - Set mClosed for the associated BrowsingContext in nsGlobalWindowOuter::FinalClose, r=nika

Since unload events aren't currently fired for OOP frames, we need to flip
mClosed prior to dispatching DOMWindowClose. We should eventually only call
BC::SetClosed in nsGlobalWindowOuter::FinalClose (after firing DOMWindowClose).

BrowsingContext currently uses mClosed in various locations under the
assumption that it is never actually flipped, so this should not land before
bug 1561899.

Comment 2

[u608768]

This work has exposed some odd window.close behaviour (particularly with how unload events are fired for cross process iframes), so we plan to hold off work on this until we clear things up.

Comment 3

[Neha Kochar [:neha]]

Kris might have already done this. Kris, please confirm.

Comment 4

[Chris Peterson [:cpeterson]]

Does this bug need to block Fission dogfooding (M5)?

Kris might have already done this. Kris, please confirm.

Even if the underlying code bug has been fixed, we need to remove peterv's FIXME comment mentioning this bug #:

https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/docshell/base/BrowsingContext.cpp#965-968

Comment 5

[Chris Peterson [:cpeterson]]

Assigning to kmag to remove FIXME comment linked from comment 4. Tracking for Fission Nightly (M6) because removing the comment doesn't block dogfooding.

Nika wonders whether we will need to set mClosed synchronously without syncing for webcompat. We want to avoid this because this behavior is janky (but matches Chrome's implementation).

Comment 6

[u608768]

(In reply to Chris Peterson [:cpeterson] from comment #5)

Nika wonders whether we will need to set mClosed synchronously without syncing for webcompat. We want to avoid this because this behavior is janky (but matches Chrome's implementation).

We'll probably need this for bug 1620714.

Comment 7

[Chris Peterson [:cpeterson]]

Reassigning to Kashav because he is working on this.

M6a

Comment 8

[u608768]

We ended up needing to set BrowsingContext::Closed before actually sending DOMWindowClose for bug 1620714.