Have NodeTraversal::nextSkippingChildren() take a reference

Have NodeTraversal::nextSkippingChildren() take a reference

Have NodeTraversal::nextSkippingChildren() take a reference instead of a pointer
as it expects the pointer to be non-null. This makes the code look safer and is
consistent with NodeTraversal::next().

R=adamk

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

[adamk, 2013-11-12 21:07:42]

lgtm with a few cleanup nits

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
Source/core/editing/TextIterator.cpp:802: while ((next =
NodeTraversal::nextSkippingChildren(*next))) {
Nit: can you remove the extra parentheses while you're modifying this line?

https://codereview.chromium.org/69543007/diff/1/Source/core/html/HTMLCollecti...
File Source/core/html/HTMLCollection.cpp (right):

https://codereview.chromium.org/69543007/diff/1/Source/core/html/HTMLCollecti...
Source/core/html/HTMLCollection.cpp:579: while ((next =
nextMatchingChildElement(this, *next, root))) {
And more extra parens here.

[Inactive, 2013-11-12 21:19:16]

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
Source/core/editing/TextIterator.cpp:802: while ((next =
NodeTraversal::nextSkippingChildren(*next))) {
On 2013/11/12 21:07:43, adamk wrote:
> Nit: can you remove the extra parentheses while you're modifying this line?

I would but I cannot :)

../../third_party/WebKit/Source/core/editing/TextIterator.cpp: In function ‘bool
WebCore::shouldEmitNewlineAfterNode(WebCore::Node&)’:
../../third_party/WebKit/Source/core/editing/TextIterator.cpp:802:60: error:
suggest parentheses around assignment used as truth value [-Werror=parentheses]

I am using gcc 4.7.

[adamk, 2013-11-12 22:21:35]

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/69543007/diff/1/Source/core/editing/TextItera...
Source/core/editing/TextIterator.cpp:802: while ((next =
NodeTraversal::nextSkippingChildren(*next))) {
On 2013/11/12 21:19:16, Chris Dumez wrote:
> On 2013/11/12 21:07:43, adamk wrote:
> > Nit: can you remove the extra parentheses while you're modifying this line?
> 
> I would but I cannot :)
> 
> ../../third_party/WebKit/Source/core/editing/TextIterator.cpp: In function
‘bool
> WebCore::shouldEmitNewlineAfterNode(WebCore::Node&)’:
> ../../third_party/WebKit/Source/core/editing/TextIterator.cpp:802:60: error:
> suggest parentheses around assignment used as truth value
[-Werror=parentheses]
> 
> I am using gcc 4.7.

Sigh. Oh well, lgtm anyway

[commit-bot: I haz the power, 2013-11-12 22:24:52]

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

[commit-bot: I haz the power, 2013-11-13 03:59:10]

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, 2013-11-13 04:15:33]

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

[commit-bot: I haz the power, 2013-11-13 04:15:43]

Failed to apply patch for Source/core/editing/ReplaceSelectionCommand.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/editing/ReplaceSelectionCommand.cpp
  Hunk #3 FAILED at 343.
  1 out of 4 hunks FAILED -- saving rejects to file
Source/core/editing/ReplaceSelectionCommand.cpp.rej

Patch:       Source/core/editing/ReplaceSelectionCommand.cpp
Index: Source/core/editing/ReplaceSelectionCommand.cpp
diff --git a/Source/core/editing/ReplaceSelectionCommand.cpp
b/Source/core/editing/ReplaceSelectionCommand.cpp
index
3f6691d3d0ee2461759ecfebc3230e46b7fae6de..6e37a783a4e27f5e90122bb44a5fd90e15f36ef2
100644
--- a/Source/core/editing/ReplaceSelectionCommand.cpp
+++ b/Source/core/editing/ReplaceSelectionCommand.cpp
@@ -315,7 +315,7 @@ void ReplacementFragment::removeInterchangeNodes(Node*
container)
     while (node) {
         RefPtr<Node> next = NodeTraversal::next(*node);
         if (isInterchangeConvertedSpaceSpan(node)) {
-            next = NodeTraversal::nextSkippingChildren(node);
+            next = NodeTraversal::nextSkippingChildren(*node);
             removeNodePreservingChildren(node);
         }
         node = next.get();
@@ -335,7 +335,7 @@ inline void
ReplaceSelectionCommand::InsertedNodes::willRemoveNodePreservingChil
     if (m_firstNodeInserted == node)
         m_firstNodeInserted = NodeTraversal::next(node);
     if (m_lastNodeInserted == node)
-        m_lastNodeInserted = node.lastChild() ? node.lastChild() :
NodeTraversal::nextSkippingChildren(&node);
+        m_lastNodeInserted = node.lastChild() ? node.lastChild() :
NodeTraversal::nextSkippingChildren(node);
 }
 
 inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node& node)
@@ -343,10 +343,11 @@ inline void
ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node& node)
     if (m_firstNodeInserted == node && m_lastNodeInserted == node) {
         m_firstNodeInserted = 0;
         m_lastNodeInserted = 0;
-    } else if (m_firstNodeInserted == node)
-        m_firstNodeInserted =
NodeTraversal::nextSkippingChildren(m_firstNodeInserted.get());
-    else if (m_lastNodeInserted == node)
+    } else if (m_firstNodeInserted == node) {
+        m_firstNodeInserted =
NodeTraversal::nextSkippingChildren(*m_firstNodeInserted);
+    } else if (m_lastNodeInserted == node) {
         m_lastNodeInserted =
NodeTraversal::previousSkippingChildren(m_lastNodeInserted.get());
+    }
 }
 
 inline void ReplaceSelectionCommand::InsertedNodes::didReplaceNode(Node& node,
Node& newNode)
@@ -703,7 +704,7 @@ static void removeHeadContents(ReplacementFragment&
fragment)
             || node->hasTagName(metaTag)
             || node->hasTagName(styleTag)
             || isHTMLTitleElement(node)) {
-            next = NodeTraversal::nextSkippingChildren(node);
+            next = NodeTraversal::nextSkippingChildren(*node);
             fragment.removeNode(node);
         } else {
             next = NodeTraversal::next(*node);

[commit-bot: I haz the power, 2013-11-13 14:35:56]

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

[commit-bot: I haz the power, 2013-11-13 16:06:20]

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

[commit-bot: I haz the power, 2013-11-13 16:07:40]

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

[commit-bot: I haz the power, 2013-11-13 18:01:41]

Change committed as 161935