Re: [w3c/FileAPI] Refactor Blob URL Store definition. (#92)

mkruisselbrink commented on this pull request.



> @@ -1598,286 +1600,161 @@ A URL for Blob and File reference</h2>
 
 This section defines a scheme for a URL used to refer to {{Blob}} objects (and {{File}} objects).

Done

> +
+<h3 id="url-model">
+Model</h3>
+
+Each user agent must maintain a <dfn id="BlobURLStore" export>Blob URL Store</dfn>.
+A [=Blob URL Store=] is a [=map=]
+where [=map/keys=] are [=valid URL strings=]
+and [=map/values=] are [=Blob URL Entries=].
+
+A <dfn>Blob URL Entry</dfn> consists of
+an <dfn for="Blob URL Entry">object</dfn> (typically a {{Blob}},
+but other specs can extend this to refer to other types of objects),
+and an <dfn for="Blob URL Entry">environment</dfn> (an [=environment settings object=]).
+
+[=map/Keys=] in the [=Blob URL Store=] (also known as <dfn lt="Blob URL" export>Blob URLs</dfn>)
+are [=valid URL strings=] that when [=URL parser|parsed=]

Yeah, no, that was entirely my fault. In reality host remains empty and path is a single string containing everything other than the scheme and optional fragment/query.

> +
+Each user agent must maintain a <dfn id="BlobURLStore" export>Blob URL Store</dfn>.
+A [=Blob URL Store=] is a [=map=]
+where [=map/keys=] are [=valid URL strings=]
+and [=map/values=] are [=Blob URL Entries=].
+
+A <dfn>Blob URL Entry</dfn> consists of
+an <dfn for="Blob URL Entry">object</dfn> (typically a {{Blob}},
+but other specs can extend this to refer to other types of objects),
+and an <dfn for="Blob URL Entry">environment</dfn> (an [=environment settings object=]).
+
+[=map/Keys=] in the [=Blob URL Store=] (also known as <dfn lt="Blob URL" export>Blob URLs</dfn>)
+are [=valid URL strings=] that when [=URL parser|parsed=]
+result in a [=/URL=] with a [=url/scheme=] equal to <code>"blob"</code>,
+a [=url/host=] equal to the [=environment settings object/origin=]
+of the [=environment settings object=] in which the URL was create,

as josh pointed out this paragraph actually didn't make sense/didn't match how blob URLs actually are parsed, so I mostly got rid of it, relying instead on the description of how to generate a blob URL to describe the format.

>  
-This section provides sample exchanges between web applications and user agents using <a>Blob URLs</a>.
-A [=/request=] can be triggered using HTML markup of the sort
-<code class="lang-markup">&lt;img src="blob:http://example.org:8080/550e8400-e29b-41d4-a716-446655440000"></code>.
-These examples merely illustrate the [=/request=] and [=/response=];
-web developers are not likely to interact with all the headers,
-but the {{XMLHttpRequest/getAllResponseHeaders()}} method of {{XMLHttpRequest}}, if used,
-will show relevant [=/response=] headers.
+1. [=Assert=]: |url|'s [=url/scheme=] is <code>"blob"</code>.
+1. Let |entry| be the result of [=Blob URL/resolve|resolving=] |url|.
+1. If |entry| is not undefined, return |entry|'s [=Blob URL Entry/environment=]'s [=environment settings object/origin=].
+1. Let |nested url| be the result of [=URL parser|parsing=] |url|'s [=url/path=][0].

> What's the [0] ?

`[=url/path=]` is defined as a (infra) list of strings, so `[0]` just gets the first (and only) element of that list.

> Also, isn't url/path the UUID and url/host is the serialized origin?

Where I wrote that I was wrong, url/path is in fact everything (other than possibly query/fragment) after the scheme.

>  
 1. Let |result| be the empty string.
-  Append the string "blob"
-  (that is, the Unicode code point sequence U+0062, U+006C, U+006F, U+0062)
+1. Append the string "blob"
+  (that is, the Unicode [=code point=] sequence U+0062, U+006C, U+006F, U+0062)

Fixed (and in a few other places where I had the quotes inside the `<code>` rather than the other way around).

>  
 1. Let |result| be the empty string.
-  Append the string "blob"
-  (that is, the Unicode code point sequence U+0062, U+006C, U+006F, U+0062)
+1. Append the string "blob"
+  (that is, the Unicode [=code point=] sequence U+0062, U+006C, U+006F, U+0062)

> You could even make this append the string "blob:" since strings are a well-defined concept now

Good point, done. 

> I wonder though if this is the way we should generate them or if we should create a URL record and then serialize that.

I don't know. I think it might be clearer to just generate the string representation. Otherwise either I'd have to jump through some (irrelevant) hoops to set some of the URL record properties that don't effect serialization, or we'd end up with a URL record for the blob URL that doesn't actually match the URL record you'd ever get from actually parsing a blob URL, neither of which seem ideal. Also there'd still be mostly string manipulation even if I do create a URL record first, as most of the blob URL is just one big opaque string in `path[0]` (unless of course I create a second URL record for that part of the URL, serialize that, wrap that in the path of another URL record and finally serialize that again; but that seems pretty ugly too).


>  
-<h4 id="originOfBlobURL">
-Origin of Blob URLs</h4>
+1. Let |store| be the user agent's [=Blob URL Store=];

Done

>  
-<h4 id="originOfBlobURL">
-Origin of Blob URLs</h4>
+1. Let |store| be the user agent's [=Blob URL Store=];
+1. Let |url| be the result of [=generating a new Blob URL=].

No, generating a blob URL does not depend on object. It only depends on a settings environment (for its origin), but at least currently it just gets that by getting the current settings environment rather than having it be passed in.

>  
-[=CORS protocol|Cross-origin requests=] on Blob URLs must return a <a>network error</a>.
+1. Let |store| be the user agent's [=Blob URL Store=];
+1. Let |url string| be the result of [=URL serializer|serializing=] |url|.
+1. Remove |store|[|url string|].

Done

>  
-<a>Blob URLs</a> are <dfn lt="dereference">dereferenced</dfn>
-when the user agent retrieves the resource identified by the Blob URL
-and returns it to the requesting entity.
-This section provides guidance on [=/requests=] and [=/responses=]
+1. [=Assert=]: |url|'s [=url/scheme=] is <code>"blob"</code>.
+1. Let |store| be the user agent's [=Blob URL Store=];

Done

>  
-This section provides sample exchanges between web applications and user agents using <a>Blob URLs</a>.
-A [=/request=] can be triggered using HTML markup of the sort
-<code class="lang-markup">&lt;img src="blob:http://example.org:8080/550e8400-e29b-41d4-a716-446655440000"></code>.
-These examples merely illustrate the [=/request=] and [=/response=];
-web developers are not likely to interact with all the headers,
-but the {{XMLHttpRequest/getAllResponseHeaders()}} method of {{XMLHttpRequest}}, if used,
-will show relevant [=/response=] headers.
+1. [=Assert=]: |url|'s [=url/scheme=] is <code>"blob"</code>.
+1. Let |entry| be the result of [=Blob URL/resolve|resolving=] |url|.
+1. If |entry| is not undefined, return |entry|'s [=Blob URL Entry/environment=]'s [=environment settings object/origin=].

I see, I wasn't sure what the infra spec meant with "or to return nothing otherwise", which seems to imply that getting a non-existent key is something that is at least sort of supported, but then I wasn't sure how I was supposed to check if it returned something or not. Changed it to your suggested phrasing.

Would it make sense to try to clarify this more in infra (perhaps by making it explicit that you should not try to get a map entry that doesn't exist)?

>  
-<a>Blob URLs</a> are <dfn lt="dereference">dereferenced</dfn>
-when the user agent retrieves the resource identified by the Blob URL
-and returns it to the requesting entity.
-This section provides guidance on [=/requests=] and [=/responses=]
+1. [=Assert=]: |url|'s [=url/scheme=] is <code>"blob"</code>.
+1. Let |store| be the user agent's [=Blob URL Store=];
+1. Let |url string| be the result of [=URL serializer|serializing=] |url| with the *exclude fragment flag* set.
+1. Let |entry| be the result of [=map/getting=] |store|[|url string|].

Done (with the extra if not exists return failure phrasing you suggested in your other comment).

> -If a resource identified with a <a>Blob URL</a> is a {{File}} object,
-user agents use that file's {{File/name}} attribute,
-as if the [=/response=] had a <code>Content-Disposition</code> header
-with the filename parameter set to the {{File}}'s {{File/name}} attribute [[!RFC6266]].
-
-Note: A corollary to this is a non-normative implementation guideline:
-that the "File Save As" user interface in user agents takes into account the {{File}}'s {{File/name}} attribute
-if presenting a default name to save the file as.
-
-<h4 id="NetworkError">
-Network Errors</h4>
-
-Responses that do not succeed with a <a href="#TwoHundredOK">200 OK</a>
-act as if a <a>network error</a> has occurred.
-Network errors are used when:
+Issue: This more or less matches chrome's behavior,

Looking back at some of the discussion about this on https://bugs.chromium.org/p/chromium/issues/detail?id=600074 I think the correct thing to do is just to standardize on the chrome/webkit/edge behavior, and get rid of this line entirely.

>  1. Let |settings| be the [=current settings object=]
 1. Let |origin| be |settings|'s [=environment settings object/origin=].
 1. Let |serialized| be the <a lt="ASCII serialization of an origin">ASCII serialization</a> of |origin|.
 1. If |serialized| is "null", set it to an implementation-defined value.
 1. Append |serialized| to |result|.
-1. Append the "/" character (U+0024 SOLIDUS) to |result|.
-1. Generate a UUID [[RFC4122]] as a Unicode string and append it to |result|.
+1. Append U+0024 SOLIDUS (/) to |result|.
+1. Generate a UUID [[!RFC4122]] as a Unicode string and append it to |result|.

Done

>  
 <h3 id="creating-revoking">
 Creating and Revoking a Blob URL</h3>
 
 <a>Blob URLs</a> are created and revoked using methods exposed on the {{URL}} object,
-supported by global objects {{Window}} [[HTML]] and {{WorkerGlobalScope}} [[Web Workers]].
+supported by global objects {{Window}} and {{WorkerGlobalScope}}.

Done

> @@ -1898,58 +1775,54 @@ unless the user agent also implements the URL [[URL]] specification.
 In other words, <code>URL.prototype</code> must evaluate to true if the user agent implements the URL [[URL]] specification,

Okay, removed it. I'm not quite sure what this section is trying to say anyway.

> -      <code>window2</code> could be an <{iframe}> inside <code>window1</code>.
-
-      <pre class="lang-javascript">
-        myurl = window1.URL.createObjectURL(myblob);
-        window2.URL.revokeObjectURL(myurl);
-      </pre>
-
-      Since <code>window1</code> and <code>window2</code> are in the <a>same origin</a>
-      and share the same <a>Blob URL Store</a>,
-      the <code>URL.{{revokeObjectURL()}}</code> call
-      ensures that subsequent dereferencing of <code>myurl</code>
-      results in a the user agent acting as if a <a>network error</a> has occurred.
-    </div>
-</dl>
+1. Let |url| be the result of [=adding an entry to the Blob URL Store=] for |blob|.
+4. Return |url|.

I don't know, I was thinking I should just use random numbers in my ordered lists since they'll get renumbered anyway :P 

> -      Since <code>window1</code> and <code>window2</code> are in the <a>same origin</a>
-      and share the same <a>Blob URL Store</a>,
-      the <code>URL.{{revokeObjectURL()}}</code> call
-      ensures that subsequent dereferencing of <code>myurl</code>
-      results in a the user agent acting as if a <a>network error</a> has occurred.
-    </div>
-</dl>
+1. Let |url| be the result of [=adding an entry to the Blob URL Store=] for |blob|.
+4. Return |url|.
+
+</div>
+
+<div algorithm="revokeObjectURL">
+The <dfn method for=URL id="dfn-revokeObjectURL">revokeObjectURL(|url|)</dfn> static method must run these steps:
+
+1. [=Remove an entry from the Blob URL Store=] for |url|.

Oops, yes. Although probably I should just stick with straight-up removing the entry with no origin checks or anything like that, since that was what the old spec had and what browsers implement. If we do actually want a origin check when revoking a URL that can be done as a follow-up.

> +
+</div>
+
+<div algorithm="revokeObjectURL">
+The <dfn method for=URL id="dfn-revokeObjectURL">revokeObjectURL(|url|)</dfn> static method must run these steps:
+
+1. [=Remove an entry from the Blob URL Store=] for |url|.
+1. Let |settings| be the [=current settings object=]
+1. Let |origin| be the origin of |url|. FIXME
+1. If |origin| is not [=same origin=] with |settings|'s [=environment settings object/origin=], return.
+1. [=Remove an entry from the Blob URL Store=] for |url|.
+
+Issue: Do we actually need a same-origin check? It doesn't seem like this is something browser currently do, but it seems nice to have.
+
+Note: This means that rather than throwing some kind of error, attempting to revoke a URL that isn't registered will silently fail.
+User agents may display a message on the error console is this happens.

Done

> +
+<div algorithm="revokeObjectURL">
+The <dfn method for=URL id="dfn-revokeObjectURL">revokeObjectURL(|url|)</dfn> static method must run these steps:
+
+1. [=Remove an entry from the Blob URL Store=] for |url|.
+1. Let |settings| be the [=current settings object=]
+1. Let |origin| be the origin of |url|. FIXME
+1. If |origin| is not [=same origin=] with |settings|'s [=environment settings object/origin=], return.
+1. [=Remove an entry from the Blob URL Store=] for |url|.
+
+Issue: Do we actually need a same-origin check? It doesn't seem like this is something browser currently do, but it seems nice to have.
+
+Note: This means that rather than throwing some kind of error, attempting to revoke a URL that isn't registered will silently fail.
+User agents may display a message on the error console is this happens.
+
+Note: Attempts to dereference |url| after it has been revoked will result in a [=network error=].

Well, the problem is that you can fetch a blob URL after it was revoked just fine as long as the dereferencing (as part of the URL parsing algorithm) happened before revoking... Not sure how to best phrase that. Maybe since this is non-normative it's okay to just s/dereference/fetch, but it wouldn't be entirely true.

> +Introduction</h3>
+
+*This section is informative.*
+
+[=Blob URL|Blob (or object) URLs=] are URLs like
+<code>blob:http://example.com/550e8400-e29b-41d4-a716-446655440000</code>.
+This enables integration of {{Blob}}s and {{File}}s with other Web Platform APIs
+that are only designed to be used with URLs, such as the <{img}> element.
+[=Blob URLs=] can also be used to navigate to as well as to trigger downloads of
+locally generated data.
+
+For this purpose two static methods are exposed on the {{URL}} interface,
+{{URL/createObjectURL(blob)}} and {{URL/revokeObjectURL(url)}}.
+The first method creates a mapping from a URL to a Blob,
+and the second method revokes said mapping.
+As long as a mapping exist a Blob  can't be garbage collected,

Done

> +*This section is informative.*
+
+[=Blob URL|Blob (or object) URLs=] are URLs like
+<code>blob:http://example.com/550e8400-e29b-41d4-a716-446655440000</code>.
+This enables integration of {{Blob}}s and {{File}}s with other Web Platform APIs
+that are only designed to be used with URLs, such as the <{img}> element.
+[=Blob URLs=] can also be used to navigate to as well as to trigger downloads of
+locally generated data.
+
+For this purpose two static methods are exposed on the {{URL}} interface,
+{{URL/createObjectURL(blob)}} and {{URL/revokeObjectURL(url)}}.
+The first method creates a mapping from a URL to a Blob,
+and the second method revokes said mapping.
+As long as a mapping exist a Blob  can't be garbage collected,
+so some care must be taken to revoke a URL as soon as a reference is no longer needed.
+All URLs are revoked when the global that created the URL itself goes away.

Done

> +that are only designed to be used with URLs, such as the <{img}> element.
+[=Blob URLs=] can also be used to navigate to as well as to trigger downloads of
+locally generated data.
+
+For this purpose two static methods are exposed on the {{URL}} interface,
+{{URL/createObjectURL(blob)}} and {{URL/revokeObjectURL(url)}}.
+The first method creates a mapping from a URL to a Blob,
+and the second method revokes said mapping.
+As long as a mapping exist a Blob  can't be garbage collected,
+so some care must be taken to revoke a URL as soon as a reference is no longer needed.
+All URLs are revoked when the global that created the URL itself goes away.
+
+<h3 id="url-model">
+Model</h3>
+
+Each user agent must maintain a <dfn id="BlobURLStore" export>Blob URL Store</dfn>.

Done

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/FileAPI/pull/92#discussion_r155221223