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.

Sign up for GitHub

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

Jump to bottom

Enhance IDB, CacheStorage algorithms. #54

Merged
merged 2 commits into from
Jan 4, 2023
Merged

Enhance IDB, CacheStorage algorithms. #54

merged 2 commits into from
Jan 4, 2023

Conversation

evanstade
Copy link
Collaborator

@evanstade evanstade commented Jan 4, 2023 •
edited by pr-preview bot
Loading

Also distinguish between the conceptual bottle and the StorageBottle object in the open() definition.

Preview | Diff

Enhance IDB, CacheStorage algorithms.
2cb97dc 
Also distinguish between the conceptual bottle and the StorageBottle object in the
open() definition.
ayuishii
ayuishii reviewed Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@ayuishii ayuishii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question, but otherwise lgtm!

index.bs

1. Let |cacheStorage| be a {{CacheStorage}} object.

1. Set the <a spec="service-workers">relevant name to cache map</a> for |cacheStorage| to |bottle map|.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be [=obtain a local storage bottle map=] similar to indexedDB? relevant name to cache map looks like it might be keyed by cacheName within a bucket, and not a key for buckets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I believe there is some confusion here because

  • a bucket has a "bottle map", i.e. map of bottle identifiers to bottles, mapping "cacheStorage" to a storage bottle for CacheStorage
  • a storage bottle has a map, used for:

A storage bottle’s map is where the actual data meant to be stored lives

The algorithm "obtain a local storage bottle map" refers to the (storage bottle)'s (map), rather than a (storage bucket)'s (bottle map).

Does this clear things up?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, somehow missed L302 😵 lgtm!

ayuishii
ayuishii reviewed Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@ayuishii ayuishii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit comments

index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
ayuishii
ayuishii approved these changes Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@ayuishii ayuishii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

index.bs

1. Let |cacheStorage| be a {{CacheStorage}} object.

1. Set the <a spec="service-workers">relevant name to cache map</a> for |cacheStorage| to |bottle map|.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, somehow missed L302 😵 lgtm!

