[bindings] Make XMLHttpRequest.send() use the generated binding code over the custom one.

[bindings] Make XMLHttpRequest.send() use the generated binding code over the custom one.

The specification [1] defines the XMLHttpRequest interface with send method specifications
which accept |ArrayBuffer|, |ArrayBufferView|, |Blob|, |Document|, |String| Or |FormData| argument to be passed as data.

MDN link [2] specifies about the same usage.

Considering the support of Union types by the bindings code generator, using the same and removing the
usage of custom bindings.

BUG=345519
[1] https://xhr.spec.whatwg.org/
[2] https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#send()

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192414

[vivekg, 2015-03-23 11:22:00]

vivekg@chromium.org changed reviewers:
+ haraken@chromium.org, vivekg@chromium.org, yhirano@chromium.org

[vivekg, 2015-03-23 11:22:01]

PTAL, thanks.

[Jens Widell, 2015-03-23 11:41:58]

jl@opera.com changed reviewers:
+ jl@opera.com

[Jens Widell, 2015-03-23 11:41:58]

TR specifications are historical monuments, and not to be taken literally. :-)

A more relevant specification to look at, and link to, in this case would be
https://xhr.spec.whatwg.org/.

[yhirano, 2015-03-23 11:50:41]

Thanks for doing this!

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:826: if (data.isHTMLDocument()) {
This branch will never be taken.

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:842:
InspectorInstrumentation::willSendXMLHttpRequest(executionContext(), url());
Is this change related to the issue?

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.h (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.h:44: class
ArrayBufferOrArrayBufferViewOrBlobOrDocumentOrHTMLDocumentOrStringOrFormData;
Sorry that I don't know much about union, should we have both Doucment and
HTMLDocument here? If we only have Document, is HTMLDocument accepted as
Document?

[vivekg, 2015-03-23 11:54:05]

On 2015/03/23 at 11:41:58, jl wrote:
> TR specifications are historical monuments, and not to be taken literally. :-)
> 
> A more relevant specification to look at, and link to, in this case would be
https://xhr.spec.whatwg.org/.

Thank you, updated the link. :)

[vivekg, 2015-03-23 11:57:36]

