Fix focus navigation for nested slot case

third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp

Index: third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp
diff --git a/third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp b/third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp
index bbffc1cef8b7212c90bc0a9a4f10efaf0e6417bf..10c651c3477e9495a611a66e7029cb222ab94b3a 100644
--- a/third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp
+++ b/third_party/WebKit/Source/core/dom/shadow/SlotScopedTraversal.cpp
@@ -10,18 +10,54 @@
 
 namespace blink {
 
+namespace {
+Element* nextSkippingChildrenOfShadowHost(const Element& start,
+                                          const Element& scope) {
+  DCHECK(scope.assignedSlot());
+  if (!start.authorShadowRoot()) {
+    if (Element* first = ElementTraversal::firstChild(start))
+      return first;
+  }
+
+  for (const Element* current = &start; current != scope;
+       current = current->parentElement()) {
+    if (Element* nextSibling = ElementTraversal::nextSibling(*current))
+      return nextSibling;
+  }
+  return nullptr;
+}
+
+Element* lastWithinOrSelfSkippingChildrenOfShadowHost(const Element& scope) {
+  Element* current = const_cast<Element*>(&scope);
+  while (!current->authorShadowRoot()) {
+    Element* lastChild = ElementTraversal::lastChild(*current);
+    if (!lastChild)
+      break;
+    current = lastChild;
+  }
+  return current;
+}
+
+Element* previousSkippingChildrenOfShadowHost(const Element& start,
+                                              const Element& scope) {
+  DCHECK(scope.assignedSlot());
+  DCHECK_NE(start, &scope);
+  if (Element* previousSibling = ElementTraversal::previousSibling(start))
+    return lastWithinOrSelfSkippingChildrenOfShadowHost(*previousSibling);
+  return start.parentElement();
+}
+}  // namespace
+
 HTMLSlotElement* SlotScopedTraversal::findScopeOwnerSlot(
     const Element& current) {
-  if (Element* nearestAncestorAssignedToSlot =
-          SlotScopedTraversal::nearestAncestorAssignedToSlot(current))
-    return nearestAncestorAssignedToSlot->assignedSlot();
+  if (Element* nearestInclusiveAncestorAssignedToSlot =
+          SlotScopedTraversal::nearestInclusiveAncestorAssignedToSlot(current))
+    return nearestInclusiveAncestorAssignedToSlot->assignedSlot();
   return nullptr;
 }
 
-Element* SlotScopedTraversal::nearestAncestorAssignedToSlot(
+Element* SlotScopedTraversal::nearestInclusiveAncestorAssignedToSlot(
     const Element& current) {
-  // nearestAncestorAssignedToSlot 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())
@@ -31,27 +67,21 @@ Element* SlotScopedTraversal::nearestAncestorAssignedToSlot(
 }
 
 Element* SlotScopedTraversal::next(const Element& current) {
-  // current.assignedSlot returns a slot only when current is assigned
-  // explicitly
-  // If current is assigned to a slot, return a descendant of current, which is
-  // in the assigned scope of the same slot as current.
-  HTMLSlotElement* slot = current.assignedSlot();
-  Element* nearestAncestorAssignedToSlot =
-      SlotScopedTraversal::nearestAncestorAssignedToSlot(current);
-  if (slot) {
-    if (Element* next = ElementTraversal::next(current, &current))
-      return next;
-  } else {
-    // If current is in assigned scope, find an assigned ancestor.
-    DCHECK(nearestAncestorAssignedToSlot);
-    if (Element* next =
-            ElementTraversal::next(current, nearestAncestorAssignedToSlot))
-      return next;
-    slot = nearestAncestorAssignedToSlot->assignedSlot();
-    DCHECK(slot);
-  }
+  Element* nearestInclusiveAncestorAssignedToSlot =
+      SlotScopedTraversal::nearestInclusiveAncestorAssignedToSlot(current);
+  DCHECK(nearestInclusiveAncestorAssignedToSlot);
+  // Search within children of an element which is assigned to a slot.
+  if (Element* next = nextSkippingChildrenOfShadowHost(
+          current, *nearestInclusiveAncestorAssignedToSlot))
+    return next;
+
+  // Seek to the next element assigned to the same slot.
+  HTMLSlotElement* slot =
+      nearestInclusiveAncestorAssignedToSlot->assignedSlot();
+  DCHECK(slot);
   HeapVector<Member<Node>> assignedNodes = slot->assignedNodes();

[hayato, 2016/10/31 08:12:01]

Could you update other places to use a reference? I think there are still
several places where a copy happens.

[kochi, 2016/10/31 09:29:10]

Done.

-  size_t currentIndex = assignedNodes.find(*nearestAncestorAssignedToSlot);
+  size_t currentIndex =
+      assignedNodes.find(*nearestInclusiveAncestorAssignedToSlot);
   DCHECK_NE(currentIndex, kNotFound);
   for (++currentIndex; currentIndex < assignedNodes.size(); ++currentIndex) {
     if (assignedNodes[currentIndex]->isElementNode())
@@ -61,33 +91,57 @@ Element* SlotScopedTraversal::next(const Element& current) {
 }
 
 Element* SlotScopedTraversal::previous(const Element& current) {
-  Element* nearestAncestorAssignedToSlot =
-      SlotScopedTraversal::nearestAncestorAssignedToSlot(current);
-  DCHECK(nearestAncestorAssignedToSlot);
-  // NodeTraversal within nearestAncestorAssignedToSlot
-  if (Element* previous =
-          ElementTraversal::previous(current, nearestAncestorAssignedToSlot))
+  Element* nearestInclusiveAncestorAssignedToSlot =
+      SlotScopedTraversal::nearestInclusiveAncestorAssignedToSlot(current);
+  DCHECK(nearestInclusiveAncestorAssignedToSlot);
+
+  if (current != nearestInclusiveAncestorAssignedToSlot) {
+    // Search within children of an element which is assigned to a slot.
+    Element* previous = previousSkippingChildrenOfShadowHost(
+        current, *nearestInclusiveAncestorAssignedToSlot);
+    DCHECK(previous);
     return previous;
-  // If null, jump to previous assigned node's descendant
+  }
+
+  // Seek to the previous element assigned to the same slot.
   const HeapVector<Member<Node>> assignedNodes =
-      nearestAncestorAssignedToSlot->assignedSlot()->assignedNodes();
+      nearestInclusiveAncestorAssignedToSlot->assignedSlot()->assignedNodes();
   size_t currentIndex =
-      assignedNodes.reverseFind(*nearestAncestorAssignedToSlot);
+      assignedNodes.reverseFind(*nearestInclusiveAncestorAssignedToSlot);
   DCHECK_NE(currentIndex, kNotFound);
   for (; currentIndex > 0; --currentIndex) {
-    const Member<Node> assignedPrevious = assignedNodes[currentIndex - 1];
-    if (assignedPrevious->isElementNode()) {
-      if (Element* last =
-              ElementTraversal::lastWithin(*toElement(assignedPrevious)))
-        return last;
-      return toElement(assignedPrevious);
-    }
+    const Member<Node> assignedNode = assignedNodes[currentIndex - 1];
+    if (!assignedNode->isElementNode())
+      continue;
+    return lastWithinOrSelfSkippingChildrenOfShadowHost(
+        *toElement(assignedNode));
+  }
+  return nullptr;
+}
+
+Element* SlotScopedTraversal::firstAssignedToSlot(HTMLSlotElement& slot) {
+  const HeapVector<Member<Node>>& assignedNodes = slot.assignedNodes();
+  for (auto assignedNode : assignedNodes) {
+    if (assignedNode->isElementNode())
+      return toElement(assignedNode);
+  }
+  return nullptr;
+}
+
+Element* SlotScopedTraversal::lastAssignedToSlot(HTMLSlotElement& slot) {
+  HeapVector<Member<Node>> assignedNodes = slot.assignedNodes();
+  for (auto assignedNode = assignedNodes.rbegin();
+       assignedNode != assignedNodes.rend(); ++assignedNode) {
+    if (!(*assignedNode)->isElementNode())
+      continue;
+    return lastWithinOrSelfSkippingChildrenOfShadowHost(
+        *toElement(*assignedNode));
   }
   return nullptr;
 }
 
 bool SlotScopedTraversal::isSlotScoped(const Element& current) {
-  return SlotScopedTraversal::nearestAncestorAssignedToSlot(current);
+  return SlotScopedTraversal::nearestInclusiveAncestorAssignedToSlot(current);
 }
 
 }  // namespace blink