1558176 - Make BrowsingContext be responsible for discard

Status: RESOLVED DUPLICATE of bug 1582832

(Core :: DOM: Navigation, enhancement, P3)

Description

[Andreas Farre [:farre]]

Currently the discard operation on browsing contexts roughly happen in BrowsingContext::Detach, but the driver behind it is nsDocShell::Destroy, it should be the other way around. Discarding a browsing context should be an operation directly performed on a browsing context in the places where browsing contexts are discarded.

Especially the discrepancy between nsDocShell::Destroy and nsDocShell::DestroyChildren would be good to clean up by consolidating discard behaviour in BrowsingContext. In particular the fact that nsDocShell::DestroyChildren doesn't actually call nsDocShell::Destroy on its children, but only removes them from its list of children.

Comment 1

[Adrian Walker]

For convenience here's a quick test to see if the originating bug id=1554715 has been fixed by any changes you make to 1555287 or 1558176.

1. https://www.executable-english.com/ibl_login.html
2. Click the GO button in the middle of the page
3. click the Go button top right.

In FF 67 you will see a display with the top frame missing. In FF 66, Opera, Chrome or Edge, you will see the correct display that shows the top and bottom frames.

Hope this helps -- Adrian

Comment 2

[Andreas Farre [:farre]]

(In reply to Adrian Walker from comment #1)

Hope this helps -- Adrian

Sure does, thanks! I'll see if I can make a wpt test case out of this.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

I'll take a look.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Just to clarify, we're talking about https://searchfox.org/mozilla-central/rev/cfd1cc461f1efe0d66c2fdc17c024a203d5a2fd8/docshell/base/nsDocShell.cpp#4584-4588 and moving the calls to mBrowsingContext->Detach() to... where exactly? To wherever we call nsDocShell:Destroy()?

Comment 5

[David Teller [:Yoric] - still alive but not very active]

As per our conversation with nika, this seems to have already been mostly done. Dropping bug.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Since bug 1582832, nsDocShell::Destroy is no longer the responsible party for performing teardown of a BrowsingContext.