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

Source/core/dom/Position.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 37 matching lines @@
 #include "wtf/text/CString.h"
 #include "wtf/unicode/CharacterNames.h"
 
 namespace WebCore {
 
 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;
     }
     return 0;
 }
 
 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;
     }
     return 0;
 }
 
 Position::Position(PassRefPtr<Node> anchorNode, LegacyEditingOffset offset)
     : m_anchorNode(anchorNode)
     , m_offset(offset.value())
     , m_anchorType(anchorTypeForLegacyEditingPosition(m_anchorNode.get(), m_offset))
@@ skipping 503 matching lines @@
     Node* boundary = enclosingVisualBoundary(startNode);
     // FIXME: PositionIterator should respect Before and After positions.
     PositionIterator lastVisible = m_anchorType == PositionIsAfterAnchor ? createLegacyEditingPosition(m_anchorNode.get(), caretMaxOffset(m_anchorNode.get())) : *this;
     PositionIterator currentPos = lastVisible;
     bool startEditable = startNode->rendererIsEditable();
     Node* lastNode = startNode;
     bool boundaryCrossed = false;
     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) {
             // Don't change editability.
             bool currentEditable = currentNode->rendererIsEditable();
             if (startEditable != currentEditable) {
                 if (rule == CannotCrossEditingBoundary)
                     break;
                 boundaryCrossed = true;
             }
             lastNode = currentNode;
         }
 
         // If we've moved to a position that is visually distinct, return the last saved position. There
         // is code below that terminates early if we're *about* to move to a visually distinct position.
         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;
         }
 
         // track last visible streamer position
         if (isStreamer(currentPos))
             lastVisible = currentPos;
 
         // Don't move past a position that is visually distinct.  We could rely on code above to terminate and
@@ skipping 80 matching lines @@
     Node* boundary = enclosingVisualBoundary(startNode);
     // FIXME: PositionIterator should respect Before and After positions.
     PositionIterator lastVisible = m_anchorType == PositionIsAfterAnchor ? createLegacyEditingPosition(m_anchorNode.get(), caretMaxOffset(m_anchorNode.get())) : *this;
     PositionIterator currentPos = lastVisible;
     bool startEditable = startNode->rendererIsEditable();
     Node* lastNode = startNode;
     bool boundaryCrossed = false;
     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) {
             // Don't change editability.
             bool currentEditable = currentNode->rendererIsEditable();
             if (startEditable != currentEditable) {
                 if (rule == CannotCrossEditingBoundary)
                     break;
                 boundaryCrossed = true;
             }
 
             lastNode = currentNode;
         }
 
         // stop before going above the body, up into the head
         // return the last visible streamer position
         if (currentNode->hasTagName(bodyTag) && currentPos.atEndOfNode())
             break;
 
         // Do not move to a visually distinct position.
         if (endsOfNodeAreVisuallyDistinctPositions(currentNode) && currentNode != boundary)
             return lastVisible;
         // Do not move past a visually disinct position.
         // Note: The first position after the last in a node whose ends are visually distinct
         // positions will be [boundary->parentNode(), originalBlock->nodeIndex() + 1].
         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;
         }
 
         // track last visible streamer position
         if (isStreamer(currentPos))
             lastVisible = currentPos;
 
         // Return position before tables and nodes which have content that can be ignored.
@@ skipping 618 matching lines @@
     pos.showTreeForThis();
 }
 
 void showTree(const WebCore::Position* pos)
 {
     if (pos)
         pos->showTreeForThis();
 }
 
 #endif