Node.appendChild should throw TypeError when newChild is not an object

Node.appendChild should throw TypeError when newChild is not an object

SPEC: http://www.w3.org/TR/WebIDL/#es-interface
According to spec,
'If Type(V) is not Object, then throw a TypeError.'

So this CL modified it as per specified.

TEST=LayoutTests/fast/dom/incompatible-operations.html
    ,LayoutTests/fast/dom/move-nodes-across-documents.html
    ,LayoutTests/fast/dom/processing-instruction-appendChild-exceptions.xhtml

In addition, with this CL,
4 test cases will be passed at
http://w3c-test.org/dom/nodes/Node-appendChild.html

Behavior in other browsers.
*)FF: PASS
*)IE: FAIL

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

[kangil_, 2014-07-08 14:23:06]

PTAL

[haraken, 2014-07-08 14:59:02]

The behavior change looks fine. The new behavior aligns with the spec and
Firefox. Also the behavior change just affects exception types, so it wouldn't
have compatibility issues.

https://codereview.chromium.org/374093002/diff/1/Source/bindings/core/v8/cust...
File Source/bindings/core/v8/custom/V8NodeCustom.cpp (right):

https://codereview.chromium.org/374093002/diff/1/Source/bindings/core/v8/cust...
Source/bindings/core/v8/custom/V8NodeCustom.cpp:114: return;

Can you refer to the generated code for [TypeChecking=Interface] and write the
same code in this custom file?

See:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[kangil_, 2014-07-09 01:34:17]

Great, thanks!
PTAL.

https://codereview.chromium.org/374093002/diff/1/Source/bindings/core/v8/cust...
File Source/bindings/core/v8/custom/V8NodeCustom.cpp (right):

https://codereview.chromium.org/374093002/diff/1/Source/bindings/core/v8/cust...
Source/bindings/core/v8/custom/V8NodeCustom.cpp:114: return;
On 2014/07/08 14:59:01, haraken wrote:
> 
> Can you refer to the generated code for [TypeChecking=Interface] and write the
> same code in this custom file?
> 
> See:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Done.

[haraken, 2014-07-09 01:38:53]

LGTM

https://codereview.chromium.org/374093002/diff/40001/Source/bindings/core/v8/...
File Source/bindings/core/v8/custom/V8NodeCustom.cpp (right):

https://codereview.chromium.org/374093002/diff/40001/Source/bindings/core/v8/...
Source/bindings/core/v8/custom/V8NodeCustom.cpp:112:
throwMinimumArityTypeErrorForMethod("appendChild", "Node", 1, info.Length(),
info.GetIsolate());

Can we make this change in a separate change with a test?

[kangil_, 2014-07-09 02:53:20]

Fortunately, current layout tests has coverage. :)
PTAL

https://codereview.chromium.org/374093002/diff/40001/Source/bindings/core/v8/...
File Source/bindings/core/v8/custom/V8NodeCustom.cpp (right):

https://codereview.chromium.org/374093002/diff/40001/Source/bindings/core/v8/...
Source/bindings/core/v8/custom/V8NodeCustom.cpp:112:
throwMinimumArityTypeErrorForMethod("appendChild", "Node", 1, info.Length(),
info.GetIsolate());
On 2014/07/09 01:38:53, haraken wrote:
> 
> Can we make this change in a separate change with a test?

Sure, I will.

[haraken, 2014-07-09 03:09:58]

> Fortunately, current layout tests has coverage. :)

Even if we already have coverage, we want to make web-exposed changes
incrementally.

[kangil_, 2014-07-09 04:53:33]

On 2014/07/09 03:09:58, haraken wrote:
> > Fortunately, current layout tests has coverage. :)
> 
> Even if we already have coverage, we want to make web-exposed changes
> incrementally.

Got it, done.
PTAL, thanks!

[haraken, 2014-07-09 04:57:17]

LGTM

[kangil_, 2014-07-09 04:57:29]

On 2014/07/09 04:53:33, kangil_ wrote:
> On 2014/07/09 03:09:58, haraken wrote:
> > > Fortunately, current layout tests has coverage. :)
> > 
> > Even if we already have coverage, we want to make web-exposed changes
> > incrementally.
> 
> Got it, done.
> PTAL, thanks!

I will make a patch again. Please hold review on.

[kangil_, 2014-07-09 05:39:41]

On 2014/07/09 04:57:29, kangil_ wrote:
> On 2014/07/09 04:53:33, kangil_ wrote:
> > On 2014/07/09 03:09:58, haraken wrote:
> > > > Fortunately, current layout tests has coverage. :)
> > > 
> > > Even if we already have coverage, we want to make web-exposed changes
> > > incrementally.
> > 
> > Got it, done.
> > PTAL, thanks!
> 
> I will make a patch again. Please hold review on.

PTAL.

Sorry for the noise.
Patchset 6 broke test cases that expect NotFoundError so I had to rework.

[haraken, 2014-07-09 05:41:29]

still LGTM

[kangil_, 2014-07-09 05:46:51]

The CQ bit was checked by kangil.han@samsung.com

[commit-bot: I haz the power, 2014-07-09 05:47:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kangil.han@samsung.com/374093002/110001

[commit-bot: I haz the power, 2014-07-09 06:48:20]

Change committed as 177721