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

Responsible browsing context of an environment settings object seems fishy #1580

Closed
Ms2ger opened this issue Jul 20, 2016 · 7 comments · Fixed by #5440
Closed

Responsible browsing context of an environment settings object seems fishy #1580

Ms2ger opened this issue Jul 20, 2016 · 7 comments · Fixed by #5440
Assignees

Comments

@Ms2ger
Copy link
Member

Ms2ger commented Jul 20, 2016

Especially when it's used on worker threads. On worker threads, it seems like only concept-bc-noscript is used, but that still looks at the active document; that seems like a race condition.

@domenic
Copy link
Member

domenic commented Jul 20, 2016

Can you explain in more detail? What do you think browsers do here for checking if scripting is disabled in workers?

@Ms2ger
Copy link
Member Author

Ms2ger commented Jul 22, 2016

I have no idea. Do browsers even check that in cases like importScripts, which are invoked from script anyway? Dedicated workers are probably killed when their document becomes non-active anyway.

@annevk
Copy link
Member

annevk commented Jul 22, 2016

Also, scripting being disabled can't be scoped to browsing contexts given service workers.

@domenic domenic self-assigned this Sep 19, 2016
@annevk
Copy link
Member

annevk commented May 9, 2017

I was thinking we could tie "scripting is disabled" to an agent cluster, but I guess what really needs to happen here is that we need to figure out what browsers do and are willing to do? @foolip @smaug---- thoughts?

@smaug----
Copy link

Not sure I understand the issue here. Which "scripting is disabled" are we talking about? Some user setting or non-current-Window objects?

But neither of those is bound to tab groups in Gecko (and I think tab group is what this new agent cluster is. (that 'agent' is for some reason really hard for me to understand in this context. ))
When window object goes away, dedicated workers get killed, and shared too, if there aren't more windows keeping them alive.

Note, if a page enters bfcache, that does not kill workers but freezes them.

@annevk
Copy link
Member

annevk commented May 9, 2017

The user agent setting to disable scripts. An agent cluster is more like a "doc group", including any associated workers.

@foolip
Copy link
Member

foolip commented May 9, 2017

I don't think I understand the original issue, is https://html.spec.whatwg.org/#responsible-browsing-context especially fishy or is the access of any browsing context from a worker fishy?

As for disabling scripts, it looks like in Blink that's a per "execution context" thing, i.e. per-document or per-worker global scope. That calls into the rough equivalent of "browsing context" to check the user setting. I can only assume it's more complicated than meets the eye.

Is there a test case that could be written to show that something is amiss here, or is it "just" that the spec says funny things that doesn't obviously map to how one would implement it?

annevk added a commit that referenced this issue Feb 26, 2019
This fixes #3846, though does not clarify the situation for all callers necessarily.

Removing this concept altogether is tricky as it's used in workers for disabled scripts. #1580 tracks that.
domenic pushed a commit that referenced this issue Feb 26, 2019
This fixes #3846, though does not clarify the situation for all callers necessarily.

Removing this concept altogether is tricky as it's used in workers for disabled scripts. #1580 tracks that.
domenic added a commit that referenced this issue Apr 6, 2020
Previously it was per-browsing context. This caused a number of
problems when used from workers; it went through the "responsible
document" indirection which did not exist for service workers and was
racy in other cases.

In theory this might be cleaner if it were per agent, i.e. if agents
maintained internal consistency about disabling or enabling scripting.
In practice user agents seem to have a single user-agent-wide switch,
and at least some of them wire that to a per-global boolean, so we just
use per-global as it fits most cleanly into existing architecture and
matches at least some implementation internals.

Closes #1580. Helps with #5422.
domenic added a commit that referenced this issue Apr 7, 2020
Previously it was per-browsing context. This caused a number of
problems when used from workers; it went through the "responsible
document" indirection which did not exist for service workers and was
racy in other cases.

In theory this might be cleaner if it were per agent, i.e. if agents
maintained internal consistency about disabling or enabling scripting.
In practice user agents seem to have a single user-agent-wide switch,
and at least some of them wire that to a per-global boolean, so we just
use per-global as it fits most cleanly into existing architecture and
matches at least some implementation internals.

Closes #1580. Helps with #5422.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants