Re-factor two parameter version of endOfLine()

Source/core/editing/VisibleUnits.cpp

Index: Source/core/editing/VisibleUnits.cpp
diff --git a/Source/core/editing/VisibleUnits.cpp b/Source/core/editing/VisibleUnits.cpp
index ee8bd3f25d5e2b2b2515911a3b7bdcdd10fab245..62799e662dc18e2a1f714a4580ee544b020a7771 100644
--- a/Source/core/editing/VisibleUnits.cpp
+++ b/Source/core/editing/VisibleUnits.cpp
@@ -1029,58 +1029,59 @@ static VisiblePosition endPositionForLine(const VisiblePosition& c, LineEndpoint
     return createVisiblePosition(pos, VP_UPSTREAM_IF_POSSIBLE);
 }
 
-static bool inSameLogicalLine(const VisiblePosition& a, const VisiblePosition& b)
-{
-    return a.isNotNull() && logicalStartOfLine(a).deepEquivalent() == logicalStartOfLine(b).deepEquivalent();
-}
-
-static VisiblePosition endOfLine(const VisiblePosition& c, LineEndpointComputationMode mode)
+// TODO(yosin) Rename this function to reflect the fact it ignores bidi levels.
+VisiblePosition endOfLine(const VisiblePosition& currentPosition)
 {
-    // TODO: this is the current behavior that might need to be fixed.
+    // TODO(yosin) this is the current behavior that might need to be fixed.
     // Please refer to https://bugs.webkit.org/show_bug.cgi?id=49107 for detail.
-    VisiblePosition visPos = endPositionForLine(c, mode);
-
-    if (mode == UseLogicalOrdering) {
-        // Make sure the end of line is at the same line as the given input position. For a wrapping line, the logical end
-        // position for the not-last-2-lines might incorrectly hand back the logical beginning of the next line.
-        // For example, <div contenteditable dir="rtl" style="line-break:before-white-space">abcdefg abcdefg abcdefg
-        // a abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg abcdefg </div>
-        // In this case, use the previous position of the computed logical end position.
-        if (!inSameLogicalLine(c, visPos))
-            visPos = previousPositionOf(visPos);
-
-        if (ContainerNode* editableRoot = highestEditableRoot(c.deepEquivalent())) {
-            if (!editableRoot->contains(visPos.deepEquivalent().computeContainerNode()))
-                return createVisiblePosition(lastPositionInNode(editableRoot));
-        }
-
-        return honorEditingBoundaryAtOrAfter(visPos, c.deepEquivalent());
-    }
-
-    // Make sure the end of line is at the same line as the given input position. Else use the previous position to
-    // obtain end of line. This condition happens when the input position is before the space character at the end
-    // of a soft-wrapped non-editable line. In this scenario, endPositionForLine would incorrectly hand back a position
-    // in the next line instead. This fix is to account for the discrepancy between lines with webkit-line-break:after-white-space style
-    // versus lines without that style, which would break before a space by default.
-    if (!inSameLine(c, visPos)) {
-        visPos = previousPositionOf(c);
+    VisiblePosition visPos = endPositionForLine(currentPosition, UseInlineBoxOrdering);
+
+    // Make sure the end of line is at the same line as the given input
+    // position. Else use the previous position to obtain end of line. This
+    // condition happens when the input position is before the space character
+    // at the end of a soft-wrapped non-editable line. In this scenario,
+    // |endPositionForLine()| would incorrectly hand back a position in the next
+    // line instead. This fix is to account for the discrepancy between lines
+    // with "webkit-line-break:after-white-space" style versus lines without
+    // that style, which would break before a space by default.
+    if (!inSameLine(currentPosition, visPos)) {
+        visPos = previousPositionOf(currentPosition);
         if (visPos.isNull())
             return VisiblePosition();
         visPos = endPositionForLine(visPos, UseInlineBoxOrdering);
     }
 
-    return honorEditingBoundaryAtOrAfter(visPos, c.deepEquivalent());
+    return honorEditingBoundaryAtOrAfter(visPos, currentPosition.deepEquivalent());
 }
 
-// FIXME: Rename this function to reflect the fact it ignores bidi levels.
-VisiblePosition endOfLine(const VisiblePosition& currentPosition)
+static bool inSameLogicalLine(const VisiblePosition& a, const VisiblePosition& b)
 {
-    return endOfLine(currentPosition, UseInlineBoxOrdering);
+    return a.isNotNull() && logicalStartOfLine(a).deepEquivalent() == logicalStartOfLine(b).deepEquivalent();
 }
 
 VisiblePosition logicalEndOfLine(const VisiblePosition& currentPosition)
 {
-    return endOfLine(currentPosition, UseLogicalOrdering);
+    // TODO(yosin) this is the current behavior that might need to be fixed.
+    // Please refer to https://bugs.webkit.org/show_bug.cgi?id=49107 for detail.
+    VisiblePosition visPos = endPositionForLine(currentPosition, UseLogicalOrdering);
+
+    // Make sure the end of line is at the same line as the given input
+    // position. For a wrapping line, the logical end position for the
+    // not-last-2-lines might incorrectly hand back the logical beginning of the
+    // next line. For example,
+    // <div contenteditable dir="rtl" style="line-break:before-white-space">xyz
+    // a xyz xyz xyz xyz xyz xyz xyz xyz xyz xyz </div>

[hajimehoshi, 2015/09/09 10:59:09]

Why did you change this comment?

[yosin_UTC9, 2015/09/09 11:08:49]

to fit into 80 chars 
in previous line.

[hajimehoshi, 2015/09/09 11:11:26]

Hm, you could keep abcdefg, but that's OK to me

+    // In this case, use the previous position of the computed logical end
+    // position.
+    if (!inSameLogicalLine(currentPosition, visPos))
+        visPos = previousPositionOf(visPos);
+
+    if (ContainerNode* editableRoot = highestEditableRoot(currentPosition.deepEquivalent())) {
+        if (!editableRoot->contains(visPos.deepEquivalent().computeContainerNode()))
+            return createVisiblePosition(lastPositionInNode(editableRoot));
+    }
+
+    return honorEditingBoundaryAtOrAfter(visPos, currentPosition.deepEquivalent());
 }
 
 template <typename Strategy>