Introduce XMLHttpRequestEventTarget IDL interface

Introduce XMLHttpRequestEventTarget IDL interface

Introduce XMLHttpRequestEventTarget IDL interface to match the latest specification:
- http://xhr.spec.whatwg.org/#xmlhttprequesteventtarget

It avoids duplication of the event handlers between XMLHttpRequest and
XMLHttpRequestUpload.

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

[arv (Not doing code reviews), 2013-08-15 14:08:56]

LGTM

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.idl (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.idl:2: * Copyright (C) 2008, 2010
Apple Inc. All rights reserved.
This header seems a bit random.

[arv (Not doing code reviews), 2013-08-15 14:10:04]

On 2013/08/15 14:08:56, arv wrote:
> LGTM

Actually, we need to add tests too. (I got too carried away by the awesomeness.)

[haraken, 2013-08-15 14:10:28]

LGTM

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.h (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.h:52: EventTargetData
m_eventTargetData;

Nit: Maybe this member should be private.

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.idl (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.idl:30: // event handler attributes

Nit: Remove this comment.

[do-not-use, 2013-08-15 14:11:20]

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.idl (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.idl:2: * Copyright (C) 2008, 2010
Apple Inc. All rights reserved.
On 2013/08/15 14:08:57, arv wrote:
> This header seems a bit random.

Since I imported the code from XMLHttpRequest.idl, I also imported its
copyright. Isn't this the right thing to do?

It is certainly not random :)

[arv (Not doing code reviews), 2013-08-15 14:23:46]

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.idl (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.idl:2: * Copyright (C) 2008, 2010
Apple Inc. All rights reserved.
On 2013/08/15 14:11:20, Christophe Dumez wrote:
> On 2013/08/15 14:08:57, arv wrote:
> > This header seems a bit random.
> 
> Since I imported the code from XMLHttpRequest.idl, I also imported its
> copyright. Isn't this the right thing to do?
> 
> It is certainly not random :)

Ah, that makes sense... I wish git would have picked that file for the
comparison. It would have made that clear.

[do-not-use, 2013-08-15 14:30:26]

On 2013/08/15 14:10:04, arv wrote:
> On 2013/08/15 14:08:56, arv wrote:
> > LGTM
> 
> Actually, we need to add tests too. (I got too carried away by the
awesomeness.)

I thought about the test. So we basically want to check that XMLHttpRequest's
parent is XMLHttpRequestTarget, knowing that XMLHttpRequestTarget is a
NoInterfaceObject.

Would such checks be acceptable?
shouldBeEqualToString('XMLHttpRequest.prototype.__proto__.constructor.name',
'XMLHttpRequestTarget');
shouldBe('XMLHttpRequest.prototype.__proto__.__proto__',
'EventTarget.prototype');

Or do you know of a nicer way? (or additional checks you would want).

[do-not-use, 2013-08-15 14:47:39]

Fixed the nits and added a test. Please let me know if the test is OK or if you
have a better proposal.

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.h (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.h:52: EventTargetData
m_eventTargetData;
On 2013/08/15 14:10:28, haraken wrote:
> 
> Nit: Maybe this member should be private.

Done.

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
File Source/core/xml/XMLHttpRequestEventTarget.idl (right):

https://codereview.chromium.org/23243002/diff/1/Source/core/xml/XMLHttpReques...
Source/core/xml/XMLHttpRequestEventTarget.idl:30: // event handler attributes
On 2013/08/15 14:10:28, haraken wrote:
> 
> Nit: Remove this comment.

Done.

[arv (Not doing code reviews), 2013-08-15 15:02:49]

LGTM

https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
File LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html
(right):

https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html:19:
shouldBe("XMLHttpRequestUpload.prototype.__proto__.__proto__",
"EventTarget.prototype");
Also:

function className(obj) {
  return Object.prototype.toString.call(obj).slice(8, -1);
}

shouldBeEqualToString("className(XMLHttpRequest.prototype.__proto__",
"XMLHttpRequestEventTargetPrototype");

This will be a FAIL because we incorrectly set the [[Class]] to
"XMLHttpRequestEventTarget"

[do-not-use, 2013-08-15 15:33:45]

https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
File LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html
(right):

https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html:19:
shouldBe("XMLHttpRequestUpload.prototype.__proto__.__proto__",
"EventTarget.prototype");
On 2013/08/15 15:02:50, arv wrote:
> Also:
> 
> function className(obj) {
>   return Object.prototype.toString.call(obj).slice(8, -1);
> }
> 
> shouldBeEqualToString("className(XMLHttpRequest.prototype.__proto__",
> "XMLHttpRequestEventTargetPrototype");
> 
> This will be a FAIL because we incorrectly set the [[Class]] to
> "XMLHttpRequestEventTarget"

I added the check. toString() actually returns Object. iirc this is not easily
fixable because there is no API in v8 to set the class name of the prototype
template, right? (I mean no ObjectTemplate::SetClassName()).

[do-not-use, 2013-08-15 15:34:58]

+jochen

[arv (Not doing code reviews), 2013-08-15 17:00:24]

On 2013/08/15 15:33:45, Christophe Dumez wrote:
>
https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
> File LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html
> (right):
> 
>
https://codereview.chromium.org/23243002/diff/12001/LayoutTests/http/tests/xm...
> LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-inheritance.html:19:
> shouldBe("XMLHttpRequestUpload.prototype.__proto__.__proto__",
> "EventTarget.prototype");
> On 2013/08/15 15:02:50, arv wrote:
> > Also:
> > 
> > function className(obj) {
> >   return Object.prototype.toString.call(obj).slice(8, -1);
> > }
> > 
> > shouldBeEqualToString("className(XMLHttpRequest.prototype.__proto__",
> > "XMLHttpRequestEventTargetPrototype");
> > 
> > This will be a FAIL because we incorrectly set the [[Class]] to
> > "XMLHttpRequestEventTarget"
> 
> I added the check. toString() actually returns Object. iirc this is not easily
> fixable because there is no API in v8 to set the class name of the prototype
> template, right? (I mean no ObjectTemplate::SetClassName()).

https://code.google.com/p/v8/issues/detail?id=2180

[do-not-use, 2013-08-16 07:06:30]

Could an API owner please take a look?

[jochen (gone - plz use gerrit), 2013-08-16 09:15:03]

lgtm

[commit-bot: I haz the power, 2013-08-16 09:33:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23243002/24001

[commit-bot: I haz the power, 2013-08-16 12:29:25]

Change committed as 156213