whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
7.87k stars 2.58k forks source link

window.opener can be cleared #4429

Open annevk opened 5 years ago

annevk commented 5 years ago

It turns out that browsers clear the opener browsing context once that's closed/discarded (see #4428; needs testing). I.e., if a child bc opens an auxiliary bc and the child bc gets closed/discarded, when does auxiliary bc's opener browsing context change to null? (Thanks Nika for pointing this out!)

cc @mystor @bzbarsky

annevk commented 5 years ago

It seems we can make it null at discard-time. The remaining question is how. Do we track all openees with a non-null opener browsing context so that when discarding happens we can clear all openees's opener browsing context? It seems like that should work?

bzbarsky commented 5 years ago

Conceptually, you could do that, or you could just check at opener-get time whether the opener browsing context is discarded, right? At least as long as we're talking about the script-visible opener. If spec algorithms examine opener directly in some way, things are more complicated.

annevk commented 5 years ago

There are indeed algorithms that examine opener directly. I suspect those are wrong (instead those need to operate on a browsing context group and its top-level browsing contexts). Perhaps we ought to fix #313 first to make sure of that though.

And currently discarded is not a state we expose on a browsing context as the assumption is that all references to it are cleared at the same time. I guess exposing the discarded state is probably a better strategy than all the additional bookkeeping my solution would take.

annevk commented 5 years ago

I forgot to add this here: I think the main problem with checking when getting is that then "opener browsing context" would have to hold a weak reference to a browsing context (since we prefer the specification to not have memory leaks these days) as well as a browsing context needing this additional state in case it's discarded, but not "freed".

Preferences anyone?

domenic commented 5 years ago

I'm not sure I'm fully following but I think your original proposal is pretty straightforward. It would still be a weak reference in that direction, I think, but the architecture just makes sense. It's easy to understand the intent of the spec that way.

But I'm also OK putting this off until work on #313 is done or similar, and then perhaps working on a script-visible-only solution.