Get rid of flat tree version of createVisibleSelection() taking two VisiblePositionInFlatTree

third_party/WebKit/Source/core/editing/PendingSelection.cpp

Index: third_party/WebKit/Source/core/editing/PendingSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/PendingSelection.cpp b/third_party/WebKit/Source/core/editing/PendingSelection.cpp
index 8641bbf17d9d2a1de8da12a627b38c934c5c2924..ef9c19d4fc1956f88d23614d4b3483d8c35f8c38 100644
--- a/third_party/WebKit/Source/core/editing/PendingSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/PendingSelection.cpp
@@ -82,17 +82,28 @@ VisibleSelectionInFlatTree PendingSelection::calcVisibleSelection(
       start, selectionType == SelectionType::RangeSelection
                  ? TextAffinity::Downstream
                  : affinity);
+  if (visibleStart.isNull())

[Xiaocheng, 2016/10/19 02:33:47]

Does it change the behavior? |createVisiblePosition(pos, pos)| currently uses
SelectionTemplate::Builder::setBaseAndExtentDeprecated, which does not
necessarily returns null when |visibleStart| is null.

[yosin_UTC9, 2016/10/19 03:59:58]

In the call site (only one), we do:

  const VisibleSelectionInFlatTree& selection =
      calcVisibleSelection(originalSelection);

  if (!selection.isRange()) {
    layoutView.clearSelection();
    return;
  }

So, caret and none are handled as same.

I think this is yet another example, it is better to handle null explicitly. ;-)
If original code does, so we don't need to discuss now.

+    return VisibleSelectionInFlatTree();
   if (paintBlockCursor) {
-    VisiblePositionInFlatTree visibleExtent =
-        createVisiblePosition(end, affinity);
-    visibleExtent = nextPositionOf(visibleExtent, CanSkipOverEditingBoundary);
-    return createVisibleSelection(visibleStart, visibleExtent);
+    const VisiblePositionInFlatTree visibleExtent = nextPositionOf(
+        createVisiblePosition(end, affinity), CanSkipOverEditingBoundary);
+    if (visibleExtent.isNull())

[Xiaocheng, 2016/10/19 02:33:47]

Same question.

[yosin_UTC9, 2016/10/19 03:59:58]

ditto.

+      return VisibleSelectionInFlatTree();
+    SelectionInFlatTree::Builder builder;
+    builder.collapse(visibleStart.toPositionWithAffinity());
+    builder.extend(visibleExtent.deepEquivalent());
+    return createVisibleSelection(builder.build());
   }
   const VisiblePositionInFlatTree visibleEnd =
       createVisiblePosition(end, selectionType == SelectionType::RangeSelection
                                      ? TextAffinity::Upstream
                                      : affinity);
-  return createVisibleSelection(visibleStart, visibleEnd);
+  if (visibleEnd.isNull())

[Xiaocheng, 2016/10/19 02:33:47]

Same question.

[yosin_UTC9, 2016/10/19 03:59:58]

ditto.

+    return VisibleSelectionInFlatTree();
+  SelectionInFlatTree::Builder builder;
+  builder.collapse(visibleStart.toPositionWithAffinity());
+  builder.extend(visibleEnd.deepEquivalent());
+  return createVisibleSelection(builder.build());
 }
 
 void PendingSelection::commit(LayoutView& layoutView) {