addAll should resolve when the cache is fully & successfully written #884
Comments
|
I think this is part of removing the incumbent/fetching spec language. I agree we should not commit addAll() to storage until we have all the bodies. And we shouldn't remove any previous entries with the same requests until that commit is done. At least, thats what gecko does now which makes me biased towards it. :-) |
|
Pre F2F notes: Probably nothing to discuss unless anyone has changed their minds. |
jungkees
added a commit
that referenced
this issue
Aug 22, 2017
This change removes the incumbent/fetching record concept that allowed committing a fetching resource to cache and match/matchAll to return a fully written existing resource, if any, with priority over a fetching resource. After this change, add/addAll/put promises resolve when the resources ared fully fetched and committed to cache. match/matchAll will not return any on-going fetching resources. This changes the specification type of underlying cache objects from a map (request to response map) to a list (request response list). The change is to make the arguments and the return values of the Cache methods and algorithms (Batch Cache Operations and Query Cache) conform to one another. The list type seems fine as the algorithms tend to iterate through the list to find certain items with search options. Looking at the details of the acutal implementations, I plan to update it further if needed. Fixes #884.
jungkees
added a commit
that referenced
this issue
Oct 13, 2017
* Remove incumbent/fetching record from Cache behavior
- Remove the incumbent/fetching record concept that allowed committing
a fetching resource to cache and match/matchAll to return a fully
written existing resource, if any, with priority over a fetching
resource. After this change, add/addAll/put promises resolve when the
resources ared fully fetched and committed to cache. match/matchAll
will not return any on-going fetching resources.
- Change the specification type of underlying cache objects from a map
(request to response map) to a list (request response list). The
change is to make the arguments and the return values of the Cache
methods and algorithms (Batch Cache Operations and Query Cache)
conform to one another. The list type seems fine as the algorithms
tend to iterate through the list to find certain items with search
options.
* Correct the use of objects associated to JS realm and global
- Replace CacheBatchOperations dictionary which isn't exposed to
JavaScript surface with cache batch operations struct.
- Do not store JS objects in the storage but store request and response
structs instead.
- Create and return JS objects in the target realm when requested (from
matchAll() and keys()).
* Refactor algorithms
- Simplify "put" operation related steps by moving/refactoring the
post-Batch Cache Operation steps, which clear the invalid items, from
addAll() and put() into Batch Cache Operations.
- Move the argument validation steps of cache.keys() out of the
parallel thread to main thread.
- Fix cacheStorage.keys() to run the steps async.
- Change to early-exit in fetch abort cases.
- Fix async steps of fulfillment handlers.
- Change the interface of Batch Cache Operations algorithm.
. Change to not return a promise
. Remove the in parallel steps and make it work synchronously
. Change the call sites to call it in a created promise's in parallel
steps
Fixes #884.
* NB: HTML files in this commit aren't updated due to huge merge
conflicts. A separate commit with the updated HTMLs will follow this
commit with a proper link to it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
At the moment it resolves on headers, which doesn't feel like a good fit for install
waitUntil.We changed
.put, and I think it's an oversight that we didn't fix it foraddAll. What do you think @wanderview?The text was updated successfully, but these errors were encountered: