[bindings] Remove custom binding usage from XMLHttpRequest.open()

[bindings] Remove custom binding usage from XMLHttpRequest.open()

Modified the XHR.open(...) signatures to match the auto generated code and redirected these
using the KURL to real open method.

BUG=345519
R=haraken@chromium.org, yhirano@chromium.org

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

[vivekg, 2015-03-24 08:39:08]

vivekg@chromium.org changed reviewers:
+ vivekg@chromium.org

[vivekg, 2015-03-24 08:39:09]

PTAL, thanks.

[haraken, 2015-03-24 08:47:56]

LGTM

[yhirano, 2015-03-24 09:10:47]

https://codereview.chromium.org/1034453002/diff/1/Source/core/xmlhttprequest/...
File Source/core/xmlhttprequest/XMLHttpRequest.h (right):

https://codereview.chromium.org/1034453002/diff/1/Source/core/xmlhttprequest/...
Source/core/xmlhttprequest/XMLHttpRequest.h:115: void open(const AtomicString&
method, const AtomicString& url, ExceptionState&);
Why do you use AtomicString for URLs?
AtomicString has a static table, so using it for URLs doesn't look good.

[vivekg, 2015-03-24 09:20:32]

Patchset #2 (id:20001) has been deleted

[vivekg, 2015-03-24 09:26:23]

On 2015/03/24 at 09:10:47, yhirano wrote:
>
https://codereview.chromium.org/1034453002/diff/1/Source/core/xmlhttprequest/...
> File Source/core/xmlhttprequest/XMLHttpRequest.h (right):
> 
>
https://codereview.chromium.org/1034453002/diff/1/Source/core/xmlhttprequest/...
> Source/core/xmlhttprequest/XMLHttpRequest.h:115: void open(const AtomicString&
method, const AtomicString& url, ExceptionState&);
> Why do you use AtomicString for URLs?
> AtomicString has a static table, so using it for URLs doesn't look good.

Done. AtomicString => String for url. Thank you. PTAL.

[vivekg, 2015-03-24 10:16:52]

Patchset #3 (id:60001) has been deleted

[vivekg, 2015-03-24 10:17:17]

Patchset #2 (id:40001) has been deleted

[yhirano, 2015-03-24 10:49:39]

https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.idl (right):

https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.idl:61: [RaisesException] void
open(DOMString method, DOMString url, optional boolean async,
[TreatUndefinedAs=NullString] optional DOMString? user,
[TreatUndefinedAs=NullString] optional DOMString? password);
TreatNullAs=NullString needed?
Can we have tests?

[vivekg, 2015-03-24 10:53:24]

On 2015/03/24 at 10:49:39, yhirano wrote:
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.idl (right):
> 
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.idl:61: [RaisesException] void
open(DOMString method, DOMString url, optional boolean async,
[TreatUndefinedAs=NullString] optional DOMString? user,
[TreatUndefinedAs=NullString] optional DOMString? password);
> TreatNullAs=NullString needed?
> Can we have tests?

There are already tests to cover 'undefined' cases and these have failed in the
first patch set.

http/tests/inspector/network/network-xhr-async-response-type-blob.html
inspector/console/console-format-es6.html
http/tests/inspector/network/network-xhr-post-payload.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/network/async-xhr-json-mime-type.html
http/tests/inspector/network/network-empty-xhr.html
http/tests/inspector/network/network-cyrillic-xhr.html
http/tests/inspector/network/network-xhr-redirect-body.html
svg/custom/massive-coordinates.svg
http/tests/inspector/network/network-xhr-redirect-method.html
http/tests/inspector/network/network-content-replacement-xhr.html
http/tests/inspector/network/network-xhr-async.html
http/tests/inspector/network/network-xhr-sync.html
http/tests/inspector/network/network-disable-cache-xhrs.html

[Jens Widell, 2015-03-24 13:44:25]

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

[Jens Widell, 2015-03-24 13:44:26]

https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
File Source/core/xmlhttprequest/XMLHttpRequest.idl (right):

https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.idl:61: [RaisesException] void
open(DOMString method, DOMString url, optional boolean async,
[TreatUndefinedAs=NullString] optional DOMString? user,
[TreatUndefinedAs=NullString] optional DOMString? password);
On 2015/03/24 10:49:39, yhirano wrote:
> TreatNullAs=NullString needed?
> Can we have tests?

'DOMString?', used here, has the same effect as [TreatNullAs=NullString].

[yhirano, 2015-03-24 14:04:14]

On 2015/03/24 13:44:26, Jens Widell wrote:
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.idl (right):
> 
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.idl:61: [RaisesException] void
> open(DOMString method, DOMString url, optional boolean async,
> [TreatUndefinedAs=NullString] optional DOMString? user,
> [TreatUndefinedAs=NullString] optional DOMString? password);
> On 2015/03/24 10:49:39, yhirano wrote:
> > TreatNullAs=NullString needed?
> > Can we have tests?
> 
> 'DOMString?', used here, has the same effect as [TreatNullAs=NullString].

Thanks, I see.

[yhirano, 2015-03-24 14:07:22]

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

https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
Source/core/xmlhttprequest/XMLHttpRequest.cpp:774: if (!user.isEmpty())
Can you tell me why you use isEmpty, not isNull? The spec says "If the username
argument is not null ...".

[vivekg, 2015-03-24 14:54:00]

On 2015/03/24 at 14:07:22, yhirano wrote:
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> File Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):
> 
>
https://codereview.chromium.org/1034453002/diff/40002/Source/core/xmlhttprequ...
> Source/core/xmlhttprequest/XMLHttpRequest.cpp:774: if (!user.isEmpty())
> Can you tell me why you use isEmpty, not isNull? The spec says "If the
username argument is not null ...".

Changed the check from isEmpty => isNull. PTAL, thank you.

[yhirano, 2015-03-24 16:12:29]

lgtm

[vivekg, 2015-03-24 17:41:12]

The CQ bit was checked by vivekg@chromium.org

[vivekg, 2015-03-24 17:41:12]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1034453002/#ps90001
(title: "isEmpty => isNull")

[commit-bot: I haz the power, 2015-03-24 17:41:22]

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

[commit-bot: I haz the power, 2015-03-24 19:52:43]

Committed patchset #3 (id:90001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192466