-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implement URL.createObjectURL
and URL.revokeObjectURL
methods
#187
base: main
Are you sure you want to change the base?
Conversation
Hey @guybedford I was hoping I could get some early feedback about the initial commit for this work: 8d7d5c6. Specifically:
|
Will ask @tschneidereit if he has any ideas about the UUID implementation - is there a small implementation we could inline into the codebase that avoids unnecessary code and might save some bytes? In general we want to watch binary size very carefully. As for the request implementation it would be supporting the Blob URL everywhere we normally support a URL - so specifically The worker location is the base origin to use. |
Just for context, the size difference between the binary from main and this branch is 26KB. I'm not sure how much of this is due to the |
Actually I think you could just use our existing implementation of |
Nice, I totally missed that :) |
6286ef3
to
88d10f2
Compare
88d10f2
to
e43472b
Compare
"status": "PASS" | ||
}, | ||
"Fetching URL.createObjectURL(invalid_type_blob) is OK": { | ||
"status": "FAIL" |
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 test checks that blob created with type: "invalid"
should result in empty Content-Type
header. It assumes that the blob type should be mime
parsable although this assumption is not consistent with other blob tests and the spec.
As stated here: https://w3c.github.io/FileAPI/#constructorBlob
If the type member of the options argument is not the empty string, run the following sub-steps:
- Let t be the type dictionary member. If t contains any characters outside the range U+0020 to U+007E, then set t to the empty string and return from these substeps.
- Convert every character in t to ASCII lowercase.
I believe this is done, although it's still depends on #181 being merged first. |
@@ -17,7 +17,7 @@ list(APPEND CMAKE_EXE_LINKER_FLAGS | |||
list(JOIN CMAKE_EXE_LINKER_FLAGS " " CMAKE_EXE_LINKER_FLAGS) | |||
|
|||
list(APPEND CMAKE_CXX_FLAGS | |||
-std=gnu++20 -Wall -Werror -Qunused-arguments | |||
-std=gnu++20 -Wall -Werror -Qunused-arguments -Wimplicit-fallthrough |
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.
Sorry for the noise here. I just spent way too much time debugging problems that were result of missing break; This just helps me to remember that switch
is not match
.
return false; | ||
} | ||
|
||
if (!URL_STORE.get().put(result, obj)) { |
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.
We could probably only use uuid
part of url as a key to save some cycles.
This adds
URL.createObjectURL
andURL.revokeObjectURL
static methods to URL as well as support for blob-scheme fetch:Some noteworthy changes include:
crypto
component,fetch-https
andfetch-blob
functions called based on url scheme,Closes: #182