Validate offset parameter of Position constructor with introducing...

Validate offset parameter of Position constructor with introducing Position::CreateWithoutValidation()

This patch changes |PositionTempalte<Strategy>| constructor to validate offset
parameter for improving code health.

This patch also introduces |CreateWithoutValidation()| for callers which need
out of bound positions, and deprecated version for callers without sane reason.

Change-Id: I0f072aacd34fe5af5b5398c824632ce6ac7a1772
Reviewed-on: https://chromium-review.googlesource.com/987655
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547597}

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

@@ -72,8 +72,10 @@ PositionTemplate<Strategy> PositionTemplate<Strategy>::EditingPositionOf(
   if (!anchor_node || anchor_node->IsTextNode())
     return PositionTemplate<Strategy>(anchor_node, offset);
 
-  if (!EditingIgnoresContent(*anchor_node))
-    return PositionTemplate<Strategy>(anchor_node, offset);
+  if (!EditingIgnoresContent(*anchor_node)) {
+    return PositionTemplate<Strategy>::CreateWithoutValidationDeprecated(
+        *anchor_node, offset);
+  }
 
   if (offset == 0)
     return PositionTemplate<Strategy>(anchor_node,
@@ -125,12 +127,23 @@ PositionTemplate<Strategy>::PositionTemplate(const Node* anchor_node,
     : anchor_node_(const_cast<Node*>(anchor_node)),
       offset_(offset),
       anchor_type_(PositionAnchorType::kOffsetInAnchor) {
-  if (anchor_node_)
-    DCHECK_GE(offset, 0);
-  else
-    DCHECK_EQ(offset, 0);
 #if DCHECK_IS_ON()
   DCHECK(CanBeAnchorNode<Strategy>(anchor_node_.Get())) << anchor_node_;
+  if (!anchor_node_) {
+    DCHECK_EQ(offset, 0);
+    return;
+  }
+  if (anchor_node_->IsCharacterDataNode()) {
+    DCHECK_GE(offset, 0);
+    DCHECK_LE(static_cast<unsigned>(offset),
+              ToCharacterData(anchor_node_)->length())
+        << anchor_node_;
+    return;
+  }
+  DCHECK_GE(offset, 0);
+  DCHECK_LE(static_cast<unsigned>(offset),
+            Strategy::CountChildren(*anchor_node))
+      << anchor_node_;
 #endif
 }
 
@@ -145,6 +158,25 @@ PositionTemplate<Strategy>::PositionTemplate(const PositionTemplate& other)
       offset_(other.offset_),
       anchor_type_(other.anchor_type_) {}
 
+// static
+template <typename Strategy>
+PositionTemplate<Strategy> PositionTemplate<Strategy>::CreateWithoutValidation(
+    const Node& container,
+    int offset) {
+  PositionTemplate<Strategy> result(container, 0);
+  result.offset_ = offset;
+  return result;
+}
+
+// static
+template <typename Strategy>
+PositionTemplate<Strategy>
+PositionTemplate<Strategy>::CreateWithoutValidationDeprecated(
+    const Node& container,
+    int offset) {
+  return CreateWithoutValidation(container, offset);
+}
+
 // --
 
 template <typename Strategy>

third_party/WebKit/Source/core/editing/Position.h

@@ -73,6 +73,19 @@ class CORE_TEMPLATE_CLASS_EXPORT PositionTemplate {
 
   PositionTemplate(const PositionTemplate&);
 
+  // Returns a newly created |Position| with |kOffsetInAnchor|. |offset| can be
+  // out of bound. Out of bound position is used for computing undo/redo
+  // selection for merging text typing.
+  static PositionTemplate<Strategy> CreateWithoutValidation(
+      const Node& container,
+      int offset);
+
+  // TODO(editing-dev): Once we get a reason to use out of bound position,
+  // we should change caller to use |CreateWithoutValidation()|.
+  static PositionTemplate<Strategy> CreateWithoutValidationDeprecated(
+      const Node& container,
+      int offset);
+
   explicit operator bool() const { return IsNotNull(); }
 
   PositionAnchorType AnchorType() const { return anchor_type_; }

third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp

@@ -454,7 +454,8 @@ void CompositeEditCommand::UpdatePositionForNodeRemovalPreservingChildren(
   position = ComputePositionForNodeRemoval(position, node);
   if (offset == 0)
     return;
-  position = Position(position.ComputeContainerNode(), offset);
+  position = Position::CreateWithoutValidationDeprecated(
+      *position.ComputeContainerNode(), offset);
 }
 
 HTMLSpanElement*

third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp

@@ -944,8 +944,9 @@ static Position ComputeExtentForForwardDeleteUndo(
           ? selection.End().ComputeOffsetInContainerNode() -
                 selection.Start().ComputeOffsetInContainerNode()
           : selection.End().ComputeOffsetInContainerNode();
-  return Position(extent.ComputeContainerNode(),
-                  extent.ComputeOffsetInContainerNode() + extra_characters);
+  return Position::CreateWithoutValidation(
+      *extent.ComputeContainerNode(),
+      extent.ComputeOffsetInContainerNode() + extra_characters);
 }
 
 void TypingCommand::ForwardDeleteKeyPressed(TextGranularity granularity,

third_party/WebKit/Source/core/layout/LayoutObject.cpp

@@ -3579,7 +3579,10 @@ PositionWithAffinity LayoutObject::CreatePositionWithAffinity(
     if (!HasEditableStyle(*node)) {
       // If it can be found, we prefer a visually equivalent position that is
       // editable.
-      const Position position = Position(node, offset);
+      // TODO(layout-dev): Once we fix callers of |CreatePositionWithAffinity()|
+      // we should use |Position| constructor. See http://crbug.com/827923
+      const Position position =
+          Position::CreateWithoutValidationDeprecated(*node, offset);
       Position candidate =
           MostForwardCaretPosition(position, kCanCrossEditingBoundary);
       if (HasEditableStyle(*candidate.AnchorNode()))