Reimplement imperative shadowdom API for supporting slot change event

In this CL, we reuse hashset that control all the assigned slots
in the shadowroot for each node, added previous CL.
Thanks to the hashset we can find the assigned slot for each
node in O(1) order, instead of searching all the slot for each
shadow root when we call slot change event.

We will improve this implementation in another CL.

Result of perftests:
https://pinpoint-dot-chromeperf.appspot.com/job/151f9a47640000

Previous implementation CL: crrev.com/c/1179555

See design doc at
https://docs.google.com/document/d/1n2iDNkI5bF4B5eefxa0SRSyS_RVchvTNSqezvA4PyF4/edit?usp=sharing

Bug: 869308
Change-Id: I96fc4c50d9083eca199c7849db0097e73436983d
Reviewed-on: https://chromium-review.googlesource.com/1201513
Commit-Queue: Kyoko Muto <kymuto@google.com>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592785}

third_party/WebKit/LayoutTests/shadow-dom/imperative-api-slotchange.html

@@ -41,7 +41,39 @@ async_test((test) => {
     n.s1.assign([n.c2]);
     assert_array_equals(n.s2.assignedNodes(), []);
   },1);
-}, 'slotchange event: assign a child to a host.');
+}, 'slotchange event: Assign a assigned child to another slot.');
+
+async_test((test) => {
+  let n = createTestTree(test1);
+  n.s1.assign([n.c1]);
+  n.s2.assign([n.c1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1,n.s2], test);
+    n.s1.assign([])
+  },1);
+}, 'slotchange event: Deassign a child from a slot, and assigned to another slot.');
+
+async_test((test) => {
+  let n = createTestTree(test1);
+  n.s1.assign([n.c1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1], test);
+
+    n.s1.assign([n.c1,n.c2]);
+  },1);
+}, 'slotchange event: Assign a new child to a slot.');
+
+async_test((test) => {
+  let n = createTestTree(test1);
+  n.s1.assign([n.c1,n.c2]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1], test);
+    n.s1.assign([n.c1])
+  },1);
+}, 'slotchange event: Deassign a child from a slot.');
 
 async_test((test) => {
   let n = createTestTree(test1);
@@ -66,15 +98,25 @@ async_test((test) => {
     doneIfSlotChange([n.s1], test);
     n.c1.remove();
   },1);
-}, 'slotchange event: Remove a child to a host.');
+}, 'slotchange event: Remove a child from a host.');
+
+async_test((test) => {
+  let n = createTestTree(test1);
+  let slot = document.createElement('slot');
+  slot.assign([n.c1])
+
+  setTimeout(function () {
+    doneIfSlotChange([slot], test);
+    n.shadowroot.appendChild(slot);
+  },1);
+}, 'slotchange event: Insert a slot.');
 
 async_test((test) => {
   let n = createTestTree(test1);
   n.s1.assign([n.c1]);
-  n.s2.assign([n.c1]);
 
   setTimeout(function () {
-    doneIfSlotChange([n.s2], test);
+    doneIfSlotChange([n.s1], test);
     n.s1.remove();
   },1);
 }, 'slotchange event: Remove a slot from a shadow tree.');
