Avoid calling slower Node::firstChild() / Node::lastChild() when possible

Avoid calling slower Node::firstChild() / Node::lastChild() when possible

Avoid calling slower Node::firstChild() / Node::lastChild() when possible by
using tighter typing. This way, we end up using ContainerNode::firstChild() /
ContainerNode::lastChild(), bypassing a useless isContainerNode() check.

R=eseidel@chromium.org, esprehn@chromium.org

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

[esprehn, 2014-06-02 19:03:37]

This contains an unsafe cast.

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
File Source/core/dom/NodeTraversal.cpp (right):

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
Source/core/dom/NodeTraversal.cpp:113: while (previous->lastChild())
This is silly, don't call it twice.

while (Node* child = previous->lastChild())
  previous = child;

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
Source/core/dom/NodeTraversal.cpp:143: next =
toContainerNode(next)->firstChild();
dito.

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp
File Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp#ne...
Source/core/dom/Range.cpp:738: Node* n =
toContainerNode(container)->firstChild();
This is not safe, a DOCUMENT_TYPE_NODE is not a ContainerNode.

[Inactive, 2014-06-02 20:36:17]

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp
File Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp#ne...
Source/core/dom/Range.cpp:738: Node* n =
toContainerNode(container)->firstChild();
On 2014/06/02 19:03:37, esprehn wrote:
> This is not safe, a DOCUMENT_TYPE_NODE is not a ContainerNode.

Good catch, thanks.

[Inactive, 2014-06-02 20:46:24]

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
File Source/core/dom/NodeTraversal.cpp (right):

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
Source/core/dom/NodeTraversal.cpp:113: while (previous->lastChild())
On 2014/06/02 19:03:37, esprehn wrote:
> This is silly, don't call it twice.
> 
> while (Node* child = previous->lastChild())
>   previous = child;

Done.

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/NodeTraversa...
Source/core/dom/NodeTraversal.cpp:143: next =
toContainerNode(next)->firstChild();
On 2014/06/02 19:03:37, esprehn wrote:
> dito.

Done.

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp
File Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/307243004/diff/1/Source/core/dom/Range.cpp#ne...
Source/core/dom/Range.cpp:738: Node* n =
toContainerNode(container)->firstChild();
On 2014/06/02 20:36:18, Chris Dumez wrote:
> On 2014/06/02 19:03:37, esprehn wrote:
> > This is not safe, a DOCUMENT_TYPE_NODE is not a ContainerNode.
> 
> Good catch, thanks.

I kept this change out for now.

[esprehn, 2014-06-05 01:12:08]

lgtm

[commit-bot: I haz the power, 2014-06-05 01:13:19]

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

[commit-bot: I haz the power, 2014-06-05 01:13:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-05 01:13:27]

Failed to apply patch for Source/core/dom/Node.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/dom/Node.cpp
  Hunk #1 FAILED at 1485.
  1 out of 1 hunk FAILED -- saving rejects to file Source/core/dom/Node.cpp.rej

Patch:       Source/core/dom/Node.cpp
Index: Source/core/dom/Node.cpp
diff --git a/Source/core/dom/Node.cpp b/Source/core/dom/Node.cpp
index
b9980baebe0adb0a7e6d33e94104122ea37304eb..eaf2289d60d8e309e2fdb118bccf2224bfa8a472
100644
--- a/Source/core/dom/Node.cpp
+++ b/Source/core/dom/Node.cpp
@@ -1485,7 +1485,7 @@ static void appendTextContent(const Node* node, bool
convertBRsToNewlines, bool&
     case Node::ATTRIBUTE_NODE:
     case Node::DOCUMENT_FRAGMENT_NODE:
         isNullString = false;
-        for (Node* child = node->firstChild(); child; child =
child->nextSibling()) {
+        for (Node* child = toContainerNode(node)->firstChild(); child; child =
child->nextSibling()) {
             if (child->nodeType() == Node::COMMENT_NODE || child->nodeType() ==
Node::PROCESSING_INSTRUCTION_NODE)
                 continue;
             appendTextContent(child, convertBRsToNewlines, isNullString,
content);

[Inactive, 2014-06-05 01:58:49]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-06-05 01:59:54]

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

[commit-bot: I haz the power, 2014-06-05 03:06:21]

Change committed as 175524