Issue 209933002: Remove custom FormData.append() bindings code.

Issue 209933002: Remove custom FormData.append() bindings code. (Closed)

Created:
6 years, 9 months ago by sof

Modified:
6 years, 9 months ago

Reviewers:
haraken, arv (Not doing code reviews)

CC:
blink-reviews, kojih, arv+blink, jsbell+bindings_chromium.org, kouhei+bindings_chromium.org, abarth-chromium, marja+watch_chromium.org, dglazkov+blink, adamk+blink_chromium.org, Nate Chapin, watchdog-blink-watchlist_google.com, Inactive

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Visibility:
Public.

More Reviews

Description

Remove custom FormData.append() bindings code. The overload resolution needed for the two FormData.append() methods can be handled directly by generated code, so drop the custom code we had in place for this API. Along with it, handle the conversion of append()'s DOMString arguments per WebIDL (as it is now), removing the special treatment of null and undefined arguments. This also aligns with how others (Gecko, Trident) handle these arguments. R=haraken@chromium.org BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169841

Patch Set 1 #

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+3 lines, -77 lines)			Patch
M	LayoutTests/http/tests/local/formdata/send-form-data-with-null-string-expected.txt	View	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/bindings/bindings.gypi	View	1 chunk	+0 lines, -1 line	0 comments	Download
D	Source/bindings/v8/custom/V8FormDataCustom.cpp	View	1 chunk	+0 lines, -70 lines	0 comments	Download
M	Source/core/html/FormData.idl	View	1 chunk	+2 lines, -5 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

haraken

LGTM. I'm super excited to see custom bindings removed :)

6 years, 9 months ago (2014-03-24 11:17:38 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/209933002/1

6 years, 9 months ago (2014-03-24 11:31:09 UTC) #4

Nils Barth (inactive)

On 2014/03/24 11:17:38, haraken wrote: > LGTM. I'm super excited to see custom bindings removed ...

6 years, 9 months ago (2014-03-25 02:37:42 UTC) #6

sof

6 years, 9 months ago (2014-03-25 06:23:20 UTC) #7

Message was sent while issue was closed.

On 2014/03/25 02:37:42, Nils Barth wrote:
> On 2014/03/24 11:17:38, haraken wrote:
> > LGTM. I'm super excited to see custom bindings removed :)
> 
> This CL is a good example of why implementing the correct
> overload resolution algorithm is worth doing:
> that's a big cause of custom bindings.
> Bug is:
> http://crbug.com/293561

+1. If needed, a useful stopping place on the way to a complete implementation
of that algorithm would be better support for union types in arguments.

(Remembering to update custom code when WebIDL type conversions change is
another issue with them; e.g. the DOMString handling here was from an older
edition of the spec, afaict.)

Expand Messages | Collapse Messages