Make sure the rootNode of a LiveNodeListBase is always a ContainerNode

Make sure the rootNode of a LiveNodeListBase is always a ContainerNode

Make sure the rootNode of a LiveNodeListBase is always a ContainerNode. This
simplifies the logic inside LiveNodeListBase. It also gets rid of ContainerNode
checks in HTMLCollection / LiveNodeList's traverseForwardToOffset() which are
hot in Dromaeo's dom-traverse.html. This improves the following test's score by
~3%:
- Bindings/node-list-access.html

The only exception was when Node.childNodes is called on a non-ContainerNode.
We now handle this special case separately by introducing an EmptyNodeList
class which inherits from NodeList instead of LiveNodeListBase.

This CL also moves the getElementsBy*() methods from Node to ContainerNode
as they only make sense for ContainerNodes. They are actually exposed to JS
on Element, not Node.

This patch is based on WebKit r158536 by <antti@apple.com>;.

R=eseidel, esprehn, abarth

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

[esprehn, 2014-01-17 07:06:19]

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/ChildNod...
File Source/core/dom/ChildNodeList.h (right):

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/ChildNod...
Source/core/dom/ChildNodeList.h:47: class EmptyNodeList FINAL : public NodeList
{
Please put this in its own file.

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
File Source/core/dom/NodeRareData.h (right):

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
Source/core/dom/NodeRareData.h:67: return
static_cast<EmptyNodeList*>(m_childNodeList);
Add a casting method, there's macros for that. toEmptyNodeList(...) that'll do
the ASSERT for you so you don't need it here.

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
Source/core/dom/NodeRareData.h:69: RefPtr<EmptyNodeList> list =
EmptyNodeList::create(node);
If you reverse the clauses you can make this function a lot shorter. You also
don't need the local.

if (!m_childNodeList)
  m_childNodeList = EmptyNodeList::create(node);
return toEmptyNodeList(m_childNodeList.get());

[Inactive, 2014-01-17 15:00:48]

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/ChildNod...
File Source/core/dom/ChildNodeList.h (right):

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/ChildNod...
Source/core/dom/ChildNodeList.h:47: class EmptyNodeList FINAL : public NodeList
{
On 2014/01/17 07:06:20, esprehn wrote:
> Please put this in its own file.

Done.

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
File Source/core/dom/NodeRareData.h (right):

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
Source/core/dom/NodeRareData.h:67: return
static_cast<EmptyNodeList*>(m_childNodeList);
On 2014/01/17 07:06:20, esprehn wrote:
> Add a casting method, there's macros for that. toEmptyNodeList(...) that'll do
> the ASSERT for you so you don't need it here.

Done.

https://codereview.chromium.org/132923003/diff/90001/Source/core/dom/NodeRare...
Source/core/dom/NodeRareData.h:69: RefPtr<EmptyNodeList> list =
EmptyNodeList::create(node);
On 2014/01/17 07:06:20, esprehn wrote:
> If you reverse the clauses you can make this function a lot shorter. You also
> don't need the local.
> 
> if (!m_childNodeList)
>   m_childNodeList = EmptyNodeList::create(node);
> return toEmptyNodeList(m_childNodeList.get());

The issue is that m_childNodeList is a raw pointer, not a RefPtr. We pass
ownership to the caller and merely keep a raw pointer to the list. This is why
we need a temporary RefPtr here. Let me know if there is still a simpler
pattern. At least, I could not come up with something better.

[Inactive, 2014-01-22 03:37:50]

I rebased the patch now that my patch using const refs in LiveNodeListBase has
landed.
How does it look?

[esprehn, 2014-01-22 04:16:47]

rslgtm

[Inactive, 2014-01-22 14:02:59]

On 2014/01/22 04:16:47, esprehn wrote:
> rslgtm

Thanks.

Adam, Eric, I still need someone to approve the small change to web/ when you
get a chance.

[adamk, 2014-01-22 16:56:10]

lgtm for Source/web/

[Inactive, 2014-01-22 16:57:57]

On 2014/01/22 16:56:10, adamk wrote:
> lgtm for Source/web/

Thanks

[commit-bot: I haz the power, 2014-01-22 16:58:16]

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

[commit-bot: I haz the power, 2014-01-22 20:27:16]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2014-01-22 20:29:52]

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

[commit-bot: I haz the power, 2014-01-23 00:22:08]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2014-01-23 01:02:00]

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

[commit-bot: I haz the power, 2014-01-23 03:01:13]

Change committed as 165598