@evanstade evanstade merged commit 3d1b4d2 into WICG:spec-draft Jan 4, 2023
ayuishii added a commit that referenced this pull request Mar 16, 2023
@evanstade @ayuishii @jyasskin
Add initial draft of spec (#62)
5126125 
* Add IndexedDB text. (#46)

* Updates around user visibility of buckets.

* Add self as author.

* Clarify wording.

* Add bucket durability and IndexedDB text.

* algorithm tag

Co-authored-by: Evan Stade <estade@chromium.org>

* Add caches and getDirectory text. (#51)

Co-authored-by: Evan Stade <estade@chromium.org>

* Creating a bucket (#48)

* Spec for creating a bucket
* fix english
* Address comments

* Algorithms for delete (#50)

* Fix bad merge

* Algorithm for keys() (#52)

* Add caches and getDirectory text. (#47)

* Updates around user visibility of buckets.

* Add self as author.

* Clarify wording.

* Add bucket durability and IndexedDB text.

* algorithm tag

* caches and getDirectory

* remove abbreviation

Co-authored-by: Evan Stade <estade@chromium.org>

* Revert "Add caches and getDirectory text. (#47)" (#49)

This reverts commit a2fa5bc.

* Algorithm for keys()

* fix append

Co-authored-by: Evan Stade <estade@gmail.com>
Co-authored-by: Evan Stade <estade@chromium.org>

* spec-draft rebase (#53)

* Add caches and getDirectory text. (#47)

* Updates around user visibility of buckets.

* Add self as author.

* Clarify wording.

* Add bucket durability and IndexedDB text.

* algorithm tag

* caches and getDirectory

* remove abbreviation

Co-authored-by: Evan Stade <estade@chromium.org>

* Revert "Add caches and getDirectory text. (#47)" (#49)

This reverts commit a2fa5bc.

Co-authored-by: Evan Stade <estade@gmail.com>
Co-authored-by: Evan Stade <estade@chromium.org>

* Enhance IDB, CacheStorage algorithms. (#54)

* Enhance IDB, CacheStorage algorithms.

Also distinguish between the conceptual bottle and the StorageBottle object in the
open() definition.

* algo tags

Co-authored-by: Evan Stade <estade@chromium.org>

* Add more algorithms for StorageBucket attributes. (#55)

* Add more algorithms for StorageBucket attributes.

* reference StorageEstimate object

Co-authored-by: Evan Stade <estade@chromium.org>

* Enhancements. (#56)

* Enhancements.

1. Specify when expired buckets are removed.

2. Improve some text around durability, quota.

* fix some link errors

* or equal to

* Review updates

Co-authored-by: Evan Stade <estade@chromium.org>

* Write persist/persisted algos (#57)

* Write persist/persisted algos

* no backticks on true

Co-authored-by: Evan Stade <estade@chromium.org>

* Fill in some error types. (#58)

* Write persist/persisted algos

* no backticks on true

* Fill in some error types.

* Indentation fixes

* Fix redundant step.

* Streamline delete bucket.

Co-authored-by: Evan Stade <estade@chromium.org>

* Add Clear Site Data. (#59)

* Add Clear Site Data.

Most of this text needs to be worked into various parts of the CSD spec.

* Better annotations

Co-authored-by: Evan Stade <estade@chromium.org>

* Address review feedback (#63)

Co-authored-by: Evan Stade <estade@chromium.org>

* Spec draft (#64)

* Address review feedback

* additional review comment

---------

Co-authored-by: Evan Stade <estade@chromium.org>

* Spec draft (#65)

* Address review feedback

* additional review comment

---------

Co-authored-by: Evan Stade <estade@chromium.org>

* Spec draft (#66)

* Address review feedback

* additional review comment

---------

Co-authored-by: Evan Stade <estade@chromium.org>

* fix typo (#67)

Co-authored-by: Evan Stade <estade@chromium.org>

* Address comments (#68)

* Use TypeError

* Update index.bs

* null or undefined

* s/and/, then/

* backticks instead of <code>

* Address comments #2 (#69)

* durability value

* quota value

* expiration value

* Address comments 3 (#70)

* quota value in bytes

* code point

* Operate open bucket on storage bucket map

* use bucket map & update persist() (#71)

* Expand usage of bucket map

* remove the

* Use bucket map for clear site data

* short-circuit if bucket is already persistent

* Update time & expiration (#72)

* Update time

* Update time 2

* check for null bucket expiration

* parse duration string

* Fix link to html spec

* make durability() not async (#73)

* Address comments (#74)

* Update to use TypeError instead of UnknownError

* Remove obtaining |storageKey|

* Fix parallel steps

* Fix dfn for storage usage

* Fix parallel steps for keys() & delete()

* Fix delete()

* Handle undefined quota

* Use wall time

* UA bucket clearing wording

* Add bucket to bucket map on creation

* Fix indent

* Add algorithm for bucket removal (#75)

* Add algorithm for bucket removal

* Update index.bs

* update algorithms with remove

* Fix wording

* Fix wording #2

* Add issues for storage endpoint quota eviction (#77)

* Address comments (#78)

* Address comments

* Fix bucket removal step
* Add Service Worker deletion to issue
* Remove string parsing step for expires

* Update duration to moment

* get or expire a bucket

* Allow UA bucket deletion on expiration

* Nit

* Apply suggestions by @jyasskin

Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>

* Update expiration (#80)

* expiration time

* Specify moment on the wall clock

* Use [=Unix epoch=]

* Mark -> set

* Fix expiration comparison

* Apply jyasskin@'s suggestions

Use "before" instead of "less than or equal"

Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>

* Use queue a task (#81)

* Use [=queue a task=]

* Additional changes

* Update persist() and add note on Promises (#82)

* Fix persist() to not reference [=this=] in parallel steps

* Note on promises

---------

Co-authored-by: Evan Stade <estade@chromium.org>
Co-authored-by: Ayu Ishii <48255136+ayuishii@users.noreply.github.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayuishii ayuishii ayuishii approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@evanstade @ayuishii