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

Define the lifetime of a blob URL created inside a service worker #688

Closed
ehsan opened this Issue Apr 28, 2015 · 14 comments

Comments

Projects
None yet
8 participants
@ehsan

ehsan commented Apr 28, 2015

Right now in Gecko at least we tie the lifetime of such URLs to the Worker or SharedWorker that created them, but doing the same (as in, invalidating the blob URL when the service worker goes away) seems wrong, since that can happen at any time at UA's discretion.

How should we handle this?

One option that comes to my mind right now is keeping those URLs valid until the service worker that registered them is unregistered.

@jakearchibald

This comment has been minimized.

Show comment
Hide comment
@jakearchibald

jakearchibald Apr 28, 2015

Collaborator

I think Gecko's current behaviour is correct. Yes, it may get GC'd when the SW closes, but same with variables. If you want a long term way to store a response, the caches api is the way to go right?

Tieing it to registration feels wrong as it would need to survive browser shut down.

What is the current behaviour breaking? I'm wondering if a way to access items in the cache api by url may be a better solution.

Collaborator

jakearchibald commented Apr 28, 2015

I think Gecko's current behaviour is correct. Yes, it may get GC'd when the SW closes, but same with variables. If you want a long term way to store a response, the caches api is the way to go right?

Tieing it to registration feels wrong as it would need to survive browser shut down.

What is the current behaviour breaking? I'm wondering if a way to access items in the cache api by url may be a better solution.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Apr 30, 2015

Member

We should maybe restrict this API inside service workers since it's a giant memory leak. And only allow the one-time-minting variant.

Member

annevk commented Apr 30, 2015

We should maybe restrict this API inside service workers since it's a giant memory leak. And only allow the one-time-minting variant.

@jakearchibald

This comment has been minimized.

Show comment
Hide comment
@jakearchibald

jakearchibald May 27, 2015

Collaborator

Is it a giant memory leak if the data is GC'd when the SW terminates?

Collaborator

jakearchibald commented May 27, 2015

Is it a giant memory leak if the data is GC'd when the SW terminates?

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Jun 9, 2015

Member

Depends on whether service workers will forever terminate quickly. It seems better to simply not offer APIs that are known to be problematic.

Member

annevk commented Jun 9, 2015

Depends on whether service workers will forever terminate quickly. It seems better to simply not offer APIs that are known to be problematic.

@slightlyoff slightlyoff added this to the Version 1 milestone Oct 28, 2015

@jakearchibald

This comment has been minimized.

Show comment
Hide comment
@jakearchibald

jakearchibald Oct 28, 2015

Collaborator

@annevk I really don't think this is a problem any more than it is on pages. Service workers are generally not alive longer than pages.

Collaborator

jakearchibald commented Oct 28, 2015

@annevk I really don't think this is a problem any more than it is on pages. Service workers are generally not alive longer than pages.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Nov 1, 2015

Member

Right, that argument was made, and the argument against was that it might not remain that way. I guess the other question is why we need blob URLs in service workers since we can just use Request/Response objects.

Member

annevk commented Nov 1, 2015

Right, that argument was made, and the argument against was that it might not remain that way. I guess the other question is why we need blob URLs in service workers since we can just use Request/Response objects.

@jungkees

This comment has been minimized.

Show comment
Hide comment
@jungkees

jungkees Jan 14, 2016

Collaborator

Need a resolution. Would like to discuss at the f2f.

Collaborator

jungkees commented Jan 14, 2016

Need a resolution. Would like to discuss at the f2f.

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto Jan 22, 2016

Member

Implementor feedback from Chrome: if we continue to allow blob URLs in service workers, tieing the lifetime to the service worker (i.e., the execution context that created it, not the registration) sounds good. That's what Chrome does already.

Member

mattto commented Jan 22, 2016

Implementor feedback from Chrome: if we continue to allow blob URLs in service workers, tieing the lifetime to the service worker (i.e., the execution context that created it, not the registration) sounds good. That's what Chrome does already.

@jakearchibald

This comment has been minimized.

Show comment
Hide comment
@jakearchibald

jakearchibald Jan 27, 2016

Collaborator

F2F resolution: remove createObjectURL

Collaborator

jakearchibald commented Jan 27, 2016

F2F resolution: remove createObjectURL

@jungkees

This comment has been minimized.

Show comment
Hide comment
@jungkees

jungkees Jan 29, 2016

Collaborator

@annevk Is it better to make URL.createObjectURL(blob), URL.createFor(blob) and URL.revokeObjectURL(url) explicitly throw in File API spec?

Collaborator

jungkees commented Jan 29, 2016

@annevk Is it better to make URL.createObjectURL(blob), URL.createFor(blob) and URL.revokeObjectURL(url) explicitly throw in File API spec?

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Jan 29, 2016

Member

@jungkees no, I would get the File API specification to use [Exposed=] similar to how XMLHttpRequest does that for e.g., responseXML, to hide these from service workers.

Member

annevk commented Jan 29, 2016

@jungkees no, I would get the File API specification to use [Exposed=] similar to how XMLHttpRequest does that for e.g., responseXML, to hide these from service workers.

@wanderview

This comment has been minimized.

Show comment
Hide comment
Member

wanderview commented Apr 13, 2016

@jungkees

This comment has been minimized.

Show comment
Hide comment
@jungkees

jungkees Apr 16, 2016

Collaborator

Closing with w3c/FileAPI#31 merged.

Collaborator

jungkees commented Apr 16, 2016

Closing with w3c/FileAPI#31 merged.

@inexorabletash

This comment has been minimized.

Show comment
Hide comment
@inexorabletash

inexorabletash Apr 22, 2016

Member

Blink tracking bug: https://bugs.chromium.org/p/chromium/issues/detail?id=604951

I filed w3c/media-source#67 since MediaSource has createObjectURL as well.

MediaStream had it as well at some point but it's been removed from the spec.

Member

inexorabletash commented Apr 22, 2016

Blink tracking bug: https://bugs.chromium.org/p/chromium/issues/detail?id=604951

I filed w3c/media-source#67 since MediaSource has createObjectURL as well.

MediaStream had it as well at some point but it's been removed from the spec.

dstockwell pushed a commit to dstockwell/chromium that referenced this issue May 12, 2016

Don't expose URL.createObjectURL and revokeObjectURL to Service Workers
Per FileAPI[1] which was updated based on Service Worker discussion[2],
minting blob URLs should not be possible within SWs.

A bug has been filed against MediaSource[3] to update the IDL there
(following the same logic), and mediastream's createObjectURL is no
longer present in the standard. Regardless, the overloads should
never execute in Service Workers as those types (MediaStream,
MediaSource) are not exposed in that context.

Note that the [Exposed] attributes are applied directly to the
interface members as they appear to be ignored on the partial
interface. (Bug?)

[1] https://w3c.github.io/FileAPI/#creating-revoking
[2] w3c/ServiceWorker#688
[3] w3c/media-source#67

Intent Thread: https://groups.google.com/a/chromium.org/d/msg/blink-dev/HuA7Ng9U0oc/CYvfMoeyBwAJ

BUG=604951,608460

Review-Url: https://codereview.chromium.org/1908263002
Cr-Commit-Position: refs/heads/master@{#393271}

moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Aug 7, 2018

Hide URL.createObjectURL from ServiceWorker
The appropriate lifetime for URLs created with URL.createObjectURL turned
out to be tricky to define, so it was decided to hide it from service
workers altogether. (w3c/ServiceWorker#688)

This commit implements this change and adds a web platform test to verify it.
It also exposes the MediaSource variant of URL.createObjectURL in DedicatedWorker
and SharedWorker contexts in order to comply with the WebIDL spec (see
w3c/media-source#168 (comment)).

Differential Revision: https://phabricator.services.mozilla.com/D2728

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1264182
gecko-commit: 40d4999db3191bca0aeb75a50b37537ad78102ed
gecko-integration-branch: autoland
gecko-reviewers: mrbkap, asuth

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 7, 2018

Bug 1264182: Hide URL.createObjectURL from ServiceWorker r=mrbkap,asuth
The appropriate lifetime for URLs created with URL.createObjectURL turned
out to be tricky to define, so it was decided to hide it from service
workers altogether. (w3c/ServiceWorker#688)

This commit implements this change and adds a web platform test to verify it.
It also exposes the MediaSource variant of URL.createObjectURL in DedicatedWorker
and SharedWorker contexts in order to comply with the WebIDL spec (see
w3c/media-source#168 (comment)).

Differential Revision: https://phabricator.services.mozilla.com/D2728

--HG--
extra : moz-landing-system : lando

jankeromnes pushed a commit to jankeromnes/gecko that referenced this issue Aug 8, 2018

Bug 1264182: Hide URL.createObjectURL from ServiceWorker r=mrbkap,asuth
The appropriate lifetime for URLs created with URL.createObjectURL turned
out to be tricky to define, so it was decided to hide it from service
workers altogether. (w3c/ServiceWorker#688)

This commit implements this change and adds a web platform test to verify it.
It also exposes the MediaSource variant of URL.createObjectURL in DedicatedWorker
and SharedWorker contexts in order to comply with the WebIDL spec (see
w3c/media-source#168 (comment)).

Differential Revision: https://phabricator.services.mozilla.com/D2728

moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Aug 8, 2018

Hide URL.createObjectURL from ServiceWorker
The appropriate lifetime for URLs created with URL.createObjectURL turned
out to be tricky to define, so it was decided to hide it from service
workers altogether. (w3c/ServiceWorker#688)

This commit implements this change and adds a web platform test to verify it.
It also exposes the MediaSource variant of URL.createObjectURL in DedicatedWorker
and SharedWorker contexts in order to comply with the WebIDL spec (see
w3c/media-source#168 (comment)).

Differential Revision: https://phabricator.services.mozilla.com/D2728

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1264182
gecko-commit: 40d4999db3191bca0aeb75a50b37537ad78102ed
gecko-integration-branch: autoland
gecko-reviewers: mrbkap, asuth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment