-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add the Origin-Isolation
header
#5545
Conversation
Hmm, I think there is a big difference between just changing IsPlatformObjectSameOrigin, and storing a boolean on the document. If we only change IsPlatformObjectSameOrigin, then other same origin-domain checks will still pass. So I think we need to store a boolean, and also write tests for other such checks. |
I like the idea though that changing the keying of the agent cluster is what disables document.domain. Unfortunately there's no clear way back from an agent cluster to its key. Setting a flag on the agent cluster similar to cross-origin isolated is something you could do though. That also avoids having to maintain a per-document boolean. |
Did you see my latest commit? I think with the historical agent cluster key map, there kind of effectively is such a way back now. |
source
Outdated
data-x="concept-response-header-list">header list</span>.</p></li> | ||
|
||
<li><p>Let <var>requestsOI</var> be true if <var>oiHeader</var> is not failure, | ||
<var>oiHeader</var> is not null, and <var>oiHeader</var>[0] is the <span>structured header |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is oiHeader indexable as such?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All structured header items are a pair of the actual value plus the parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess Fetch should define how they translate to Infra then if we want to do this. Might also be nice for parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the concepts used are already Infra concepts: pairs, maps, booleans, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, if only they would have referenced it as well. But okay, that should make any formal equating pretty straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed you still used "structured header boolean true". If they are the same we don't have to do that, right?
6b88ccf
to
93ef2db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this looks really good, but I think we should stick more to COOP+COEP design in a few places.
source
Outdated
|
||
<li><p>Let <var>requestsOI</var> be true if <var>oiHeader</var> is not failure, | ||
<var>oiHeader</var> is not null, and <var>oiHeader</var>[0] is the <span>structured header | ||
boolean</span> true.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's too early, but I can't help but think we want an abstraction to make this less awkward and encourage consistency in handling. No great ideas other than collapsing failure into null though. (Or maybe that is what we should always do?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do suspect collapsing failure into null might be what all callers want. Perhaps let's reassess after this lands, COOP lands, COEP lands, and CORP gets converted to a structured header. (And maybe by then Permission Policy / Document Policy might be ready too.)
source
Outdated
|
||
<p class="XXX"><a href="https://github.com/whatwg/html/pull/4734">Issue #4734</a> is expected to | ||
further modify this algorithm to increase the number of cases where the returned value is an | ||
<span>origin</span>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the approach in that PR by modifying "obtain a similar-origin window agent" and inlining "obtain an agent cluster key".
I also wonder, if we did that, could we modify browsing context group's historical agent cluster key map before invoking that algorithm so it could use that map to make decisions, requiring us to pass less state around?
Furthermore, now that we tie state to browsing context groups, it almost seems like we should have different types of them. Those that carry a historical map and those that are cross-origin isolated. But some asserts is probably enough initially.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/whatwg/html/pull/4734/files#r405428958 indicates we want to fold all three algorithms together. I'll give that a shot...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This turned out pretty nicely.
I didn't address your latest paragraph so I'll leave this un-resolved. If you have concrete suggestions (either for now or as a followup) then let me know; otherwise I wasn't sure where to go with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was mostly wondering if we should make normal browsing context groups and cross-origin isolated browsing context groups more clearly mutually exclusive. We can address it in the context of cross-origin isolated.
It's unfortunate PR Preview no longer seems to work here. I added a space to try to trigger it, but it's not working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the map issue I realized that another thing we need to agree on is how cross-origin isolated and origin-isolation restricted play together.
My initial take was that cross-origin isolated implies the latter, but this would do the wrong thing for things that are already keyed on origin. Perhaps we should revisit that as allowing those to be origin-isolation restricted despite it not meaning anything would simplify several things at this point?
source
Outdated
|
||
<p class="XXX"><a href="https://github.com/whatwg/html/pull/4734">Issue #4734</a> is expected to | ||
further modify this algorithm to increase the number of cases where the returned value is an | ||
<span>origin</span>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was mostly wondering if we should make normal browsing context groups and cross-origin isolated browsing context groups more clearly mutually exclusive. We can address it in the context of cross-origin isolated.
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. - Other failures are due to the lack of origin vs site-keying, which is now more directly observable. See bug 1067389. Bug: 1042415, 1067389 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34
The way I was envisioning it was that cross-origin isolated just causes requestsOI to be true (regardless of the WDYT? |
That could work I think. We'd change
to also check for bcg's cross-origin isolated and where we write into the map (TBD) we add another conditional to only do so if bcg's cross-origin isolated is false. |
I don't think this would be strictly necessary, because cross-origin isolation ensures consistency within a BCG itself. So the map would just be redundant for any BCG with cross-origin isolated pages; writing to it would be fine (and have no effect). I guess maybe that gets to your point above about making normal BCGs and COI BCGs mutually exclusive. |
If we did that I think we'd have to add a note, but maybe we should do that either way. (Agreed that there is no observable difference.) |
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank is not properly returning true, so about-blank is failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. - Other failures are due to the lack of origin vs site-keying, which is now more directly observable. See bug 1067389. Bug: 1042415, 1067389 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34
566ce66
to
3ac64ea
Compare
3ac64ea
to
3f475d1
Compare
This now has a secure context check. It, like COOP, needed to check the response's HTTPS state, so maybe we should have an abstraction? There's no great one though, since reservedEnvironment and response are separate anyway. The best we could do is something like
although I think there should be better names. Anyway, this is now blocked on WICG/origin-agent-cluster#31, I guess. |
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672}
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672}
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672}
…nIsolationRestricted, a=testonly Automatic update from web-platform-tests Origin isolation: implement window.originIsolationRestricted See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672} -- wpt-commits: 9e23aa452a1261c637ce32cbb8bee450e7006f8d wpt-pr: 24194
…nIsolationRestricted, a=testonly Automatic update from web-platform-tests Origin isolation: implement window.originIsolationRestricted See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672} -- wpt-commits: 9e23aa452a1261c637ce32cbb8bee450e7006f8d wpt-pr: 24194
Closes #31. Closes #32. Note that the specification pull request at whatwg/html#5545 is already updated for these semantics.
Closes #31. Closes #32. Note that the specification pull request at whatwg/html#5545 is already updated for these semantics.
Closes #31. Closes #32. Note that the specification pull request at whatwg/html#5545 is already updated for these semantics.
5202ca4
to
c41ac69
Compare
source
Outdated
</dd> | ||
</dl> | ||
|
||
<p>A <code>Document</code> delivered over a <span>secure context</span> can opt in to origin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
into? We only have two instances of "in to" and both look suspect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for cases like "zoom in" or "opt in" it's best to keep those phrases separate from the "to". There's nothing very concrete on this online; e.g. https://english.stackexchange.com/questions/422848/opt-into-vs-opt-in-to is unanswered and https://forum.wordreference.com/threads/opt-in-or-opt-into.3560327/ is wishy washy. The headline at https://www.wsj.com/articles/more-parents-opt-into-opt-out-1429893574 separates them at least.
source
Outdated
<li><p>Set <var>group</var>'s <span>historical agent cluster key map</span>[<var>origin</var>] | ||
to <var>key</var>.</p></li> | ||
</ol> | ||
</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we change this whole addition to start with "Otherwise, if" we could keep the historical agent cluster key map and cross-origin isolated separated. I would somewhat prefer that as the less (unneeded) bookkeeping the better.
(It would also allow more easily for introducing different group types in the future.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'll give it a try.
source
Outdated
<var>group</var>'s <span>historical agent cluster key map</span>[<var>origin</var>].</p></li> | ||
|
||
<li> | ||
<p>Otherwise, if <var>requestsOI</var> is true, then:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the change is wrong. I initially read the algorithm like this, but that ends up making the map redundant as it would only ever contain origin values. The requestsOI conditional has to be part of overwriting the key only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I misunderstood what you meant by keeping them separate; PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, but I think what would also work (and is preferable) is keeping this as
Otherwise:
- If requestsOI is true, set ...
- Modify map.
My "Otherwise, if" comment was about the step above this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks, that makes sense now. Done.
This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868
This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#788297}
This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#788297}
This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#788297}
…, a=testonly Automatic update from web-platform-tests Origin isolation: initial getter updates This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#788297} -- wpt-commits: d02251699d90d93f792461a2b61baf48a0e786ec wpt-pr: 24593
…, a=testonly Automatic update from web-platform-tests Origin isolation: initial getter updates This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/master@{#788297} -- wpt-commits: d02251699d90d93f792461a2b61baf48a0e786ec wpt-pr: 24593
…, a=testonly Automatic update from web-platform-tests Origin isolation: initial getter updates This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <domenicchromium.org> Reviewed-by: James MacLean <wjmacleanchromium.org> Reviewed-by: Daniel Cheng <dchengchromium.org> Cr-Commit-Position: refs/heads/master{#788297} -- wpt-commits: d02251699d90d93f792461a2b61baf48a0e786ec wpt-pr: 24593 UltraBlame original commit: 51e11a32fa228d0aa74fe89621f5b204cd63f322
…, a=testonly Automatic update from web-platform-tests Origin isolation: initial getter updates This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <domenicchromium.org> Reviewed-by: James MacLean <wjmacleanchromium.org> Reviewed-by: Daniel Cheng <dchengchromium.org> Cr-Commit-Position: refs/heads/master{#788297} -- wpt-commits: d02251699d90d93f792461a2b61baf48a0e786ec wpt-pr: 24593 UltraBlame original commit: 51e11a32fa228d0aa74fe89621f5b204cd63f322
…, a=testonly Automatic update from web-platform-tests Origin isolation: initial getter updates This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <domenicchromium.org> Reviewed-by: James MacLean <wjmacleanchromium.org> Reviewed-by: Daniel Cheng <dchengchromium.org> Cr-Commit-Position: refs/heads/master{#788297} -- wpt-commits: d02251699d90d93f792461a2b61baf48a0e786ec wpt-pr: 24593 UltraBlame original commit: 51e11a32fa228d0aa74fe89621f5b204cd63f322
1741355
to
960dda9
Compare
960dda9
to
85d7635
Compare
Alright, I've updated all the tests to use the standard @annevk, want to do the final review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great apart from these minor nits around initializing requestsOI. Nice work!
See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#782672} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 4778c3539efd8603b157ae5d2d4555122ea25f7c
This makes window.originIsolationRestricted available in non-secure contexts, per the latest updates in whatwg/html#5545. It also updates the tests to expect the values we would get from that spec. It does *not* update the web platform tests, or the implementation, for the new name; that's https://crbug.com/1103866. Bug: 1095653 Change-Id: I9ac31d36e2744a3a49eb61c6899debbe38180868 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290782 Commit-Queue: Domenic Denicola <[email protected]> Reviewed-by: James MacLean <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#788297} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 35d531cb87017e4af4cba43b7f2190528ba1651f
…nIsolationRestricted, a=testonly Automatic update from web-platform-tests Origin isolation: implement window.originIsolationRestricted See WICG/origin-agent-cluster#24 and WICG/origin-agent-cluster#30 for background, and whatwg/html#5545 for the specification. Failing test expectations include: - We implement (3) from WICG/origin-agent-cluster#24 instead of (2) for now, so we fail getter-sandboxed-iframe. Tracking at https://crbug.com/1095653. - The initial about:blank, as well as removed iframes, are not properly returning true, so about-blank and removing-iframes are failing. Also tracking at https://crbug.com/1095653. - data: URLs are not [SecureContext] in Chromium (https://crbug.com/1095656) so getter-data-url fails. Note that per ongoing discussion in WICG/origin-agent-cluster#31 the naming of this API, as well as its edge-case behavior (e.g. for sandboxed iframes) will likely change. Bug: 1042415 Change-Id: I20c2d3e3fec7a5c0f1d12c386999c32fe27b6a34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243994 Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: James MacLean <[email protected]> Commit-Queue: Domenic Denicola <[email protected]> Cr-Commit-Position: refs/heads/master@{#782672} -- wpt-commits: 9e23aa452a1261c637ce32cbb8bee450e7006f8d wpt-pr: 24194
See https://github.com/WICG/origin-isolation.
(See WHATWG Working Mode: Changes for more details.)
(Obsolete) Things that I needed help on but are now fixed:
This has a similar problem as Add cross-origin opener policy #5334 in that it wants to do a secure context check before the environment settings object/Window/Document are created. It seems like we'll need to refactor the secure contexts spec a good bit, for both of them... But I'd like someone to confirm that line of thinking before I go off and do so. /cc @mikewest
I'm not sure if changing the agent cluster key automatically makes
document.domain
a no-op, or not. I think by my current reading, IsPlatformObjectSameOrigin does not care about agent clusters, so this PR as-written does not prevent cross-agent-cluster sync access. Should we add an agent cluster check to IsPlatformObjectSameOrigin, and let the domain component change via thedocument.domain
setter? Or should we store an "is origin-isolated" boolean on the Document, and bail out of thedocument.domain
setter algorithm if that boolean is true? The former seems a bit nicer, but I'm unsure if there's any observable difference between the two. /cc @dtapuska/browsers.html ( diff )
/browsing-the-web.html ( diff )
/iana.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )
/origin.html ( diff )
/webappapis.html ( diff )
/window-object.html ( diff )