Thank you.

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:826: if (data.isHTMLDocument()) {
On 2015/03/23 at 11:50:41, yhirano wrote:
> This branch will never be taken.

Right!

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:842:
InspectorInstrumentation::willSendXMLHttpRequest(executionContext(), url());
On 2015/03/23 at 11:50:41, yhirano wrote:
> Is this change related to the issue?

Yes this is being handled in
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
and without this change the following cases fail in inspector:

inspector/sources/debugger/xhr-breakpoints.html
inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints.html

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.h (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.h:44: class
ArrayBufferOrArrayBufferViewOrBlobOrDocumentOrHTMLDocumentOrStringOrFormData;
On 2015/03/23 at 11:50:41, yhirano wrote:
> Sorry that I don't know much about union, should we have both Doucment and
HTMLDocument here? If we only have Document, is HTMLDocument accepted as
Document?

Yes I think this needs to be fixed in the bindings generator code for Union. May
be haraken@ or jl@ can comment?

[Jens Widell, 2015-03-23 12:03:05]

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.h (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.h:44: class
ArrayBufferOrArrayBufferViewOrBlobOrDocumentOrHTMLDocumentOrStringOrFormData;
On 2015/03/23 11:57:36, vivekg_ wrote:
> On 2015/03/23 at 11:50:41, yhirano wrote:
> > Sorry that I don't know much about union, should we have both Doucment and
> HTMLDocument here? If we only have Document, is HTMLDocument accepted as
> Document?
> 
> Yes I think this needs to be fixed in the bindings generator code for Union.
May
> be haraken@ or jl@ can comment?

If we only have Document, all types of documents would be accepted, including
HTMLDocument. Having HTMLDocument as well is confusing; it will depend on the
order which type an actual HTMLDocument would be identified as, since it's both
a Document and an HTMLDocument.

I think we ought to just drop HTMLDocument here.

[vivekg, 2015-03-23 12:13:42]

On 2015/03/23 at 12:03:05, jl wrote:
>
https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.h (right):
> 
>
https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.h:44: class
ArrayBufferOrArrayBufferViewOrBlobOrDocumentOrHTMLDocumentOrStringOrFormData;
> On 2015/03/23 11:57:36, vivekg_ wrote:
> > On 2015/03/23 at 11:50:41, yhirano wrote:
> > > Sorry that I don't know much about union, should we have both Doucment and
> > HTMLDocument here? If we only have Document, is HTMLDocument accepted as
> > Document?
> > 
> > Yes I think this needs to be fixed in the bindings generator code for Union.
May
> > be haraken@ or jl@ can comment?
> 
> If we only have Document, all types of documents would be accepted, including
HTMLDocument. Having HTMLDocument as well is confusing; it will depend on the
order which type an actual HTMLDocument would be identified as, since it's both
a Document and an HTMLDocument.
> 
> I think we ought to just drop HTMLDocument here.

Oops, I did create a confusion to all... Sorry, cleared the HTMLDocument.

[yhirano, 2015-03-23 12:44:34]

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:842:
InspectorInstrumentation::willSendXMLHttpRequest(executionContext(), url());
On 2015/03/23 11:57:36, vivekg_ wrote:
> On 2015/03/23 at 11:50:41, yhirano wrote:
> > Is this change related to the issue?
> 
> Yes this is being handled in
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> and without this change the following cases fail in inspector:
> 
> inspector/sources/debugger/xhr-breakpoints.html
> inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints.html

Thanks, I see.

https://codereview.chromium.org/1030613002/diff/40001/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/1030613002/diff/40001/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:801: send(exceptionState);
InspectorInstrumentation::willSendXMLHttpRequest will be called again.

[vivekg, 2015-03-23 12:49:53]

On 2015/03/23 at 12:44:34, yhirano wrote:
>
https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):
> 
>
https://codereview.chromium.org/1030613002/diff/20001/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.cpp:842:
InspectorInstrumentation::willSendXMLHttpRequest(executionContext(), url());
> On 2015/03/23 11:57:36, vivekg_ wrote:
> > On 2015/03/23 at 11:50:41, yhirano wrote:
> > > Is this change related to the issue?
> > 
> > Yes this is being handled in
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > and without this change the following cases fail in inspector:
> > 
> > inspector/sources/debugger/xhr-breakpoints.html
> > inspector/sources/debugger/frameworks-dom-xhr-event-breakpoints.html
> 
> Thanks, I see.
> 
>
https://codereview.chromium.org/1030613002/diff/40001/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):
> 
>
https://codereview.chromium.org/1030613002/diff/40001/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.cpp:801: send(exceptionState);
> InspectorInstrumentation::willSendXMLHttpRequest will be called again.

Fixed the repeat call. Thanks.

[tyoshino (SeeGerritForStatus), 2015-03-23 13:08:20]

lgtm

How about adding a test to check string conversion like
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
for WebSockets?

[vivekg, 2015-03-23 14:02:38]

On 2015/03/23 at 13:08:20, tyoshino wrote:
> lgtm
> 
> How about adding a test to check string conversion like
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
for WebSockets?

Thank you. Added a new test, PTAL.

[yhirano, 2015-03-23 14:35:16]

On 2015/03/23 14:02:38, vivekg_ wrote:
> On 2015/03/23 at 13:08:20, tyoshino wrote:
> > lgtm
> > 
> > How about adding a test to check string conversion like
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> for WebSockets?
> 
> Thank you. Added a new test, PTAL.

Can you write tests using js-test or testharness, as recommended in [1] and [2]?

1: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yt-GYRNM58M
2:
https://docs.google.com/a/chromium.org/document/d/1Yl4SnTLBWmY1O99_BTtQvuoffP...

[vivekg, 2015-03-24 04:11:03]

On 2015/03/23 at 14:35:16, yhirano wrote:
> On 2015/03/23 14:02:38, vivekg_ wrote:
> > On 2015/03/23 at 13:08:20, tyoshino wrote:
> > > lgtm
> > > 
> > > How about adding a test to check string conversion like
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > for WebSockets?
> > 
> > Thank you. Added a new test, PTAL.
> 
> Can you write tests using js-test or testharness, as recommended in [1] and
[2]?
> 
> 1:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yt-GYRNM58M
> 2:
https://docs.google.com/a/chromium.org/document/d/1Yl4SnTLBWmY1O99_BTtQvuoffP...

Thank you. Done, PTAL.

[yhirano, 2015-03-24 04:19:25]

lgtm

[vivekg, 2015-03-24 04:23:44]

The CQ bit was checked by vivekg@chromium.org

[vivekg, 2015-03-24 04:23:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org
Link to the patchset: https://codereview.chromium.org/1030613002/#ps100001
(title: "Patch for landing")

[commit-bot: I haz the power, 2015-03-24 04:24:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1030613002/100001

[haraken, 2015-03-24 04:56:31]

LGTM

[tyoshino (SeeGerritForStatus), 2015-03-24 05:14:41]

lgtm

[commit-bot: I haz the power, 2015-03-24 05:47:07]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192414