Make PositionAlgorithm<Strategy>::next() to use Strategy::index() (issue 1263033003 by yosin@chromium.org)

[yo...@chromium.org]

Reviewers: tkent, yoichio, hajimehoshi,

Message:
PTAL

Description:
Make PositionAlgorithm<Strategy>::next() to use Strategy::index()

This patch changes |PositionAlgorithm<Strategy>::next()| to use
|Strategy::index(Node&)| instead of |Node::nodexIndex()| to compute offset
of node in parent node with DOM tree or composed tree base on |Strategy|
template parameter for correct behavior in |PositionInComposedTree|.

This patch is a preparation of templatizing |VisiblePosition|.

BUG=513568
TEST=PositionTest.NextNodeIndex


Please review this at https://codereview.chromium.org/1263033003/

Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Affected files (+14, -1 lines):
M Source/core/dom/Position.cpp
M Source/core/dom/PositionTest.cpp


Index: Source/core/dom/Position.cpp
diff --git a/Source/core/dom/Position.cpp b/Source/core/dom/Position.cpp
index
68fc3ac705d545b561cbff4d8e78c1867d51bd6c..1b381a296ce602913504801ffc41e927ec631e83
100644
--- a/Source/core/dom/Position.cpp
+++ b/Source/core/dom/Position.cpp
@@ -464,7 +464,7 @@ PositionAlgorithm<Strategy>
PositionAlgorithm<Strategy>::next(PositionMoveType m
}

if (ContainerNode* parent = Strategy::parent(*node))
- return createLegacyEditingPosition(parent, node->nodeIndex() + 1);
+ return createLegacyEditingPosition(parent, Strategy::index(*node)
+ 1);
return PositionAlgorithm<Strategy>(*this);
}

Index: Source/core/dom/PositionTest.cpp
diff --git a/Source/core/dom/PositionTest.cpp
b/Source/core/dom/PositionTest.cpp
index
516d4a8ed23840fb884eea4a17379289fcf8f8f4..a5ae99170fa9e9401026c42a85dfc2f2ea261d42
100644
--- a/Source/core/dom/PositionTest.cpp
+++ b/Source/core/dom/PositionTest.cpp
@@ -12,6 +12,19 @@ namespace blink {
class PositionTest : public EditingTestBase {
};

+TEST_F(PositionTest, NextNodeIndex)
+{
+ const char* bodyContent = "<p id='host'>00<b id='one'>11</b><b
id='two'>22</b>33</p>";
+ const char* shadowContent = "<content select=#two></content><content
select=#one></content>";
+ setBodyContent(bodyContent);
+ RefPtrWillBeRawPtr<ShadowRoot> shadowRoot =
setShadowContent(shadowContent);
+ Node* host = document().getElementById("host");
+ Node* two = document().getElementById("two");
+
+ EXPECT_EQ(Position(host, 3), Position(two, 2).next());
+ EXPECT_EQ(PositionInComposedTree(host, 1), PositionInComposedTree(two,
2).next());
+}
+
TEST_F(PositionTest, NodeAsRangeLastNodeNull)
{
EXPECT_EQ(nullptr, Position().nodeAsRangeLastNode());

[hajim...@chromium.org]

lgtm

https://codereview.chromium.org/1263033003/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1263033003/1
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1263033003/1


https://codereview.chromium.org/1263033003/

[commit-bot@chromium.org via codereview.chromium.org]

Try jobs failed on following builders:
blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/38966)

https://codereview.chromium.org/1263033003/

[tk...@chromium.org]

lgtm



https://codereview.chromium.org/1263033003/

[commit-bot@chromium.org via codereview.chromium.org]

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199920

https://codereview.chromium.org/1263033003/