@@ -84,7 +126,7 @@ async_test((test) => {
   n.s1.assign([n.c1]);
   let slot = document.createElement('slot');
   slot.assign([n.c1])
-  assert_array_equals(slot.assignedNodes(), []);
+  assert_array_equals(n.s1.assignedNodes(), [n.c1]);
 
   setTimeout(function () {
     doneIfSlotChange([slot,n.s1], test);
@@ -92,4 +134,66 @@ async_test((test) => {
     assert_array_equals(n.s1.assignedNodes(), []);
   },1);
 }, 'slotchange event: Insert a slot before another slot.');
+
+async_test((test) => {
+  let n = createTestTree(test1);
+  n.s1.assign([n.c1]);
+  n.s2.assign([n.c1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1,n.s2], test);
+    n.s1.remove();
+  },1);
+}, 'slotchange event: Remove a preceding slot from a shadow tree.');
+
+</script>
+ <div id="test2">
+  <div id="host1">
+    <template id="shadowroot" data-mode="open" data-slotting="manual">
+      <div id="host2">
+        <template id="shadowroot1" data-mode="open" data-slotting="manual">
+          <slot id="s3"></slot>
+          <slot id="s2"></slot>
+        </template>
+        <slot id="s1"></slot>
+      </div>
+    </template>
+    <div id="c1"></div>
+   </div>
+  </div>
+<script>
+async_test((test) => {
+  let n = createTestTree(test2);
+  n.s2.assign([n.s1]);
+  n.s3.assign([n.s1,n.c1]);
+  n.s1.assign([n.c1]);
+  assert_array_equals(n.s3.assignedNodes(), [n.s1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1,n.s2,n.s3], test);
+    n.host2.insertBefore(n.s3,n.s1);
+    assert_array_equals(n.s2.assignedNodes(), [n.s1]);
+  },1);
+}, 'slotchange event: Move a slot before another slot.');
+
+async_test((test) => {
+  let n = createTestTree(test2);
+  n.s1.assign([n.c1]);
+  n.s2.assign([n.s1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s1,n.s2], test);
+    n.c1.remove();
+  },1);
+}, 'slotchange event: A slot is assigned to another slot.');
+
+async_test((test) => {
+  let n = createTestTree(test2);
+  n.s2.assign([n.s1]);
+
+  setTimeout(function () {
+    doneIfSlotChange([n.s2], test);
+    n.s1.remove();
+  },1);
+}, 'slotchange event: Even if distributed nodes do not change, slotchange should be fired if assigned nodes are changed.');
 </script>

third_party/WebKit/LayoutTests/shadow-dom/imperative-api.html

@@ -5,139 +5,161 @@ See https://crbug.com/869308
 -->
 <script src="../resources/testharness.js"></script>
 <script src="../resources/testharnessreport.js"></script>
-<div id="host">
-  <div id="child1"></div>
-  <div id="child2"></div>
-  <div id="child3"></div>
+<script src="resources/shadow-dom.js"></script>
+<div id="test1">
+  <div id="host"></div>
+  <div id="host1"></div>
+  <div id="host2"></div>
 </div>
-<div id="host1">
-</div>
-</div>
-<div id="host2">
-</div>
-</div>
-<div id="host3">
-</div>
-<div id="child4"></div>
-<div id="host4">
-  <div id="child5"></div>
-</div>
-</div>
-<div id="host5">
-</div>
-
 <script>
