Add DCHECK for Before/After position

This patch adds |DCHECK()| to narrow down the case of returning null from
|Position::ComputeContainerNode()| for helping to search the root cause of
the bug[1] caused by |ComputeContainerNode()| returning null for non-null
position.

This patch also changes tests using before/after position of shadow root.
Node: document and document fragment, including shadow root, can not be an
anchor node of before/after position, since these position can not be represent
by RangeBoundaryPoint == container node with offset of child.

[1] https://crbug.com/882592 SelectionEditor should not call
Node::ContainsIncludingHostElements() with null

Bug: 882592, 889737
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I9c7b0c8ad1c97d20a9080aa9baddea69faf805c5
Reviewed-on: https://chromium-review.googlesource.com/1248361Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594636}

third_party/blink/renderer/core/dom/document_test.cc

@@ -341,7 +341,8 @@ TEST_F(DocumentTest, CreateRangeAdjustedToTreeScopeWithPositionInShadowTree) {
       "<div><select><option>012</option></div>");
   Element* const select_element = GetDocument().QuerySelector("select");
   const Position& position =
-      Position::AfterNode(*select_element->UserAgentShadowRoot());
+      Position(*select_element->UserAgentShadowRoot(),
+               select_element->UserAgentShadowRoot()->CountChildren());
   Range* const range =
       Document::CreateRangeAdjustedToTreeScope(GetDocument(), position);
   EXPECT_EQ(range->startContainer(), select_element->parentNode());

third_party/blink/renderer/core/editing/position.cc

@@ -97,25 +97,23 @@ PositionTemplate<Strategy>::PositionTemplate(const Node* anchor_node,
     : anchor_node_(const_cast<Node*>(anchor_node)),
       offset_(0),
       anchor_type_(anchor_type) {
-  if (!anchor_node_) {
-    anchor_type_ = PositionAnchorType::kOffsetInAnchor;
-    return;
-  }
+#if DCHECK_IS_ON()
+  DCHECK(anchor_node_);
+  DCHECK_NE(anchor_type_, PositionAnchorType::kOffsetInAnchor);
+  DCHECK(CanBeAnchorNode<Strategy>(anchor_node_.Get())) << anchor_node_;
   if (anchor_node_->IsTextNode()) {
     DCHECK(anchor_type_ == PositionAnchorType::kBeforeAnchor ||
-           anchor_type_ == PositionAnchorType::kAfterAnchor);
+           anchor_type_ == PositionAnchorType::kAfterAnchor)
+        << *this;
     return;
   }
-  if (anchor_node_->IsDocumentNode()) {
-    // Since |RangeBoundaryPoint| can't represent before/after Document, we
-    // should not use them.
-    DCHECK(IsBeforeChildren() || IsAfterChildren()) << anchor_type_;
+  if (!Strategy::Parent(*anchor_node_)) {
+    // Before/After |anchor_node_| should have a parent node for converting
+    // to offset in anchor position.
+    DCHECK(IsBeforeChildren() || IsAfterChildren()) << *this;
     return;
   }
-#if DCHECK_IS_ON()
-  DCHECK(CanBeAnchorNode<Strategy>(anchor_node_.Get())) << anchor_node_;
 #endif
-  DCHECK_NE(anchor_type_, PositionAnchorType::kOffsetInAnchor);
 }
 
 // TODO(editing-dev): Once we change type of |anchor_node_| to
@@ -190,8 +188,12 @@ Node* PositionTemplate<Strategy>::ComputeContainerNode() const {
     case PositionAnchorType::kOffsetInAnchor:
       return anchor_node_.Get();
     case PositionAnchorType::kBeforeAnchor:
-    case PositionAnchorType::kAfterAnchor:
-      return Strategy::Parent(*anchor_node_);
+    case PositionAnchorType::kAfterAnchor: {
+      Node* const parent = Strategy::Parent(*anchor_node_);
+      // TODO(https://crbug.com/889737), Once we fix the issue, we should have
+      // |DCHECK(parent)|.
+      return parent;
+    }
   }
   NOTREACHED();
   return nullptr;

third_party/blink/renderer/core/editing/position_test.cc

@@ -201,12 +201,6 @@ TEST_F(PositionTest, ToPositionInFlatTreeWithShadowRoot) {
   EXPECT_EQ(PositionInFlatTree(host, PositionAnchorType::kBeforeChildren),
             ToPositionInFlatTree(
                 Position(shadow_root, PositionAnchorType::kBeforeChildren)));
-  EXPECT_EQ(PositionInFlatTree(host, PositionAnchorType::kAfterAnchor),
-            ToPositionInFlatTree(
-                Position(shadow_root, PositionAnchorType::kAfterAnchor)));
-  EXPECT_EQ(PositionInFlatTree(host, PositionAnchorType::kBeforeAnchor),
-            ToPositionInFlatTree(
-                Position(shadow_root, PositionAnchorType::kBeforeAnchor)));
 }
 
 TEST_F(PositionTest,