Bug 171589 – update FormData to latest spec webidl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Ben Kelly]

This bug is to expose FormData on worker threads.  Currently its only exposed on Window in WebKit, AFAICT.  XHR is exposed in workers, so it would be able to take advantage of this.  Also, I'd like to integrate it with Fetch API in bug 161190 and bug 171581 which also exposed on workers.

https://xhr.spec.whatwg.org/#formdata

[Ben Kelly]

Actually, the webkit DOMFormData.idl is a bit out of date with the spec.  Looks like the following is missing in addition to the Exposed(Worker):

  void delete(USVString name);
  FormDataEntryValue? get(USVString name);
  sequence<FormDataEntryValue> getAll(USVString name);
  boolean has(USVString name);
  void set(USVString name, USVString value);
  void set(USVString name, Blob blobValue, optional USVString filename);
  iterable<USVString, FormDataEntryValue>;

Let me see if I can just bring the impl update to spec here.

[Rob Buis]

Looks like this was implemented in https://bugs.webkit.org/show_bug.cgi?id=176674 (worker) and https://bugs.webkit.org/show_bug.cgi?id=176659 (the rest).

[Rob Buis]

Created attachment 410122 [details]
Patch

[Rob Buis]

Created attachment 410125 [details]
Patch

[Rob Buis]

Created attachment 410126 [details]
Patch

[Rob Buis]

Created attachment 410131 [details]
Patch

[Sam Weinig]

Is the optional argument not compiling?

[Rob Buis]

(In reply to Sam Weinig from comment #7)
> Is the optional argument not compiling?

Right, I got a "not supported" error from the IDL processing when I tried.

[Chris Dumez]

Comment on attachment 410131 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410131&action=review

> Source/WebCore/html/DOMFormData.idl:40
> +    // FIXME: should be optional HTMLFormElement.

I think we should make this work. This change makes our IDL look worse.
I am actually very surprised it does not work.

[Sam Weinig]

(In reply to Rob Buis from comment #8)
> (In reply to Sam Weinig from comment #7)
> > Is the optional argument not compiling?
> 
> Right, I got a "not supported" error from the IDL processing when I tried.

hm, interesting. If you can try again, and give me the full output, I can probably point to what's wrong.

[Chris Dumez]

(In reply to Sam Weinig from comment #10)
> (In reply to Rob Buis from comment #8)
> > (In reply to Sam Weinig from comment #7)
> > > Is the optional argument not compiling?
> > 
> > Right, I got a "not supported" error from the IDL processing when I tried.
> 
> hm, interesting. If you can try again, and give me the full output, I can
> probably point to what's wrong.

Optional arguments of non-nullable wrapper types are not supported () at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 118.
$VAR1 = ' at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 6196.
	CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70), IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8), "create", "    ") called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 7765
	CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70), "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 7696
	CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70), "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", IDLInterface=HASH(0x7f96e289ffa8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 4498
	CodeGeneratorJS::GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118), IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500), ARRAY(0x7f96e28a0ff8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 177
	CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118), IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500), ARRAY(0x7f96e28a0ff8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGenerator.pm line 256
	CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0), IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGenerator.pm line 177
	CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0), IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"...) called at WebCore/bindings/scripts/generate-bindings.pl line 132
	main::generateBindings("WebCore/html/DOMFormData.idl") called at WebCore/bindings/scripts/generate-bindings.pl line 82
';
make[1]: *** [JSDOMFormData.h] Error 255
Command /bin/sh failed with exit code 2
** BUILD FAILED **

[Sam Weinig]

(In reply to Chris Dumez from comment #11)
> (In reply to Sam Weinig from comment #10)
> > (In reply to Rob Buis from comment #8)
> > > (In reply to Sam Weinig from comment #7)
> > > > Is the optional argument not compiling?
> > > 
> > > Right, I got a "not supported" error from the IDL processing when I tried.
> > 
> > hm, interesting. If you can try again, and give me the full output, I can
> > probably point to what's wrong.
> 
> Optional arguments of non-nullable wrapper types are not supported () at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 118.
> $VAR1 = ' at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 6196.
> 	CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70),
> IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8),
> "create", "    ") called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 7765
> 	CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70),
> "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400))
> called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 7696
> 	CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70),
> "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> IDLInterface=HASH(0x7f96e289ffa8)) called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 4498
> 	CodeGeneratorJS::
> GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118),
> IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500),
> ARRAY(0x7f96e28a0ff8)) called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGeneratorJS.pm line 177
> 	CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118),
> IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS
> ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500),
> ARRAY(0x7f96e28a0ff8)) called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGenerator.pm line 256
> 	CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0),
> IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at
> /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> CodeGenerator.pm line 177
> 	CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0),
> IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> ENABLE_ACCESSIBILIT"...) called at
> WebCore/bindings/scripts/generate-bindings.pl line 132
> 	main::generateBindings("WebCore/html/DOMFormData.idl") called at
> WebCore/bindings/scripts/generate-bindings.pl line 82
> ';
> make[1]: *** [JSDOMFormData.h] Error 255
> Command /bin/sh failed with exit code 2
> ** BUILD FAILED **

Oh fun. Wonder why I (or perhaps someone else) did that. Probably about disambiguating between null and optional, but I don't think that should matter here. Will take a look later on this afternoon.

[Chris Dumez]

(In reply to Sam Weinig from comment #12)
> (In reply to Chris Dumez from comment #11)
> > (In reply to Sam Weinig from comment #10)
> > > (In reply to Rob Buis from comment #8)
> > > > (In reply to Sam Weinig from comment #7)
> > > > > Is the optional argument not compiling?
> > > > 
> > > > Right, I got a "not supported" error from the IDL processing when I tried.
> > > 
> > > hm, interesting. If you can try again, and give me the full output, I can
> > > probably point to what's wrong.
> > 
> > Optional arguments of non-nullable wrapper types are not supported () at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 118.
> > $VAR1 = ' at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 6196.
> > 	CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70),
> > IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8),
> > "create", "    ") called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 7765
> > 	CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70),
> > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> > IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400))
> > called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 7696
> > 	CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70),
> > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> > IDLInterface=HASH(0x7f96e289ffa8)) called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 4498
> > 	CodeGeneratorJS::
> > GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118),
> > IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500),
> > ARRAY(0x7f96e28a0ff8)) called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGeneratorJS.pm line 177
> > 	CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118),
> > IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS
> > ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500),
> > ARRAY(0x7f96e28a0ff8)) called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGenerator.pm line 256
> > 	CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0),
> > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> > ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at
> > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > CodeGenerator.pm line 177
> > 	CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0),
> > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> > ENABLE_ACCESSIBILIT"...) called at
> > WebCore/bindings/scripts/generate-bindings.pl line 132
> > 	main::generateBindings("WebCore/html/DOMFormData.idl") called at
> > WebCore/bindings/scripts/generate-bindings.pl line 82
> > ';
> > make[1]: *** [JSDOMFormData.h] Error 255
> > Command /bin/sh failed with exit code 2
> > ** BUILD FAILED **
> 
> Oh fun. Wonder why I (or perhaps someone else) did that. Probably about
> disambiguating between null and optional, but I don't think that should
> matter here. Will take a look later on this afternoon.

I think the issue is that our generated bindings always call the implementation function with ALL parameters (even the optional ones that were omitted). As a result, we need a value to pass the implementation. When a parameter is an interface and nullable, we pass null to the implementation. However, when the interface is not nullable, the bindings currently don't know what to pass.

I guess there could be at least 2 ways to fix it:
1. Pass null to the implementation when the parameter is omitted and it is an interface type (even if not nullable). We'd have to make sure we're properly throwing when the JS passes null though.
2. Omit the parameters that were omitted in JS when calling the implementation function

[Sam Weinig]

