createFragmentFromMarkupWithContext() should use EphemeralRange.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/editing/EphemeralRange.h"
 
 #include "core/dom/Range.h"
 #include "core/editing/EditingTestBase.h"
 #include <sstream>
 
 namespace blink {
 
 class EphemeralRangeTest : public EditingTestBase {
  protected:
   template <typename Traversal = NodeTraversal>
   std::string traverseRange(Range*) const;
 
   template <typename Strategy>
   std::string traverseRange(const EphemeralRangeTemplate<Strategy>&) const;
 
+  template <typename Strategy>
+  std::string commonAncesstorNode(
+      const EphemeralRangeTemplate<Strategy>&) const;
+
   Range* getBodyRange() const;
 };
 
 template <typename Traversal>
 std::string EphemeralRangeTest::traverseRange(Range* range) const {
   std::stringstream nodesContent;
   for (Node* node = range->firstNode(); node != range->pastLastNode();
        node = Traversal::next(*node)) {
     nodesContent << "[" << *node << "]";
   }
@@ skipping 10 matching lines @@
 
   return nodesContent.str();
 }
 
 Range* EphemeralRangeTest::getBodyRange() const {
   Range* range = Range::create(document());
   range->selectNode(document().body());
   return range;
 }
 
+template <typename Strategy>
+std::string EphemeralRangeTest::commonAncesstorNode(
+    const EphemeralRangeTemplate<Strategy>& range) const {
+  std::stringstream nodesContent;
+
+  nodesContent << "[" << *range.commonAncestorContainer() << "]";
+
+  return nodesContent.str();
+}
+
 // Tests that |EphemeralRange::nodes()| will traverse the whole range exactly as
 // |for (Node* n = firstNode(); n != pastLastNode(); n = Traversal::next(*n))|
 // does.
 TEST_F(EphemeralRangeTest, rangeTraversal) {
   const char* bodyContent =
       "<p id='host'><b id='one'></b><b id='two'>22</b></p>";
   setBodyContent(bodyContent);
 
   const std::string expectedNodes(
       "[BODY][P id=\"host\"][B id=\"one\"][B id=\"two\"][#text \"22\"]");
@@ skipping 80 matching lines @@
   // Tree iterators have only |operator !=| ATM.
   EXPECT_FALSE(iterable.begin() != iterable.end());
 
   const EphemeralRange singlePositionRange(getBodyRange()->startPosition());
   EXPECT_FALSE(singlePositionRange.isNull());
   EXPECT_EQ(std::string(), traverseRange(singlePositionRange));
   EXPECT_EQ(singlePositionRange.startPosition().nodeAsRangeFirstNode(),
             singlePositionRange.endPosition().nodeAsRangePastLastNode());
 }
 
+TEST_F(EphemeralRangeTest, commonAncesstor) {

[Xiaocheng, 2017/03/02 20:00:16]

Each test case is supposed to be minimized.

So, we should have two separate test cases, one for DOM tree and the other for
flat tree.

[tanvir, 2017/03/04 13:55:31]

Separate test cases for DOM and Flat tree is added!!

+  const char* bodyContent =
+      "<p id='host'><b id='one'></b><input type='text' "

[Xiaocheng, 2017/03/02 20:00:16]

Hmm... I don't like the existing test cases at all. They introduce extra
dependency on the UA shadow dom under <input>, which is totally irrelevant to
EphemralRange.

I've filed crbug.com/697989 for it.

For your new test cases, could you use some other better HTML? There are full of
good examples in, say, VisibleSelectionTest, VisibleUnitsTest,
EditingUtilitiesTest, ...

[tanvir, 2017/03/04 13:55:31]

New HTML used.

+      "value='some'><span "
+      "id='two'></p>";
+
+  setBodyContent(bodyContent);
+
+  Range* range = getBodyRange();
+  range->setStart(document().getElementById("one"), 0,
+                  IGNORE_EXCEPTION_FOR_TESTING);
+
+  // Range* endPos = getBodyRange();
+  range->setEnd(document().getElementById("two"), 0,
+                IGNORE_EXCEPTION_FOR_TESTING);
+
+  const EphemeralRange ephemeralRange(range->startPosition(),
+                                      range->endPosition());
+  EXPECT_EQ("[P id=\"host\"]", commonAncesstorNode<>(ephemeralRange));

[Xiaocheng, 2017/03/02 20:00:16]

There's no need to do string comparison, as there is only one node involved in
the result.

Just compare the nodes directly:

const Node* commonAncestor = document().getElementById(...);
...
const EphemeralRange range(..., ...);
EXPECT_EQ(commonAncestor, range.commonAncestorContainer());

[tanvir, 2017/03/04 13:55:31]

Done!!

+
+  // CommonAncesstorCotainer for FlatTree
+  EXPECT_EQ("[P id=\"host\"]",

[Xiaocheng, 2017/03/02 20:00:16]

For the test case for flat tree, to make the test non-trivial, the start and end
positions of the range must be in different tree scopes. For example, we can
have start position in the main document, and end position in a shadow tree.

[tanvir, 2017/03/04 13:55:31]

Taken care of this scenario. This tree is returning HTML as a common ancesstor
container and therefore documentElement() is used.

+            commonAncesstorNode(EphemeralRangeInFlatTree(
+                toPositionInFlatTree(range->startPosition()),
+                toPositionInFlatTree(range->endPosition()))));
+}
+
 }  // namespace blink