Clarify storage infrastructure #86

annevk · 2020-04-30T15:21:23Z

At least two implementers are interested (and none opposed):
- Chrome
- Firefox
Tests are written and can be reviewed and commented upon at:
- N/A (or maybe for sessionStorage implications?)
Implementation bugs are filed:
- N/A

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

TODO Closes #18.

annevk · 2020-05-02T13:11:25Z

I think this now defines sufficient infrastructure to define storage and session storage APIs. It does not:

Define a replacement operation that is needed for Add API to allow origin to purge all storage #4 and Clear-Site-Data.
Change the keying away from origin (though it does acknowledge and isolate the problem to a single point).

A storage API would do something like:

Let map be the result of running obtain a storage bucket area map with environment, "storage", and "localStorage".

It can then use that map as it pleases, for instance in response to API calls.

(Writing this down I realize we should probably expose a separate "obtain a storage bucket area map" and a "obtain a session storage bucket area map" so APIs do not have to pass a storage type.)

It should also provide a solid basis for any kind of multiple storage bucket API though in the absence of one looks a bit complicated.

If this looks agreeable I would like, in order:

Merge this.
Work on defining the replacement operation and get that merged.
Work on a PR for localStorage/sessionStorage to make use of this.
Continue work on storage partitioning for improvements to the storage key.

I could wait with 1 until 2 is further along, but I would greatly appreciate detailed feedback on this PR first.

asutherland · 2020-05-04T00:56:48Z

I understand this to be the hierarchy:

Storage Map
Storage Unit
Storage Bucket
(Storage) Area

While the following sounds very silly, my brain has a lot of trouble with there being no inherent containment relationship among the terms in use and with 3 of the 4 being generic. What about something like:

Storage Shed
Storage Shelf
Storage Bucket
Storage Bottle

annevk · 2020-05-04T05:13:07Z

Note that now it's clearly prefixed, "storage box" would work as well. Nobody commented on that suggestion from #51 (comment).

pwnall · 2020-05-04T09:24:49Z

I'd like to express mild support for keeping bucket. It wasn't the first word that came to my mind, but it has grown on me.

Storage box is a bit of a mouthful. I worry that developers everywhere (web apps, browsers) will end up with variables named box instead of storageBox. I'd definitely use the word "boxes" when discussing this concept with my colleagues.

For better or worse, "bucket" is not used in other parts of the web platform (as far as I know). I think this will make everyone's (web developers, browser vendors) life easier when searching in code / docs. I expect that this concept will be a big deal, and deserves a unique name.

annevk · 2020-05-04T09:48:54Z

Fair. Any thoughts on @asutherland's suggested names? (And the remainder of the PR? 😊)

annevk · 2020-05-04T10:54:05Z

cc @youennf

storage.bs

annevk · 2020-05-07T17:03:09Z

I plan to address the remainder of @domenic's comments tomorrow as well as fully adopt @asutherland's fancy naming scheme. If you all have comments beyond those now would be a great time to make them. Thanks everyone!

inexorabletash

Mostly nitpicks.

I think there will be enough churn after domenic's comments are addressed that another read through will be needed.

inexorabletash · 2020-05-07T17:48:19Z

storage.bs

+
+ <li><p>If <var>key</var> is an <a>opaque origin</a>, then return failure.
+
+ <li><p>If the user has disabled storage, then return failure.


Do we need to define "disabled storage" somewhere?

Maybe as a follow-up? I'm not entirely sure what that would look like, but happy for someone to take a stab at it.

storage.bs

inexorabletash · 2020-05-07T18:20:10Z

storage.bs

+</table>
+
+<p class="note no-backref">Standards can use these <a>storage identifiers</a> with
+<a>obtain a more-durable storage bottle map</a> and <a>obtain a session storage bottle map</a>.


https://youtu.be/mZYu7DnsCF0?t=3

storage.bs

… note about multiple types

… explain need for replacement

annevk · 2020-05-08T09:59:08Z

I did the refactoring in batches, but reviewing https://whatpr.org/storage/86.html#model is probably easier. Note that apart from the Model section this also improved various aspects of the Storage Standard itself, by making it much more clear what exactly various aspects are talking about and when things might fail and such.

inexorabletash

LGTM! (one maybe nit)

storage.bs

domenic · 2020-05-12T19:17:30Z

storage.bs

+
+<p class="XXX">This allows for the <a for="storage proxy map">backing map</a> to be replaced. This
+is needed for <a href="https://github.com/whatwg/storage/issues/4">issue #4</a> and potentially the
+<a href="https://privacycg.github.io/storage-access/">Storage Access API</a>.


I guess maybe it'll be used as part of the future infrastructure, but right now the "proxy map reference set" is append-only, and never read from.

I also share this concern, although it seems like that's a future-work follow-up on the "replace" algorithm proposed at the bottom of #18 (comment) ?

I'm assuming that's what's going to happen, in which case we might as well wait, as it seems likely to be non-trivial.

Yeah you need to know who have a proxy map in order to let their proxies point elsewhere when storage is replaced.

I'm a bit surprised by this feedback though as I thought from #18 that we agreed on this approach.

asutherland

Multiple buckets, here we come! Thank you!