(In reply to Chris Dumez from comment #13)
> (In reply to Sam Weinig from comment #12)
> > (In reply to Chris Dumez from comment #11)
> > > (In reply to Sam Weinig from comment #10)
> > > > (In reply to Rob Buis from comment #8)
> > > > > (In reply to Sam Weinig from comment #7)
> > > > > > Is the optional argument not compiling?
> > > > > 
> > > > > Right, I got a "not supported" error from the IDL processing when I tried.
> > > > 
> > > > hm, interesting. If you can try again, and give me the full output, I can
> > > > probably point to what's wrong.
> > > 
> > > Optional arguments of non-nullable wrapper types are not supported () at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 118.
> > > $VAR1 = ' at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 6196.
> > > 	CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70),
> > > IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8),
> > > "create", "    ") called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 7765
> > > 	CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70),
> > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> > > IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400))
> > > called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 7696
> > > 	CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70),
> > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData",
> > > IDLInterface=HASH(0x7f96e289ffa8)) called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 4498
> > > 	CodeGeneratorJS::
> > > GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118),
> > > IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500),
> > > ARRAY(0x7f96e28a0ff8)) called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGeneratorJS.pm line 177
> > > 	CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118),
> > > IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS
> > > ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500),
> > > ARRAY(0x7f96e28a0ff8)) called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGenerator.pm line 256
> > > 	CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0),
> > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> > > ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at
> > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/
> > > CodeGenerator.pm line 177
> > > 	CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0),
> > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY
> > > ENABLE_ACCESSIBILIT"...) called at
> > > WebCore/bindings/scripts/generate-bindings.pl line 132
> > > 	main::generateBindings("WebCore/html/DOMFormData.idl") called at
> > > WebCore/bindings/scripts/generate-bindings.pl line 82
> > > ';
> > > make[1]: *** [JSDOMFormData.h] Error 255
> > > Command /bin/sh failed with exit code 2
> > > ** BUILD FAILED **
> > 
> > Oh fun. Wonder why I (or perhaps someone else) did that. Probably about
> > disambiguating between null and optional, but I don't think that should
> > matter here. Will take a look later on this afternoon.
> 
> I think the issue is that our generated bindings always call the
> implementation function with ALL parameters (even the optional ones that
> were omitted). As a result, we need a value to pass the implementation. When
> a parameter is an interface and nullable, we pass null to the
> implementation. However, when the interface is not nullable, the bindings
> currently don't know what to pass.
> 
> I guess there could be at least 2 ways to fix it:
> 1. Pass null to the implementation when the parameter is omitted and it is
> an interface type (even if not nullable). We'd have to make sure we're
> properly throwing when the JS passes null though.
> 2. Omit the parameters that were omitted in JS when calling the
> implementation function

#1 is what we will likely want to do. For non-wrapper types, what we would do here is have the implementation use Optional<type> as the parameter. This does lead to the potential for a problem if you have a case like:

undefined foo(optional long a);
undefined foo(long? a);

We wouldn't be able to distinguish between the two overloads in the implementation (I am not sure that is actually valid WebIDL, but you get the idea) as they would both be:

void foo(Optional<int>);


For wrapper types, we don't use Optional<>, but use nullable pointers for nullable types, and references for non-nullable types. So if you have:

undefined foo(optional Node a);
undefined foo(Node? a);

you would again would have the same situation, where both the implementation functions would have the same signature:

void foo(Node*).


But, given we already live with the ambiguity with the non-wrapper type case, I am not sure why we didn't implement the wrapper case the same way (but using pointers instead of Optional).

Seems like we should just treat optional Node and Node? the same at the implementation level (but of course keep the bindings throwing for the optional case when you pass null).

One day, we may have to make things more explicit, and have the implementations use specific IDLOptional<> / IDLNullable<> types, but lets wait for that to actually crop up.

[Sam Weinig]

Ok, blocking bug is now in. You should be able to use:

  constructor(optional HTMLFormElement form);

[Chris Dumez]

(In reply to Sam Weinig from comment #15)
> Ok, blocking bug is now in. You should be able to use:
> 
>   constructor(optional HTMLFormElement form);

Yay, thanks Sam.

[Rob Buis]

Created attachment 410303 [details]
Patch

[Rob Buis]

(In reply to Chris Dumez from comment #16)
> (In reply to Sam Weinig from comment #15)
> > Ok, blocking bug is now in. You should be able to use:
> > 
> >   constructor(optional HTMLFormElement form);
> 
> Yay, thanks Sam.

Yes, thanks Sam! The patch is now trivial.

[EWS]

Committed r267866: <https://trac.webkit.org/changeset/267866>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410303 [details].

[Radar WebKit Bug Importer]

<rdar://problem/69870343>