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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 64 matching lines @@
                                           PositionMoveType::CodeUnit)
                          : end;
     selection.setWithoutValidation(start, endPosition);
     return selection;
   }
 
   const VisiblePositionInFlatTree& visibleStart = createVisiblePosition(
       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) {
   if (!hasPendingSelection())
     return;
   DCHECK(!layoutView.needsLayout());
   m_hasPendingSelection = false;
 
   const VisibleSelectionInFlatTree& originalSelection =
       m_frameSelection->visibleSelection<EditingInFlatTreeStrategy>();
@@ skipping 44 matching lines @@
   DCHECK(layoutView == endLayoutObject->view());
   layoutView.setSelection(startLayoutObject, startPos.computeEditingOffset(),
                           endLayoutObject, endPos.computeEditingOffset());
 }
 
 DEFINE_TRACE(PendingSelection) {
   visitor->trace(m_frameSelection);
 }
 
 }  // namespace blink