Consider slots as a focus scope

third_party/WebKit/Source/core/dom/shadow/AssignedElementTraversal.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/dom/shadow/AssignedElementTraversal.h"
+
+#include "core/dom/Element.h"
+#include "core/dom/ElementTraversal.h"
+#include "core/html/HTMLSlotElement.h"
+
+namespace blink {
+
+HTMLSlotElement* AssignedElementTraversal::slot(const Element& current)
+{
+    if (Element* assignedAncestor = AssignedElementTraversal::assignedAncestor(current))
+        return assignedAncestor->assignedSlot();
+    return nullptr;
+}
+
+Element* AssignedElementTraversal::assignedAncestor(const Element& current)
+{
+    // assignedAncestor returns an ancestor element of current which is directly assigned to a slot.
+    Element* element = const_cast<Element*>(&current);
+    for (; element; element = element->parentElement()) {
+        if (element->assignedSlot())
+            break;
+    }
+    return element;
+}
+
+Element* AssignedElementTraversal::next(const Element& current)
+{
+    // current.assignedSlot returns a slot only when current is assigned explicitly
+    // if current is directly assigned to a slot, return a descendant of current, which is indirectly assigned to the same slot as current.
+    HTMLSlotElement* slot = current.assignedSlot();
+    if (slot) {
+        if (Element* next = ElementTraversal::next(current, &current))
+            return next;
+    } else {
+        // if current is indirectly assigned to a slot, find a directly-assigned ancestor.

[hayato, 2016/03/03 05:42:21]

A sentence in a comment should start with a capital letter.

I understand what *directly/indirectly* means here, but could you avoid to use
*directly/indirectly* here?

e.g. 
// If current is not assigned to a slot, find an ancestor which is assigned to a
slot.

[yuzuchan, 2016/03/04 03:48:37]

Done.

+        Element* assignedAncestor = AssignedElementTraversal::assignedAncestor(current);

[hayato, 2016/03/03 05:42:21]

Could you assert ASSERT(assignedAncestor)?

[yuzuchan, 2016/03/04 03:48:37]

Done.

+        if (Element* next = ElementTraversal::next(current, assignedAncestor))
+            return next;
+        slot = AssignedElementTraversal::slot(*assignedAncestor);

[hayato, 2016/03/03 05:42:21]

"slot = assingedAncestor.assignedSlot()" might be enough.

Nit: ASSERT(slot) can be inserted.

[yuzuchan, 2016/03/04 03:48:37]

Done.

+    }
+    WillBeHeapVector<RefPtrWillBeMember<Node>> assignedNodes = slot->getAssignedNodes();
+    size_t currentIndex = assignedNodes.find(current);

[hayato, 2016/03/03 05:42:21]

Looks |assignedAncestor| should be used instead of |current| if
"current.assignedSlot()" is null.

[yuzuchan, 2016/03/04 03:48:37]

Done.

+    if (currentIndex != kNotFound && currentIndex < assignedNodes.size() - 1)

[hayato, 2016/03/03 05:42:21]

We can ASSERT(currentIndex != kNotFound). It looks we always passes an element
which exists in slot.getAssignedNodes().

[yuzuchan, 2016/03/04 03:48:37]

Done.

+        return toElement(assignedNodes[currentIndex + 1]);

[hayato, 2016/03/03 05:42:21]

assignedNodes can contain a non-Element Node, such as a TextNode.
We have to skip that!

[yuzuchan, 2016/03/04 03:48:37]

Done.

+    return nullptr;
+}
+
+Element* AssignedElementTraversal::previous(const Element& current)
+{
+    Element* assignedAncestor = AssignedElementTraversal::assignedAncestor(current);

[hayato, 2016/03/03 05:42:21]

Can we assert ASSERT(assignedAncestor)?

[yuzuchan, 2016/03/04 03:48:37]

Done.

+    // NodeTraversal within assignedAncestor
+    if (Element* previous = ElementTraversal::previous(current, assignedAncestor))
+        return previous;
+    // if null, jump to previous assigned node's descendant
+    const WillBeHeapVector<RefPtrWillBeMember<Node>> assignedNodes = AssignedElementTraversal::slot(*assignedAncestor)->getAssignedNodes();
+    size_t currentIndex = assignedNodes.reverseFind(current);

[hayato, 2016/03/03 05:42:21]

Looks we have to pass |assignedAncestor|, instead of |current|, to
assignedNodes.reverseFind().

[yuzuchan, 2016/03/04 03:48:37]

Done.

+    if (currentIndex != kNotFound && currentIndex > 0) {

[hayato, 2016/03/03 05:42:21]

ASSERT(currentIndex != kNotFound)

[yuzuchan, 2016/03/04 03:48:37]

Done.

+        Node* assignedPrevious = assignedNodes[currentIndex - 1];
+        if (Node* lastChild = assignedPrevious->lastChild())

[hayato, 2016/03/03 05:42:22]

Looks ElementTraversal::lastWithin(assignedPrevious) should be used here.

[yuzuchan, 2016/03/04 03:48:37]

Done.

+            return toElement(lastChild);
+        // if the previous assigned node has no children, return the assigned node itself

[hayato, 2016/03/03 05:42:21]

assignedPrevious can be non-element Node, such as TextNode.
We should skip a non-element Node and continue to find an Element.

[yuzuchan, 2016/03/04 03:48:37]

Done.

+        return toElement(assignedPrevious);
+    }
+    return nullptr;
+}
+
+bool AssignedElementTraversal::isInAssignedScope(const Element& current)

[hayato, 2016/03/03 05:42:21]

Nit: How about renaming this to isInSubtreeAssignedToSlot?

[yuzuchan, 2016/03/04 03:48:37]

Renamed to isSlotScoped.

+{
+    return AssignedElementTraversal::assignedAncestor(current);
+}
+
+} // namespace blink