Re: WebIDL usage for Algorithms

On Tue, Mar 11, 2014 at 4:11 PM, Ryan Sleevi <sleevi@google.com> wrote:

> Mark,
>
> I do not believe we've agreed upon asynchronous for *ALL* WebIDL errors,
> which I have also previously communicated. It certainly is not what WebKit
> or Blink have implemented, and certainly not how I proposed to you that we
> resolve this.
>
> The *only* thing that needs to be asynchronous is Algorithm Normalization.
>
> As it reads, I object to the notion that all WebIDL errors are
> asynchronous, and had the WG been given sufficient time to review the spec,
> would have raised this as a LC blocking issue. This is certainly something
> that must block CR, and ideally something that would not have made it to LC.
>
> I agree with Richard that the current text highlights that we've
> functionally done the equivalent of "#define ONE -2", in that our spec does
> not match expectations - of implementors or developers who, in every other
> WebIDL using spec they read, know precisely what to expect.
>
> Richard,
>
> To make actual forward progress on this, do you agree that we can use the
> "any"/"object" type within WebIDL, combined with a strict *synchronous*
> cloning (for lack of a better term, although NOT to be confused with
> structured clone necessarily) of the object prior to returning the promise,
> and then all *future* operations happen on that cloned object.
>

This is exactly what I was working on implementing this afternoon.  FWIW,
my internal WebIDL uses "object".

As far as verbs: In discussing this with Boris Zbarsky, who knows WebIDL
far better than me, the verb we were using was "coerce".  Not sure if that
has any wider usage.  The WebIDL spec uses "convert", as in, "An ECMAScript
value V is converted to an IDL dictionary type value...".
<http://www.w3.org/TR/WebIDL/#es-dictionary>

--Richard