-const host = document.querySelector('#host');
-const host1 = document.querySelector('#host1');
-const host2 = document.querySelector('#host2');
-const host3 = document.querySelector('#host3');
-const child1 = document.querySelector('#child1');
-const child2 = document.querySelector('#child2');
-const child3 = document.querySelector('#child3');
-const child4 = document.querySelector('#child4');
-const child5 = document.querySelector('#child5');
-const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
-const shadow_root1 = host4.attachShadow({ mode: 'open' });
-const shadow_root2 = host5.attachShadow({ mode: 'open', slotting: 'manual' });
-const slot1 = document.createElement('slot');
-const slot2 = document.createElement('slot');
-const slot3 = document.createElement('slot');
-const slot4 = document.createElement('slot');
-const slot5 = document.createElement('slot');
-const slot6 = document.createElement('slot');
-shadow_root.appendChild(slot1);
-shadow_root.appendChild(slot2);
-shadow_root.appendChild(slot3);
-shadow_root.appendChild(slot6);
-shadow_root1.appendChild(slot4);
-
 test(() => {
-  assert_not_equals(host1.attachShadow({ mode: 'open', slotting: 'manual' }),
+  let n = createTestTree(test1);
+  assert_not_equals(n.host.attachShadow({ mode: 'open', slotting: 'manual' }),
                     null, 'slotting manual should work');
-  assert_not_equals(host2.attachShadow({ mode: 'open', slotting: 'auto' }),
+  assert_not_equals(n.host1.attachShadow({ mode: 'open', slotting: 'auto' }),
                     null, 'slotting auto should work');
   assert_throws(new TypeError(), () => {
-    host3.attachShadow({ mode: 'open', slotting: 'exceptional' })},
+  n.host2.attachShadow({ mode: 'open', slotting: 'exceptional' })},
                 'others should throw exception');
 }, 'attachShadow can take slotting parameter');
 
+</script>
+<div id="test2">
+  <div id="host">
+    <template id="shadow_root" data-mode="open" data-slotting="manual">
+      <slot id="s1"></slot>
+      <slot id="s2"></slot>
+      <slot id="s3"></slot>
+    </template>
+    <div id="c1"></div>
+    <div id="c2"></div>
+    <div id="c3"></div>
+  </div>
+  <div id="c4"></div>
+  <div id="host4">
+    <template id="shadow_root1" data-mode="open" data-slotting="manual">
+      <slot id="s5" name="s5"></slot>
+    </template>
+  </div>
+</div>
+<script>
 test(() => {
-  assert_array_equals(slot2.assignedElements(), []);
-  assert_equals(child1.assignedSlot, null);
+  let n = createTestTree(test2);
+  assert_array_equals(n.s2.assignedElements(), []);
+  assert_equals(n.c1.assignedSlot, null);
 
-  slot2.assign([child1]);
+  n.s2.assign([n.c1]);
 
-  assert_array_equals(slot2.assignedNodes(), [child1]);
-  assert_equals(child1.assignedSlot, slot2);
+  assert_array_equals(n.s2.assignedNodes(), [n.c1]);
+  assert_equals(n.c1.assignedSlot, n.s2);
 
-  slot1.assign([child2,child1]);
-  assert_array_equals(slot1.assignedNodes(), [child1,child2]);
-  assert_array_equals(slot2.assignedNodes(), []);
-  assert_equals(child1.assignedSlot, slot1);
-  assert_equals(child2.assignedSlot, slot1);
+  n.s1.assign([n.c2,n.c1]);
+  assert_array_equals(n.s1.assignedNodes(), [n.c1,n.c2]);
+  assert_array_equals(n.s2.assignedNodes(), []);
+  assert_equals(n.c1.assignedSlot, n.s1);
+  assert_equals(n.c2.assignedSlot, n.s1);
 
-  slot1.assign([child2]);
-  assert_array_equals(slot1.assignedNodes(), [child2]);
-  assert_array_equals(slot2.assignedNodes(), [child1]);
+  n.s1.assign([n.c2]);
+  assert_array_equals(n.s1.assignedNodes(), [n.c2]);
+  assert_array_equals(n.s2.assignedNodes(), [n.c1]);
 }, 'assignedNodes/Slot can be used in manual slotting');
 
 test(() => {
-  slot3.assign([child4]);
-  assert_array_equals(slot3.assignedNodes(), []);
+  let n = createTestTree(test2);
+  n.s1.assign([n.c4]);
+  assert_array_equals(n.s1.assignedNodes(), []);
 
-  host.appendChild(child4);
-  assert_array_equals(slot3.assignedNodes(), [child4]);
+  n.host.appendChild(n.c4);
+  assert_array_equals(n.s1.assignedNodes(), [n.c4]);
 }, 'Nodes should be assigned to slot only when nodes are host\'s children');
 
 test(() => {
-  slot5.assign([child1]);
-  assert_array_equals(slot5.assignedNodes(), []);
+  let n = createTestTree(test2);
+  n.s5.assign([n.c1]);
+  assert_array_equals(n.s5.assignedNodes(), []);
 
-  shadow_root.insertBefore(slot5,slot1);
-  assert_array_equals(slot5.assignedNodes(), [child1]);
+  n.shadow_root.insertBefore(n.s5,n.s1);
+  assert_array_equals(n.s5.assignedNodes(), [n.c1]);
 }, 'Nodes should be assigned to a slot only when the slot is in host\'s shadowtree');
 
 test(() => {
-  slot1.assign([child2]);
-  slot2.assign([child2]);
-  assert_array_equals(slot2.assignedNodes(), []);
+  let n = createTestTree(test2);
+  n.s1.assign([n.c1]);
+  n.s2.assign([n.c1]);
+  assert_array_equals(n.s2.assignedNodes(), []);
 
-  shadow_root.insertBefore(slot2,slot1);
-  assert_array_equals(slot2.assignedNodes(), [child2]);
+  n.shadow_root.insertBefore(n.s2,n.s1);
+  assert_array_equals(n.s2.assignedNodes(), [n.c1]);
 }, 'A node should be assigned to a slot when the slot is inserted before the assigned slot');
 
 test(() => {
-  shadow_root.appendChild(slot1);
-  shadow_root.appendChild(slot2);
-  shadow_root.appendChild(slot6);
-  slot1.assign([child2]);
-  slot2.assign([child2]);
-  slot6.assign([child2]);
-  assert_array_equals(slot1.assignedNodes(), [child2]);
-
-  shadow_root.insertBefore(slot6,slot2);
-  slot1.remove();
-  assert_array_equals(slot6.assignedNodes(), [child2]);
+  let n = createTestTree(test2);
+  n.s1.assign([n.c1]);
+  n.s2.assign([n.c1]);
+  n.s3.assign([n.c1]);
+  n.shadow_root.insertBefore(n.s2,n.s1);
+  n.shadow_root.insertBefore(n.s3,n.s1);
+
+  n.s2.remove();
+
+  assert_array_equals(n.s1.assignedNodes(), []);
+  assert_array_equals(n.s3.assignedNodes(), [n.c1]);
 }, 'A slot should be assigned to a node even after the slot is inserted after the assigned slot, and the assigned slot are removed')
 
-  test(() => {
-  shadow_root.insertBefore(slot1,slot2);
-  slot2.assign([child3,child3]);
-  assert_array_equals(slot2.assignedNodes(), [child3]);
+test(() => {
+  let n = createTestTree(test2);
+  n.s1.assign([n.c1]);
+  assert_array_equals(n.s1.assignedNodes(), [n.c1]);
+  n.shadow_root1.insertBefore(n.s1,n.s5);
+
+  assert_array_equals(n.s1.assignedNodes(), []);
+  n.s1.assign([]);
+  n.shadow_root.insertBefore(n.s1,n.s2);
 
-  slot1.assign([child3]);
-  assert_array_equals(slot2.assignedNodes(), []);
+  assert_array_equals(n.s1.assignedNodes(), []);
+}, 'A slot should not be assigned to a node once after the slot is inserted in another shadow root, and assigned another slot.')
 
-  shadow_root2.appendChild(slot2);
-  assert_array_equals(slot2.assignedNodes(), []);
+test(() => {
+  let n = createTestTree(test2);
+  n.shadow_root.insertBefore(n.s1,n.s2);
+  n.s2.assign([n.c1,n.c1]);
+  assert_array_equals(n.s2.assignedNodes(), [n.c1]);
+
+  n.s1.assign([n.c1]);
+  assert_array_equals(n.s2.assignedNodes(), []);
+
+  n.shadow_root1.appendChild(n.s2);
+  assert_array_equals(n.s2.assignedNodes(), []);
 }, 'Same nodes should be ignored');
 
 test(() => {
-  assert_array_equals(slot4.assignedElements(), [child5]);
-  assert_equals(child5.assignedSlot, slot4);
+  let n = createTestTree(test2);
+  n.s1.assign([n.c3]);
+  n.s1.assign([n.c3]);
+
+  n.s1.remove();
+
+  assert_equals(n.c3.assignedSlot, null);
+}, 'Same assignment should be ignored');
+
+</script>
+<div id="test3">
+  <div id="host1">
+    <template id="shadow_root" data-mode="open">
+      <slot id="s1" name="s1"></slot>
+    </template>
+    <div id="c1" slot="s1"></div>
+  </div>
+</div>
+<script>
+test(() => {
+  let n = createTestTree(test3);
+  assert_array_equals(n.s1.assignedElements(), [n.c1]);
+  assert_equals(n.c1.assignedSlot, n.s1);
+
+  n.s1.assign([]);
+  assert_array_equals(n.s1.assignedElements(), [n.c1]);
+  assert_equals(n.c1.assignedSlot, n.s1);
+}, 'Slotting API should not have any effect in auto mode');
 
-  slot4.assign([]);
-  assert_array_equals(slot4.assignedElements(), [child5]);
-  assert_equals(child5.assignedSlot, slot4);
-}, 'slotting API should not have any effect in auto mode');
 </script>

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

@@ -39,6 +39,7 @@ void SlotAssignment::DidAddSlot(HTMLSlotElement& slot) {
   needs_collect_slots_ = true;
   if (owner_->IsManualSlotting()) {
     DidAddSlotInternalInManualMode(slot);
+    SetNeedsAssignmentRecalc();
     return;
   }
 
@@ -59,14 +60,10 @@ void SlotAssignment::DidRemoveSlot(HTMLSlotElement& slot) {
   DCHECK_GT(slot_count_, 0u);
   --slot_count_;
   if (owner_->IsManualSlotting()) {
-    DCHECK(!needs_collect_slots_);
-    CallSlotChangeIfNeeded(slot);
+    CallSlotChangeAfterRemoved(slot);
     DeleteSlotInChildSlotMap(slot);
-
+    SetNeedsAssignmentRecalc();
     needs_collect_slots_ = true;
-    // TODO(crbug.com/869308):Avoid calling Slots in order not to hit the
-    // DCHECK(!needs_collect_slots_)
-    Slots();
     return;
   }
   needs_collect_slots_ = true;
@@ -127,17 +124,10 @@ void SlotAssignment::DidAddSlotInternal(HTMLSlotElement& slot) {
 }
 
 void SlotAssignment::DidAddSlotInternalInManualMode(HTMLSlotElement& slot) {
-  for (Node& child : NodeTraversal::ChildrenOf(owner_->host())) {
-    auto* change_slot = FindSlotChange(slot, child);
-    if (change_slot) {
-      slot.SignalSlotChange();
-      if (change_slot != slot)
-        change_slot->SignalSlotChange();
-    }
-  }
   for (auto node : slot.AssignedNodesCandidate()) {
     InsertSlotInChildSlotMap(slot, *node);
   }
+  CallSlotChangeIfNeeded(slot);
 }
 
 void SlotAssignment::DidRemoveSlotInternal(
@@ -272,8 +262,10 @@ void SlotAssignment::RecalcAssignment() {
     if (!is_user_agent) {
       if (owner_->IsManualSlotting()) {
         auto it = node_to_assigned_slot_candidate_in_tree_order_.find(&child);
-        if (it != node_to_assigned_slot_candidate_in_tree_order_.end())
-          slot = *it.Get()->value.begin();
+        if (it != node_to_assigned_slot_candidate_in_tree_order_.end()) {
+          DCHECK(!it.Get()->value.IsEmpty());
+          slot = it.Get()->value[0];
+        }
       } else {
         slot = FindSlotByName(child.SlotName());
       }
@@ -393,30 +385,54 @@ HTMLSlotElement* SlotAssignment::FindSlotInUserAgentShadow(
 
 HTMLSlotElement* SlotAssignment::FindSlotChange(HTMLSlotElement& slot,
                                                 Node& child) {
-  HTMLSlotElement* found_this_slot = nullptr;
-  for (auto a_slot : Slots()) {
-    if (a_slot == slot) {
-      found_this_slot = &slot;
-      continue;
-    }
-    if (a_slot->ContainsInAssignedNodesCandidates(child)) {
-      if (found_this_slot) {
-        // case2 in DidRemoveSlotChange or DidAddSlotChange
-        return a_slot;
+  // Find the changed slot to call slot change event
+
+  // There are the following 3 cases for addition:
+  //         Before:              After:                           Return
+  // case 1: []                -> [*slot*]                         *slot*
+  // case 2: [old_active, ...] -> [*slot*, old_active, ...]        old_active
+  // case 3: [old_active, ...] -> [old_active, ..., *slot*, ...]   nullptr
+
+  // Also, there are the following 3 cases for removal:
+  //         Before:                            After:             Return
+  // case 1: [*slot*]                        -> []                 *slot*
+  // case 2: [*slot*, new_active, ...]       -> [new_active, ...]  new_active
+  // case 3: [new_active, ..., *slot*, ...]  -> [new_active, ...]  nullptr
+
+  auto it = node_to_assigned_slot_candidate_in_tree_order_.find(&child);
+  if (it != node_to_assigned_slot_candidate_in_tree_order_.end()) {
+    HeapVector<Member<HTMLSlotElement>>& assigned_slots = it.Get()->value;
+    if (assigned_slots[0] == slot) {
+      if (assigned_slots.size() == 1) {
+        // case1
+        return &slot;
       }
-      // case3 in DidRemoveSlotChange or DidAddSlotChange
-      return nullptr;
+      // case2
+      return assigned_slots[1];
     }
   }
-  // case1 in DidRemoveSlotChange or DidAddSlotChange or no slot for the child
-  return found_this_slot;
+  // case3 or no slot for the child
+  return nullptr;
 }
 
 void SlotAssignment::CallSlotChangeIfNeeded(HTMLSlotElement& slot) {
   for (Node& child : NodeTraversal::ChildrenOf(owner_->host())) {
-    auto* change_slot = FindSlotChange(slot, child);
-    if (change_slot && change_slot != slot)
-      change_slot->SignalSlotChange();
+    auto* changed_slot = FindSlotChange(slot, child);
+    if (changed_slot) {
+      slot.SignalSlotChange();
+      if (changed_slot != slot)
+        changed_slot->SignalSlotChange();
+    }
+  }
+}
+void SlotAssignment::CallSlotChangeAfterRemoved(HTMLSlotElement& slot) {
+  for (Node& child : NodeTraversal::ChildrenOf(owner_->host())) {
+    auto* changed_slot = FindSlotChange(slot, child);
+    if (changed_slot) {
+      slot.SignalSlotChangeAfterRemoved();
+      if (changed_slot != slot)
+        changed_slot->SignalSlotChange();
+    }
   }
 }
 

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

@@ -44,6 +44,7 @@ class SlotAssignment final : public GarbageCollected<SlotAssignment> {
 
   bool FindHostChildBySlotName(const AtomicString& slot_name) const;
   void CallSlotChangeIfNeeded(HTMLSlotElement& slot);
+  void CallSlotChangeAfterRemoved(HTMLSlotElement& slot);
   HTMLSlotElement* FindSlotChange(HTMLSlotElement& slot, Node& child);
 
   void DeleteSlotInChildSlotMap(HTMLSlotElement& slot);

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

@@ -189,40 +189,56 @@ const HeapVector<Member<Element>> HTMLSlotElement::AssignedElementsForBinding(
   return elements;
 }
 
-bool HTMLSlotElement::IsAssignedNodeSameWithBefore(
-    HeapVector<Member<Node>>& new_assigned_nodes,
-    HeapHashSet<Member<Node>>& old_assigned_nodes) {
-  if (new_assigned_nodes.size() != old_assigned_nodes.size())
-    return false;
-  for (auto node : old_assigned_nodes) {
-    if (!new_assigned_nodes.Contains(*node))
-      return false;
+HeapVector<Member<Node>>
+HTMLSlotElement::DeleteCommonAssignedNodeAndReturnAddedAssignedNode(
+    const HeapVector<Member<Node>>& new_assigned_nodes) {
+  DCHECK(!assigned_nodes_candidates_.IsEmpty());
+  if (new_assigned_nodes.IsEmpty())
+    return new_assigned_nodes;
+  HeapVector<Member<Node>> added_assigned_nodes = new_assigned_nodes;
+  for (auto node : new_assigned_nodes) {
+    if (assigned_nodes_candidates_.Contains(node)) {
+      assigned_nodes_candidates_.erase(node);
+      added_assigned_nodes.EraseAt(added_assigned_nodes.Find(node));
+    }
   }
-  return true;
+  return added_assigned_nodes;
 }
 
 void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes) {
+  // There are the following 2 groups for each of assignednodescandidates:
+  // Previous AssignedNodesCandidates:     AssignedNodesCandidates:
+  // [{old_assigned_nodes} {same_nodes}]-> [{same_nodes} {new_assigned_nodes}]
+
+  // Classify above three groups and insert and call slot change for
+  // old_assigned_nodes
+  HeapVector<Member<Node>> added_assigned_nodes;
   if (ContainingShadowRoot()) {
-    SlotChangeType slot_change_type =
-        IsAssignedNodeSameWithBefore(nodes, assigned_nodes_candidates_)
-            ? SlotChangeType::kSuppressSlotChangeEvent
-            : SlotChangeType::kSignalSlotChangeEvent;
-    DidSlotChange(slot_change_type);
-    ContainingShadowRoot()->GetSlotAssignment().DeleteSlotInChildSlotMap(*this);
+    if (!assigned_nodes_candidates_.IsEmpty()) {
+      added_assigned_nodes =
+          DeleteCommonAssignedNodeAndReturnAddedAssignedNode(nodes);
+      ContainingShadowRoot()->GetSlotAssignment().CallSlotChangeIfNeeded(*this);
+      ContainingShadowRoot()->GetSlotAssignment().DeleteSlotInChildSlotMap(
+          *this);
+    } else {
+      added_assigned_nodes = nodes;
+    }
   }
+  // Refrash all assigned_nodes_candidates_
   assigned_nodes_candidates_.clear();
   for (Member<Node> child : nodes) {
     assigned_nodes_candidates_.insert(child);
   }
+  // Insert and call slot change for new_assigned_nodes
   if (ContainingShadowRoot()) {
-    ContainingShadowRoot()->GetSlotAssignment().CallSlotChangeIfNeeded(*this);
     // TODO(crbug.com/869308): Don't call SetNeedsAssignmentRecalc if we can
     // detect all possible slotchange surely
     ContainingShadowRoot()->GetSlotAssignment().SetNeedsAssignmentRecalc();
-    for (auto child : nodes) {
+    for (auto child : added_assigned_nodes) {
       ContainingShadowRoot()->GetSlotAssignment().InsertSlotInChildSlotMap(
           *this, *child);
     }
+    ContainingShadowRoot()->GetSlotAssignment().CallSlotChangeIfNeeded(*this);
   }
 }
 
@@ -235,6 +251,10 @@ void HTMLSlotElement::SignalSlotChange() {
   DidSlotChange(SlotChangeType::kSignalSlotChangeEvent);
 }
 
+void HTMLSlotElement::SignalSlotChangeAfterRemoved() {
+  DidSlotChangeAfterRemovedFromShadowTree();
+}
+
 const HeapVector<Member<Node>>& HTMLSlotElement::GetDistributedNodes() {
   DCHECK(!RuntimeEnabledFeatures::IncrementalShadowDOMEnabled());
   DCHECK(!NeedsDistributionRecalc());

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

@@ -53,15 +53,16 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement {
   const HeapVector<Member<Element>> AssignedElementsForBinding(
       const AssignedNodesOptions&);
 
-  bool IsAssignedNodeSameWithBefore(
-      HeapVector<Member<Node>>& new_assigned_nodes,
-      HeapHashSet<Member<Node>>& old_assigned_nodes);
+  HeapVector<Member<Node>> DeleteCommonAssignedNodeAndReturnAddedAssignedNode(
+      const HeapVector<Member<Node>>& new_assigned_nodes);
+
   void assign(HeapVector<Member<Node>> nodes);
   bool ContainsInAssignedNodesCandidates(Node&) const;
-  void SignalSlotChange();
   HeapHashSet<Member<Node>>& AssignedNodesCandidate() {
     return assigned_nodes_candidates_;
   }
+  void SignalSlotChange();
+  void SignalSlotChangeAfterRemoved();
 
   const HeapVector<Member<Node>> FlattenedAssignedNodes();