Do not traverse nodes not having a layout object while searching editable visible position.

Source/core/editing/htmlediting.cpp

Index: Source/core/editing/htmlediting.cpp
diff --git a/Source/core/editing/htmlediting.cpp b/Source/core/editing/htmlediting.cpp
index 721a3afc75e61b9ac4dc4b994cbc2d844ae3aaa5..a15a40e9988df27afc6c0263da367eb2270ce13f 100644
--- a/Source/core/editing/htmlediting.cpp
+++ b/Source/core/editing/htmlediting.cpp
@@ -250,12 +250,17 @@ PositionInComposedTree nextCandidate(const PositionInComposedTree& position)
 
 Position nextVisuallyDistinctCandidate(const Position& position)
 {
+    // TODO(changseok): Use PositionIterator instead.
     Position p = position;
     Position downstreamStart = p.downstream();
     while (!p.atEndOfTree()) {
         p = p.next(Character);
         if (p.isCandidate() && p.downstream() != downstreamStart)
             return p;
+        if (auto* node = p.containerNode()) {
+            if (!node->isDocumentFragment() && !node->layoutObject())

[leviw_travelin_and_unemployed, 2015/06/29 20:17:56]

Can you help me understand why this is needed?

[changseok, 2015/06/30 05:50:58]

I'm not sure if I correctly understand the root cause on this though, let me try
my best to elaborate the situation here. After recent commits on htmlediting.cpp
(since https://codereview.chromium.org/1174183004 maybe?) document-fragment
nodes have come in this function. As my understanding, the document-fragment is
an invisible document because it's a kind of offscreen document tree and is not
appended to DOM tree yet.
(https://developer.mozilla.org/en-US/docs/Web/API/Document/createDocumentFragment)
According to a normal flow before my cl, the p.containerNode() for the last
iteration in this while loop is a documentFragment node and its p.atEndOfTree()
at the moment is true. So we don't go over the next iteration. After my cl,
however node->layoutObject() is false for the documentFragment node, so we find
a new p through lastPositionInOrAfterNode(node) at the following line and
p.atEndOfTree() of the new p is false now. Thus we go into an infinite loop.
To avoid this loop, I added a condition, !node->isDocumentFragment() so that we
don't find a new position for documentFragment even though it has no
layoutObject.

[yosin_UTC9, 2015/07/01 01:50:12]

Bottom line:
You don't need to check |!node->isDocumentFragment()| here, since it is shadow
root and we don't define behavior on this function on shadow tree.


Background:
I'm working on make this function to work on composed tree for selection for
composed tree, aka selection on shadow DOM tree, by templatizing this function
as |previousVisuallyDistinctCandidate()|. Once it is done, we don't see shadow
root here.

In other words, functions for |Position| should not handle shadow tree related
things, e.g. shadow root, active insertion point (CONTENT/SHADOW elements).
These function will be worked on composed tree.

+                p = lastPositionInOrAfterNode(node);
+        }
     }
     return Position();
 }
@@ -286,12 +291,17 @@ PositionInComposedTree previousCandidate(const PositionInComposedTree& position)
 template <typename PositionType>
 PositionType previousVisuallyDistinctCandidateAlgorithm(const PositionType& position)
 {
+    // TODO(changseok): Use PositionIterator instead.
     PositionType p = position;
     PositionType downstreamStart = p.downstream();
     while (!p.atStartOfTree()) {
         p = p.previous(Character);
         if (p.isCandidate() && p.downstream() != downstreamStart)
             return p;
+        if (auto* node = p.containerNode()) {
+            if (!node->isDocumentFragment() && !node->layoutObject())
+                p = PositionType::firstPositionInOrBeforeNode(node);
+        }
     }
     return PositionType();
 }