> The goal here is to avoid any manipulation of the object at the point in
> which control is returned to the caller, much in the way that we wish to
> avoid manipulation to ArrayBuffers.
>
> If so, I will attempt to draft text to this effect.
>
>
> On Tue, Mar 11, 2014 at 8:36 AM, Richard Barnes <rlb@ipv.sx> wrote:
>
>> I would really like to avoid re-writing WebIDL code for this spec.  That
>> seems like an indication that we've gotten our layering wrong.
>>
>> I'm willing to allow WebIDL binding errors to be synchronous.  If we use
>> something like "any" or "object" for Algorithm, then you're really doing
>> something wrong if you cause an error at that level.
>>
>> --Richard
>>
>>
>> On Tue, Mar 11, 2014 at 3:07 PM, Mark Watson <watsonm@netflix.com> wrote:
>>
>>>
>>>
>>>
>>> On Mon, Mar 10, 2014 at 7:28 PM, Ryan Sleevi <sleevi@google.com> wrote:
>>>
>>>> I haven't reviewed the proposed language, but as expressed to you in
>>>> email, I agree it needs to change from Algorithm into a more opaque type to
>>>> support such an action.
>>>>
>>>> I don't think we can use prose to wipe away a normative reference to
>>>> WebIDL.
>>>>
>>> We can do whatever we like, it's a question of whether we *should*. We
>>> can say "in this specification the symbol '5' is used to represent the the
>>> result of the sum '2+2' if we want to, but we probably shouldn't.
>>>
>>> We have said we want all errors to be asynchronous*. This means that
>>> WebIDL type mapping cannot happen for* any of the method parameters*until we say so. It's not even an option to replace all method parameters
>>> with "any", because even type mapping to IDL "any" can throw an exception.
>>> Replacing just some of the types with "any" doesn't help.
>>>
>>> So, we have to say that implementations of WebCrypto must be compliant
>>> to WebIDL *except* that the procedures specified in Section 4.4.7 of
>>> (WebIDL http://www.w3.org/TR/WebIDL/#es-operations) are delayed until
>>> the asynchronous part. There is actually a possibility of a TypeError here
>>> before we get to argument type mapping - in the identification of the
>>> "this" value - so we might need to be more explicit than presently about
>>> which procedures are deferred to the asynchronous part.
>>>
>>> It does seem that, longer term, it would make sense for WebIDL to
>>> incorporate Promises explicitly, so that the delay of these WebIDL steps to
>>> the asynchronous part is placed on a common footing. Then one could expect
>>> generated bindings to be developed for methods with Promise return types
>>> that do this work in the asynchronous part.
>>>
>>> Furthermore, it does seem that Dictionary inheritance is not as useful
>>> as it could be without proper support for polymorphism. WebIDL could
>>> consider having Dictionary IDL types carry with them a reference to the
>>> original object so that downcasts are possible.
>>>
>>> Note that all of the above points to things WebIDL could do that would -
>>> eventually - lead to better development tool support for what we want to do
>>> in WebCrypto. But none of this discussion changes what we want to achieve
>>> in terms of how our methods are called and how they behave. Implementers
>>> are going to have to support the desired behavior "by hand" until WebIDL
>>> has better support for it.
>>>
>>> ...Mark
>>>
>>> * I'm assuming we do not want to revisit this and say that *some* errors
>>> are synchronous: if even one is synchronous then it might as well be many.
>>>
>>>
>>>
>>>
>>>>  On Mar 10, 2014 7:13 PM, "Mark Watson" <watsonm@netflix.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On Mar 10, 2014, at 6:39 PM, Richard Barnes <rlb@ipv.sx> wrote:
>>>>>
>>>>> So it seems like the editors' inclination (and what's in
>>>>> WebKit/Chrome) is option (1), requiring custom bindings.  I can agree with
>>>>> that, and understand the utility of still having the dictionaries around in
>>>>> that case.
>>>>>
>>>>> It still seems like we need to change the use of Algorithm in the
>>>>> SubtleCrypto methods to prevent binding at that method invocation.
>>>>>
>>>>>
>>>>> We have text that says the type mapping doesn't happen on method
>>>>> invocation but when explicitly stated in the asynchronous part. Do you
>>>>> think we need more ?
>>>>>
>>>>> ...Mark
>>>>>
>>>>>
>>>>> --Richard
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Mar 10, 2014 at 11:50 PM, Mark Watson <watsonm@netflix.com>wrote:
>>>>>
>>>>>> Richard,
>>>>>>
>>>>>> Yes, Ryan and I discussed this and I've proposed a solution in the ED.
>>>>>>
>>>>>> First, since we decided all errors must be asynchronous, we can't
>>>>>> have WebIDL type mapping happening at the usual time. We explicitly note
>>>>>> this and then explicitly specify when to do type mapping. The type mapping
>>>>>> is done in the "normalize to X" algorithm which takes an object and does
>>>>>> WebIDL type mapping to dictionary type X (as well as resolving any aliases,
>>>>>> though we still don't have any of these defined).
>>>>>>
>>>>>> As you say, this mapping discards members not mentioned in X, so
>>>>>> "normalize to Algorithm" leaves you only with name. The individual
>>>>>> algorithm operations call "normalize to Y" where Y is the appropriate
>>>>>> subclass.
>>>>>>
>>>>>> ...Mark
>>>>>>
>>>>>>
>>>>>> On Mon, Mar 10, 2014 at 4:18 PM, Ryan Sleevi <sleevi@google.com>wrote:
>>>>>>
>>>>>>>
>>>>>>> On Mar 10, 2014 4:12 PM, "Richard Barnes" <rlb@ipv.sx> wrote:
>>>>>>> >
>>>>>>> > TL;DR: Usage of Dictionary inheritance for Algorithms is broken.
>>>>>>>  Should use WebIDL object type or callback interfaces instead.
>>>>>>> >
>>>>>>> > BACKGROUND:
>>>>>>> > In the current spec, algorithm identifiers and parameters are
>>>>>>> passed to encrypt(), decrypt(), etc. using dictionaries that inherit from
>>>>>>> Algorithm, which is itself a dictionary type.  For example:
>>>>>>> >   dictionary Algorithm { DOMString name; }
>>>>>>> >   dictionary AesCbcParams : Algorithm { CryptoOperationData iv; }
>>>>>>> >   typedef (Algorithm or DOMString) AlgorithmIdentifier;
>>>>>>> >   Promise<any> encrypt(AlgorithmIdentifier algorithm, ... )
>>>>>>> >
>>>>>>> > PROBLEM:
>>>>>>> > According to the WebIDL spec for handling of dictionary types, the
>>>>>>> implementation is required to create the dictionary object by copying out
>>>>>>> of the source object the fields specified in the type definition.
>>>>>>> > http://www.w3.org/TR/WebIDL/#es-dictionary
>>>>>>> >
>>>>>>>
>>>>>>> Yes, I raised this privately with Mark when discussing things I
>>>>>>> didn't think were LC ready.
>>>>>>>
>>>>>>> > For WebCrypto, this means that polymorphism via inheritance of
>>>>>>> dictionaries doesn't work.  If someone creates a dictionary that matches
>>>>>>> AesCbcParams and passes it to encrypt(), then the WebIDL implementation
>>>>>>> will only store the "name" field, and throw everything else away.  Anything
>>>>>>> using the WebIDL object will not be able to see the "iv" parameter.
>>>>>>> >
>>>>>>>
>>>>>>> Did the wording Mark proposed to handle this not address it?
>>>>>>>
>>>>>>> Note the asynchronous behavior of WebIDL conversions that are
>>>>>>> written.
>>>>>>>
>>>>>>> > As far as implementation: I discovered this bug while trying to
>>>>>>> implement using the Firefox WebIDL implementation, so clearly Firefox has
>>>>>>> this problem :)  It appears that Chromium is internally treating
>>>>>>> "algorithm" arguments as generic objects (not following the WebIDL spec for
>>>>>>> dictionaries), then using a "parseAlgorithm" method to pull out relevant
>>>>>>> information.
>>>>>>> >
>>>>>>>
>>>>>>> Both Blink and WebKit have implemented this with custom bindings.
>>>>>>>
>>>>>>> > PROPOSED RESOLUTION:
>>>>>>> > It seems to me that we have a couple of options here.
>>>>>>> >
>>>>>>> > 1. "typedef object Algorithm" - Don't have the WebIDL handle any
>>>>>>> of the internal structure of Algorithm objects, but instead lay this out in
>>>>>>> the text of the spec.  In this case, there's no inheritance to speak of,
>>>>>>> just lists of what parameters must be present in each use of Algorithm.  We
>>>>>>> would need to specify at what point in the process values are extracted
>>>>>>> from the object.
>>>>>>> >
>>>>>>>
>>>>>>> Or just call it "any"
>>>>>>>
>>>>>>> > 2. "callback interface Algorithm" - This would use inheritance and
>>>>>>> have the expected behavior, since callback interfaces keep around a copy of
>>>>>>> the provided JS object.  However, there might be some weird behaviors if
>>>>>>> people pass in exotic objects, e.g., side-effects or changing values.
>>>>>>>  Also, the spec would need to carefully define exactly when the properties
>>>>>>> are accessed, and all per-spec access to the properties would need to be
>>>>>>> done on the main thread, since it needs to be able to run script.
>>>>>>> >
>>>>>>>
>>>>>>> Strongly opposed to this. Its going to be quite ugly and requires
>>>>>>> painting over all of the ES5/ES6 interactions.
>>>>>>>
>>>>>>> > I have a slight preference for (2).  It continues to delegate most
>>>>>>> of the work to WebIDL, and it seems unlikely that people will do exotic
>>>>>>> enough things to cause trouble, given that the way WebCrypto uses these
>>>>>>> objects is fairly straightforward.
>>>>>>> >
>>>>>>> > --Richard
>>>>>>> >
>>>>>>> >
>>>>>>>
>>>>>>> Considering that the WebIDL spec changed in order to support other
>>>>>>> use cases of ours - notably, the Promise<resolve type> notation - have you
>>>>>>> considered approaching the WebIDL WG with this?
>>>>>>>
>>>>>>> >
>>>>>>> >
>>>>>>> >
>>>>>>> >
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>
>