Modify Storage Access to use a "per-frame" model #141
Conversation
This is an attempt to address privacycg#122.
| 1. Return the [=agent cluster/storage access map=] of |doc|'s [=relevant agent=]'s [=agent cluster=]. | ||
| Modify the definition of [=source snapshot params=] in the following manner: | ||
| 1. Add a new member called <dfn for="source snapshot params">has storage access flag</dfn> of type [=boolean=]. | ||
| 1. Add a new member called <dfn for="source snapshot params">environment id</dfn> of type opaque [=string=]. |
There was a problem hiding this comment.
This should just be string or you should define what opaque means for the type.
There was a problem hiding this comment.
I included "opaque" since this member will contain values from environment/id, which is defined as an opaque string.
|
(Edited the checklist to indicate multi-browser support + the Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1401089) |
|
I think I would have a much easier time reviewing this once #138 lands and this is rebased given the overlap. Given that tests are not ready yet either I think it's okay that this potentially has to wait until early January. I will try to reply to all the questions above though. |
This is building on top of w3c/permissions#390 to integrate SAA with permissions. It's deleting a lot of old manual state management but doesn't get rid of the (global) storage access map altogether, since that is done in #141. Co-authored-by: Anne van Kesteren <[email protected]>
There was a problem hiding this comment.
@johannhof and I went over this. It generally looks good, but there's a few loose ends to tie up still.
|
WPTs coming up in https://crrev.com/c/4117243 |
But that means this task would en up running after the rejection has already happened, right? So in the rejection task the activation wouldn't have been consumed. I think this looks good now, but maybe @bvandersloot-mozilla can do a final read as there's been quite a bit of back-and-forth. |
There was a problem hiding this comment.
I dug through the resolved conversations and re-read the diff.
This generally matches the consensus we have on what per-frame means and accomplishes it in simple ways that are consistent with conversations. I'm not super familiar with the new "navigator" stuff still, but reading that spec makes the environment id and cross origin navigation protections look good.
Just one inline comment.
This is an attempt to migrate to a "per-frame" model, as discussed in #122. This is built on top of #138 as a starting point.
The approach is to define a flag that lives on environment, and is set by
document.requestStorageAccessand read bydocument.hasStorageAccess. In order to propagate storage access during self-initiated, same-origin navigations, we also add a flag to thesource snapshot paramsused duringcreate navigation params by fetching, and conditionally copy thesourceDocument's relevant settings object's flag over to the new environment that will be created. This should let us achieve the benefits of the BrowsingContext approach discussed in #122, without having to add and clear state in BrowsingContext.Leaving the checklist for now, though I'd like to get some feedback before I try to write WPTs:
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff