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.

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

createObjectURL() not up-to-date with File API #10

Closed
phantom10111 opened this issue Sep 30, 2015 · 9 comments
Closed

createObjectURL() not up-to-date with File API #10

phantom10111 opened this issue Sep 30, 2015 · 9 comments

Comments

@phantom10111
Copy link

The description of createObjectURL() has the following note:

This algorithm is intended to mirror the behavior of the createObjectURL()[FILE-API] method with autoRevoke set to true.

The note follows with an algorithm similar to the one in a draft from 25 October 2012

The issue is that the current version of File API has reworded the description of the algorithm and removed the autoRevoke parameter from that function. createObjectURL() will now always create non auto revoking URLs. createFor() was added to create auto revoking URLs.

I suggest that createFor() should be added to the specification and that the behaviour of both functions should mirror the appropriate functions from the current File API. If for some reason you only want to provide the auto revoking version, I suggest that the function should be called createFor() to avoid confusion with File API.

I personally think that forcing users to deal with the URL auto revoking at an unspecified time isn't a great thing to do and that both versions should be provided.

@jdsmith3000 jdsmith3000 modified the milestone: V.Next Oct 13, 2015
@jdsmith3000 jdsmith3000 added this to the V1 milestone Oct 15, 2015
@jdsmith3000
Copy link
Contributor

I don't object to the recommendation to keep createObjectURL() in non-autorevoking form, and adding createFor() as the auto-revoking alternative.

@wolenetz
Copy link
Member

non-autorevoking createObjectURL() change would be a breaking change (and potentially require re-entry of spec into CR phase). I tend to not want to make such a change.

@plehegar plehegar self-assigned this Nov 10, 2015
@yfuna yfuna assigned yfuna and unassigned plehegar Feb 20, 2016
@yfuna
Copy link

yfuna commented Feb 26, 2016

I thought over this issue and support changing the definition of createObjectURL() in a way aligned with the current File API WD. Let me explain.

Firstly, the Web Apps WG has changed the definition of createObjectURL() in the File API spec because the old definition in the Working Draft 25 October 2012 had a bug or unacceptable ambiguity in semantics, esp. around the term "stable state." The new definition in the Working Draft 21 April 2015 has clearer semantics by introducing two new specification artifacts: the Blob URL Store and the Revocation List.

In light of this reason to have improved the definition in the File API spec, referencing the old definition from the MSE spec would be inadequate because it would introduce a bug in semantics which the Web Apps WG avoided. This is the first reason why we should change it.

Secondly, this change wouldn't bring any problem to existing Web applications which use the MSE createObjectURL() with assuming the old definition in the File API spec. This is because the logic of the applications should work fine with autoRevoke=false behavior of the function, given that the app's logic always needs to take care of the case where an auto-revoking object url would be never automatically revoked considering that the old definition of the File API spec left it unspecified when a user agent actually revokes an object url under autoRevoke=true.

In other words, the api change is semantically backward compatible for api users under the condition that when to revoke is unspecified.

Thirdly, the current implementation status of createObjctURL() for File API and MSE is also positive to change it to the new definition. Here is a rough implementation report I created by skimming the source code of Blink, Firefox, and Webkit:

createObjectURL() for Blob (File API)

  • Blink: non auto revoking; follows the new definition.
  • Firefox: non auto revoking; follows the new definition.
  • Webkit: non auto revoking; follows the new definition.

createObjectURL() for MediaSource (MSE)

  • Blink: non auto revoking; shares the internal function with createObjectURL() for Blob
  • Firefox: perhaps non auto revoking but with garbage collection; a different implementation from createObjectURL() for Blob
  • Webkit: non auto revoking; shares the internal function with createObjectURL() for Blob

Fourthly and lastly, the File API spec looks stable around creatObjectURL() after the WG published a WD with the new definition on April, 2015.

Putting all these considerations together, changing the definition in the MSE spec to the new one would be not harmful but fully beneficial to Web app developers, and I believe this change would be also beneficial to implementers thanks to the clearer semantics.

As for the createFor() api, the fact that createFor() in the File API spec has not yet been implemented in any of the UAs above is discouraging to add it to MSE. I recommend we don't adopt it.

What do you think?

@wolenetz
Copy link
Member

@yfuna : Thanks for the excellent diligence on this. I agree with changing MSE spec to refer to the current FILE API spec, retaining usage of just createObjectURL() while dropping the autoRevoke language from MSE as well as steps 2 (provide stable state) and 3 (revokeObjectURL) of MSE createObjectURL.

I also think we can leave createFor() out of this V1 bug and reconsider it perhaps if it is needed in MSE later.

I request a sanity check though: would non-auto-revoked object URLs for MediaSource objects prevent effective garbage collection of those objects by user agents unless apps explicitly revoke those object URLs? If so, to prevent undue risk of memory pressure to long-lasting documents which may use multiple MediaSource objects over their lifetimes, I further recommend adding some non-normative note to the MSE spec giving guidance to web authors to consider explicitly revoking the objectURLs for MediaSources by calling revokeObject() on their objectURLs to allow more effective resource reclamation, when the MediaSources are no longer going to be used by the web app.

@yfuna
Copy link

yfuna commented Apr 12, 2016

@wolenetz : I've checked the source code of Blink, Firefox, and Webkit and found that all of them seemingly keep the MediaSource object in memory while there remains a Blob URL referencing to it. It means, the UAs will keep the object unless apps explicitly revoke object URLs.

I think it's a great idea to add a non-normative note to encourage Web apps authors to explicitly revoke object URLs when they don't need them anymore.

Would you like to write the note by yourself or would you like me to write a draft note?

@wolenetz
Copy link
Member

@yfuna - if you could prepare a PR, that would be great. If not, assign to me and I'll get that done. Thanks!

@paulbrucecotton
Copy link

@yfuna: Are you going to create the PR or should one of the Editors do this?

@paulbrucecotton
Copy link

@wolenetz: I suggest you go ahead and create the necessary PR for this issue.

@wolenetz
Copy link
Member

038cdcf fixes this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants