Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

window.opener can be cleared #4429

Open
annevk opened this issue Mar 12, 2019 · 5 comments
Open

window.opener can be cleared #4429

annevk opened this issue Mar 12, 2019 · 5 comments
Labels
needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan topic: browsing context

Comments

@annevk
Copy link
Member

annevk commented Mar 12, 2019

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 annevk added needs tests Moving the issue forward requires someone to write tests topic: browsing context labels Mar 12, 2019
annevk added a commit to web-platform-tests/wpt that referenced this issue Mar 13, 2019
@annevk
Copy link
Member Author

annevk commented Mar 13, 2019

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
Copy link
Contributor

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
Copy link
Member Author

annevk commented Mar 13, 2019

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
Copy link
Member Author

annevk commented Mar 22, 2019

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?

@annevk annevk added needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan and removed needs tests Moving the issue forward requires someone to write tests labels Mar 22, 2019
@domenic
Copy link
Member

domenic commented Mar 28, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan topic: browsing context
Development

No branches or pull requests

3 participants