asutherland · 2020-05-12T19:46:54Z

storage.bs

+
+<p class="XXX">This allows for the <a for="storage proxy map">backing map</a> to be replaced. This
+is needed for <a href="https://github.com/whatwg/storage/issues/4">issue #4</a> and potentially the
+<a href="https://privacycg.github.io/storage-access/">Storage Access API</a>.


I also share this concern, although it seems like that's a future-work follow-up on the "replace" algorithm proposed at the bottom of #18 (comment) ?

I'm assuming that's what's going to happen, in which case we might as well wait, as it seems likely to be non-trivial.

domenic

Generally looks good. I think all the support infra for stuff that hasn't landed yet is a bit confusing as a reader, but I'm happy to leave decisions there to editor discretion.

This was referenced Apr 30, 2020

Bucket hooks #18

Closed

Allow 'session' bucket #71

Open

annevk force-pushed the annevk/storage-infrastructure branch from 0e2a03b to f3ca585 Compare May 2, 2020 12:21

Finally define storage infrastructure

fd84f61

TODO Closes #18.

annevk force-pushed the annevk/storage-infrastructure branch from f3ca585 to fd84f61 Compare May 2, 2020 12:39

annevk added 2 commits May 2, 2020 14:49

permission revocation

70e1717

address remaining comments

7d4b2d9

annevk marked this pull request as ready for review May 2, 2020 13:11

annevk requested review from asutherland, domenic and inexorabletash May 2, 2020 13:12

give storage/session-storage endpoints their own function

bdddd00

define storage endpoints (will help with defining replacement)

edae6fc

annevk mentioned this pull request May 4, 2020

Add UsageDetails dictionary #69

Open

domenic reviewed May 4, 2020

View reviewed changes

annevk added 5 commits May 7, 2020 17:54

rename area to bottle and make it standalone

0eaeb5d

more-durable storage

7c967e9

document storage identifiers a bit better

79b89b2

document storage types idea in source for now

4acfe64

storage unit's map -> bucket map

2cbdd5c

inexorabletash reviewed May 7, 2020

View reviewed changes

annevk added 2 commits May 8, 2020 10:36

more-durable -> local

5c797b4

rename storage unit to storage shelf

4185ffc

annevk added 5 commits May 8, 2020 10:51

Add obtain a local storage shelf for internal usage

04e949e

add an intro to the model section, move sheds up

76a62ca

Move storage endpoints up, cover storage type there, and add a public…

6b2cd7e

… note about multiple types

Storage bottles in their own section + cleanup some variables

e67d699

Entry points can return failure, centralize lack of browsing session,…

0ccb17e

… explain need for replacement

annevk requested review from inexorabletash and domenic May 8, 2020 09:59

annevk added 4 commits May 8, 2020 14:35

nit

f9d1a2b

Align with assert style used in URLSearchParams

29563a7

minor cleanup

1c1370d

just "getter steps" now \o/

a1f2afc

inexorabletash approved these changes May 12, 2020

View reviewed changes

storage.bs Outdated Show resolved Hide resolved

domenic reviewed May 12, 2020

View reviewed changes

asutherland approved these changes May 12, 2020

View reviewed changes

foolip mentioned this pull request May 13, 2020

Define how localStorage is synchronized between browser tabs/windows whatwg/html#403

Open

moar feedback

21861db

annevk requested a review from domenic May 13, 2020 12:43

domenic approved these changes May 13, 2020

View reviewed changes

annevk mentioned this pull request May 15, 2020

Do service/shared workers and BroadcastChannel deserve a special strategy? privacycg/storage-partitioning#9

Closed

annevk merged commit 25ac6d4 into master May 15, 2020

annevk deleted the annevk/storage-infrastructure branch May 15, 2020 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify storage infrastructure #86

Clarify storage infrastructure #86

annevk commented Apr 30, 2020 •

edited by pr-preview bot

Loading

annevk commented May 2, 2020

asutherland commented May 4, 2020

annevk commented May 4, 2020

pwnall commented May 4, 2020

annevk commented May 4, 2020

annevk commented May 4, 2020

annevk commented May 7, 2020

inexorabletash left a comment

inexorabletash May 7, 2020

annevk May 8, 2020

inexorabletash May 7, 2020

annevk commented May 8, 2020

inexorabletash left a comment

domenic May 12, 2020

asutherland May 12, 2020

annevk May 13, 2020

annevk May 13, 2020

asutherland left a comment

asutherland May 12, 2020

domenic left a comment


		<li><p>If <var>key</var> is an <a>opaque origin</a>, then return failure.

		<li><p>If the user has disabled storage, then return failure.

Clarify storage infrastructure #86

Clarify storage infrastructure #86

Conversation

annevk commented Apr 30, 2020 • edited by pr-preview bot Loading

annevk commented May 2, 2020

asutherland commented May 4, 2020

annevk commented May 4, 2020

pwnall commented May 4, 2020

annevk commented May 4, 2020

annevk commented May 4, 2020

annevk commented May 7, 2020

inexorabletash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented May 8, 2020

inexorabletash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asutherland left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

annevk commented Apr 30, 2020 •

edited by pr-preview bot

Loading