Refactoring: Change the order of conditions to avoid computing rendererIsEditable()

Source/core/dom/Position.cpp

Index: Source/core/dom/Position.cpp
diff --git a/Source/core/dom/Position.cpp b/Source/core/dom/Position.cpp
index 72e1ad7579147b72a7abf9b457e4f6d57bb82151..a55af45fa0e427e86f7c0de6fde7392161aa955d 100644
--- a/Source/core/dom/Position.cpp
+++ b/Source/core/dom/Position.cpp
@@ -55,11 +55,11 @@ using namespace HTMLNames;
 static Node* nextRenderedEditable(Node* node)
 {
     while ((node = node->nextLeafNode())) {
-        if (!node->rendererIsEditable())
-            continue;
         RenderObject* renderer = node->renderer();
         if (!renderer)
             continue;
+        if (!node->rendererIsEditable())
+            continue;
         if ((renderer->isBox() && toRenderBox(renderer)->inlineBoxWrapper()) || (renderer->isText() && toRenderText(renderer)->firstTextBox()))
             return node;
     }
@@ -69,11 +69,11 @@ static Node* nextRenderedEditable(Node* node)
 static Node* previousRenderedEditable(Node* node)
 {
     while ((node = node->previousLeafNode())) {
-        if (!node->rendererIsEditable())
-            continue;
         RenderObject* renderer = node->renderer();
         if (!renderer)
             continue;
+        if (!node->rendererIsEditable())
+            continue;
         if ((renderer->isBox() && toRenderBox(renderer)->inlineBoxWrapper()) || (renderer->isText() && toRenderText(renderer)->firstTextBox()))
             return node;
     }
@@ -597,6 +597,11 @@ Position Position::upstream(EditingBoundaryCrossingRule rule) const
     for (; !currentPos.atStart(); currentPos.decrement()) {
         Node* currentNode = currentPos.node();
 
+        // skip position in unrendered or invisible node
+        RenderObject* renderer = currentNode->renderer();

[ojan, 2013/10/01 19:21:39]

Doesn't this change behavior because you've now moved it above the early return?

+        if (!renderer || renderer->style()->visibility() != VISIBLE)
+            continue;
+
         // Don't check for an editability change if we haven't moved to a different node,
         // to avoid the expense of computing rendererIsEditable().
         if (currentNode != lastNode) {
@@ -615,11 +620,6 @@ Position Position::upstream(EditingBoundaryCrossingRule rule) const
         if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
             return lastVisible;
 
-        // skip position in unrendered or invisible node
-        RenderObject* renderer = currentNode->renderer();
-        if (!renderer || renderer->style()->visibility() != VISIBLE)
-            continue;
-
         if (rule == CanCrossEditingBoundary && boundaryCrossed) {
             lastVisible = currentPos;
             break;
@@ -720,6 +720,11 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
     for (; !currentPos.atEnd(); currentPos.increment()) {
         Node* currentNode = currentPos.node();
 
+        // skip position in unrendered or invisible node
+        RenderObject* renderer = currentNode->renderer();

[ojan, 2013/10/01 19:21:39]

Ditto.

+        if (!renderer || renderer->style()->visibility() != VISIBLE)
+            continue;
+
         // Don't check for an editability change if we haven't moved to a different node,
         // to avoid the expense of computing rendererIsEditable().
         if (currentNode != lastNode) {
@@ -748,11 +753,6 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
         if (boundary && boundary->parentNode() == currentNode)
             return lastVisible;
 
-        // skip position in unrendered or invisible node
-        RenderObject* renderer = currentNode->renderer();
-        if (!renderer || renderer->style()->visibility() != VISIBLE)
-            continue;
-
         if (rule == CanCrossEditingBoundary && boundaryCrossed) {
             lastVisible = currentPos;
             break;