Bug 20582 - Definitions of "valid key" and key comparison are underdefined and not compatible with WebIDL
Definitions of "valid key" and key comparison are underdefined and not compat...
Status: RESOLVED FIXED
Product: WebAppsWG
Classification: Unclassified
Component: Indexed Database API
unspecified
PC All
: P2 normal
: ---
Assigned To: Joshua Bell
public-webapps-bugzilla
:
Depends on:
Blocks: 20394
  Show dependency treegraph
 
Reported: 2013-01-07 01:52 UTC by Boris Zbarsky
Modified: 2013-04-02 18:46 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Joshua Bell 2013-01-08 23:19:05 UTC
The structured clone algorithm in HTML is similarly under-defined. Referring to http://lists.w3.org/Archives/Public/public-webapps/2012OctDec/0087.html for numbering:

1) Array - similarly under-defined in structured clone algorithm

2) float - agree with Boris' suggestion to change this to "unrestricted float", although given the tight dependency between this spec and ECMA-262, dropping the WebIDL-isms and just using "string primitive" or "number primitive" might be more appropriate.

3) getters - the structured clone algorithm is similarly underdefined here, although it has text about enumerable properties, getters and setters, and not walking the prototype chain. Presumably we would want similar text.

4) toString/valueOf - the structured clone algorithm distinguishes between Objects, and primitive values. Presumably we would want similar text.

5) Date - similarly under-defined in structured clone algorithm

6, 7) comparison - I believe all implementations act AS IF they have performed a structured clone on the key value after validity is asserted and the clone is used rather than the original object for all further operations. (Chrome uses a custom data structure, I assume other browsers do the same). This means by the time a comparison is being performed there are guaranteed to be no side effects i.e. an implementation cannot observe the comparison algorithm in progress.

I believe #3, #6, #7 could be clarified by explicitly invoking the structured clone algorithm and having operations defined as acting on the clone. That, at least, pushes off the problem of side effects to the structured clone algorithm and makes the platform behave consistently.

#1, #2, #4 and #5 still bypass WebIDL, but in a manner identical to the structured clone algorithm. Is there work under way to define structured clone in terms of WebIDL that could be leveraged, for consistency?
Comment 2 Joshua Bell 2013-01-08 23:22:41 UTC
See also https://www.w3.org/Bugs/Public/show_bug.cgi?id=20394
Comment 3 Boris Zbarsky 2013-01-09 02:19:54 UTC
Structured cloning is defined to work on ES values, not on WebIDL values last I checked.  If it doesn't say that clearly, it really should.  When it says Array it means it the way it's meant in ECMA-262: things whose internal [[Class]] is Array.

I would probably be OK with IndexedDB clearly defining that it's working on ES values, and really has nothing to do with WebIDL.  It's mixing the two that's a problem, since getting a WebIDL anything out of an ES value involves type-coercion in various ways.

> 3) getters - the structured clone algorithm is similarly underdefined here, 

How so?  It seems quite clearly defined.  It describes the exact order in which you call getters on all the objects it operates on, assuming that your enumeration order is defined.  All structured cloning does is enumerate all the properties and grab their values.  It doesn't try to talk about anything being defined, or any constraints on property values.  It also clearly defines cycle handling and such.

> 4) toString/valueOf - the structured clone algorithm distinguishes between
> Objects, and primitive values

This is easy to do in the ES context.  WebIDL doesn't so much have great tools for this, though "any" can represent the difference in some cases.  You just have to be a little careful how you state things.

> 6, 7) comparison - I believe all implementations act AS IF they have
> performed a structured clone on the key value after validity is asserted and
> the clone is used rather than the original object for all further operations.

OK.  If that's how it works and if the spec explicitly defines the exact point in time when the structured clones happen, that would resolve issues 6 and 7.  There is no problem with structured cloning here; it already completely defines order of operations, to the extent to which enumeration order is defined.

> #1, #2, #4 and #5 still bypass WebIDL

I don't think it's a problem per se as long as they do so consistently.
Comment 4 Joshua Bell 2013-01-09 17:31:27 UTC
(In reply to comment #3)
> Structured cloning is defined to work on ES values, not on WebIDL values
> last I checked.  If it doesn't say that clearly, it really should.  When it
> says Array it means it the way it's meant in ECMA-262: things whose internal
> [[Class]] is Array.
> 
> I would probably be OK with IndexedDB clearly defining that it's working on
> ES values, and really has nothing to do with WebIDL.  It's mixing the two
> that's a problem, since getting a WebIDL anything out of an ES value
> involves type-coercion in various ways.

That seems like the best approach (and what I was leading towards in my #2).
 
> > 3) getters - the structured clone algorithm is similarly underdefined here, 
> 
> How so?  It seems quite clearly defined. 

My bad. I was assuming that "the goal" was to define spec interaction with scripts in terms of WebIDL. 

> > 6, 7) comparison - I believe all implementations act AS IF they have
> > performed a structured clone on the key value after validity is asserted and
> > the clone is used rather than the original object for all further operations.
> 
> OK.  If that's how it works and if the spec explicitly defines the exact
> point in time when the structured clones happen, that would resolve issues 6
> and 7. 

So, in summary:

* Change section 3.1.3 to describe keys in terms of ECMA-262 not WebIDL, borrowing text from HTML structured clone algorithm where appropriate. 3.1.4 should be updated as well.

* Change operations that take explicit key values to state that they act "as if" a structured clone of the key value is made, and specify ordering. E.g. for IDBFactory.cmp(key1, key2), specify that key1 is effectively cloned before key2, and for IDBObjectStore.put(value, key) the clone is done when the method is called, not when the asynchronous task is executed.

(IIRC we put in explicit text in add/put/update about cloning the value to ensure that the clone was made at call time, for similar reasons.)

None of these should impact implementations as they should effectively be doing this under the hood already, although test for these edges will probably turn up some inconsistencies.
Comment 5 Boris Zbarsky 2013-01-09 17:45:17 UTC
That plan sounds good to me.  Thanks!
Comment 6 Joshua Bell 2013-03-01 18:42:14 UTC
This should be resolved in https://dvcs.w3.org/hg/IndexedDB/rev/f74a5b0a703c

Please take a look at the changes.
Comment 7 Boris Zbarsky 2013-04-02 17:40:56 UTC
It's much better, thanks.  The only remaining issue I see is that the "implicit copying step" note doesn't make sense; in particular the clause that putatively has "side effects" as subject doesn't actually have a verb.  that is, it's not clear what this note is saying about "side effects".
Comment 8 Joshua Bell 2013-04-02 18:00:15 UTC
(In reply to comment #7)
> It's much better, thanks.  The only remaining issue I see is that the
> "implicit copying step" note doesn't make sense; in particular the clause
> that putatively has "side effects" as subject doesn't actually have a verb. 
> that is, it's not clear what this note is saying about "side effects".

Thanks. Yes, my text was pretty unintelligible. I've reworded the note as:

This implicit copying step ensures that key values do not change after the operation begins, due to side effects such as JavaScript [ECMA-262] getters, setters and type conversion functions including toString() and valueOf().
Comment 9 Boris Zbarsky 2013-04-02 18:46:26 UTC
Great, that's much better!