Adds ability to restore previous state in manual slot assignment when encountered exception.

Previous to this CL, during manual slot assignment, if an exception is
thrown, the state of the HTMLSlotElement is indeterminate.
HTMLSlotElement should be restored to the previous state where previous
assigned nodes are preserved.

This CL implements this behavior by doing the exception check of
assigned nodes before the actual assignment. Thus, if an exception
is encountered, the assign() function could just return, preserving
the previous state. However, this way of doing slot assignment adds
an extra loop for assigned nodes. I had tried to do it in
one loop. But that required saving the previous state of the Slot
and restoring it if exception is detected. I think the added
complexity isn't worth it for the little performance benefit.

In addition, I made sure that the state of the
candidate_assigned_slot_map_ is accurate after each slot
assignment recalc. The previous implementation could leave removed
nodes still in the map. Because in SlotAssignment:RecalcAssignment(),
the children traversal, NodeTraversal::ChildrenOf(owner_->host(),
wouldn't encounter removed nodes.

Bug: 869308
Change-Id: I3c8938f1c4360d664497b6d16a7753c545729e6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124293
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757723}

third_party/blink/renderer/core/dom/slot_assignment.cc

@@ -260,13 +260,18 @@ void SlotAssignment::RecalcAssignment() {
         FindSlotByName(HTMLSlotElement::UserAgentCustomAssignSlotName());
   }
 
+  bool is_manual_slot_assignment = owner_->IsManualSlotting();
+  // Replaces candidate_assigned_slot_map_ after the loop, to avoid stale
+  // references resulting from calls to slot->DidRecalcAssignedNodes().
+  HeapHashMap<Member<Node>, Member<HTMLSlotElement>> candidate_map;
+
   for (Node& child : NodeTraversal::ChildrenOf(owner_->host())) {
     if (!child.IsSlotable())
       continue;
 
     HTMLSlotElement* slot = nullptr;
     if (!is_user_agent) {
-      if (owner_->IsManualSlotting()) {
+      if (is_manual_slot_assignment) {
         if (auto* candidate_slot = candidate_assigned_slot_map_.at(&child)) {
           if (candidate_slot->ContainingShadowRoot() == owner_) {
             slot = candidate_slot;
@@ -300,6 +305,8 @@ void SlotAssignment::RecalcAssignment() {
 
     if (slot) {
       slot->AppendAssignedNode(child);
+      if (is_manual_slot_assignment)
+        candidate_map.Set(&child, slot);
     } else {
       child.ClearFlatTreeNodeData();
       child.RemovedFromFlatTree();
@@ -314,6 +321,9 @@ void SlotAssignment::RecalcAssignment() {
 
   for (auto& slot : Slots())
     slot->DidRecalcAssignedNodes();
+
+  if (is_manual_slot_assignment)
+    candidate_assigned_slot_map_.swap(candidate_map);
 }
 
 const HeapVector<Member<HTMLSlotElement>>& SlotAssignment::Slots() {

third_party/blink/renderer/core/html/html_slot_element.cc

@@ -171,6 +171,21 @@ const HeapVector<Member<Element>> HTMLSlotElement::AssignedElementsForBinding(
   return elements;
 }
 
+bool HTMLSlotElement::CheckNodesValidity(HeapVector<Member<Node>> nodes,
+                                         ExceptionState& exception_state) {
+  auto* host = OwnerShadowHost();
+  for (auto& node : nodes) {
+    if (node->parentNode() != host) {
+      exception_state.ThrowDOMException(
+          DOMExceptionCode::kNotAllowedError,
+          "Node:  '" + node->nodeName() +
+              "' is invalid for manual slot assignment.");
+      return false;
+    }
+  }
+  return true;
+}
+
 void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes,
                              ExceptionState& exception_state) {
   if (!SupportsAssignment() || !ContainingShadowRoot()->IsManualSlotting()) {
@@ -180,30 +195,19 @@ void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes,
     return;
   }
 
+  if (!CheckNodesValidity(nodes, exception_state))
+    return;
+
   ContainingShadowRoot()->GetSlotAssignment().ClearCandidateNodes(
       assigned_nodes_candidates_);
   assigned_nodes_candidates_.clear();
-  auto* host = OwnerShadowHost();
-  bool has_invalid_node = false;
   for (auto& node : nodes) {
-    if (node->parentNode() != host) {
-      exception_state.ThrowDOMException(
-          DOMExceptionCode::kNotAllowedError,
-          "Node:  '" + node->nodeName() +
-              "' is invalid for manual slot assignment.");
-      assigned_nodes_candidates_.clear();
-      has_invalid_node = true;
-      break;
-    }
-
     // Before assignment, see if this node belongs to another slot.
     ContainingShadowRoot()->GetSlotAssignment().UpdateCandidateNodeAssignedSlot(
         *node, *this);
     assigned_nodes_candidates_.AppendOrMoveToLast(node);
   }
-
-  if (!has_invalid_node)
-    ContainingShadowRoot()->GetSlotAssignment().SetNeedsAssignmentRecalc();
+  ContainingShadowRoot()->GetSlotAssignment().SetNeedsAssignmentRecalc();
 }
 
 void HTMLSlotElement::AppendAssignedNode(Node& host_child) {

third_party/blink/renderer/core/html/html_slot_element.h

@@ -138,6 +138,7 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement {
       const HeapVector<Member<Node>>& new_slotted);
 
   void SetNeedsDistributionRecalcWillBeSetNeedsAssignmentRecalc();
+  bool CheckNodesValidity(HeapVector<Member<Node>> nodes, ExceptionState&);
 
   // SlotAssignnment:recalc runs in tree order. Update to assigned order.
   void UpdateManuallyAssignedNodesOrdering();

third_party/blink/web_tests/external/wpt/shadow-dom/slots-imperative-slot-api.tentative-expected.txt

@@ -6,7 +6,7 @@ PASS Imperative slot API can assign nodes in manual slot assignment.
 PASS Order of slotables is preserved in manual slot assignment.
 PASS Previously assigned slotable is moved to new slot when it's reassigned.
 PASS Order and assignment of nodes are preserved during multiple assignment in a row.
-FAIL Assigning invalid nodes causes exception and slot returns to its previous state. assert_array_equals: lengths differ, expected array [] length 0, got [Element node <div id="c1"></div>, Element node <div id="c2"></div>] length 2
+PASS Assigning invalid nodes causes exception and slot returns to its previous state.
 PASS Moving a slot to a new host, the slot loses its previously assigned slotables.
 PASS Moving a slot's tree order position within a shadow host has no impact on its assigned slotables.
 PASS Appending slotable to different host, it loses slot assignment. It can be re-assigned within a new host.