Element moving between shadow-trees for no-op between lifecycles.

FlatTreeNodeData is cleared on insertion so that nodes moving between
shadow trees do not keep stale parent information since we use the
FlatTreeNodeData ancestor to mark ancestors style dirty without updating
the slot assignment.

This is normally fine since inserting a child node of a shadow host will
mark the host for slot re-assignment, and the slot re-assignment will update
FlatTreeNodeData accordingly. The diffing of flat tree children in
NotifySlottedNodesOfFlatTreeChange will mark the inserted node for style
recalc via FlatTreeParentChanged.

There is however a glitch which can happen if a node is removed from a
shadow host, inserted into a different node, and then moved back into
the original shadow host without having any re-slotting happening in
between. If the node ends up in the same flat tree position, we will not
be able to detect that it needs FlatTreeParentChanged to be marked for
style recalc.

Example:

<div id="host">
  <:shadow-root>
    <slot></slot>
  </:shadow-root>
  <span>PASS</span></div>
<div id="other"></div>
<script>
  // Make everything clean. FlatTreeNodeData ancestor for the span is
  // the slot.
  host.offsetTop;
  let slotted = host.querySelector("span");

  // The span is removed, the layout object is detached, and the
  // ComputedStyle is cleared.
  slotted.remove();

  // Insert span into the #other element, FlatTreeNodeData for the span
  // is cleared. The host is marked for slot re-assignment.
  other.appendChild(slotted);

  // The span is re-added as a child of #host, but nothing is marked
  // style dirty since the #host has a shadow root and the span is not
  // yet part of the flat tree.
  host.appendChild(slotted);

  // Slot re-assignment happens, but the result of slotting is the same
  // as for the last lifecycle update, and nothing is marked dirty. That
  // results in the span not getting a ComputedStyle nor a LayoutObject.
  host.offsetTop;
</script>

The solution is currently to do a FlatTreeParentChanged for the case
where we:

1. Already have a FlatTreeNodeData.
2. The FlatTreeNodeData ancestor is null before the slot assignments.
3. The node is assigned to a slot.

It is definitely correct and necessary that we call
FlatTreeParentChanged in this case, but we end up doing it twice for
other elements which were added to the host, which used to be a child of
another host (FlatTreeNodeData being non-null), and will also have
FlatTreeParentChanged being called in the diffing as well. The question
is if that will cause perf regressions, typically in microbenchmarks.

Bug: 1072475
Change-Id: I5e697570b74599b0a3e01ca869eac022105fc6b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2160930
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763537}

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

@@ -273,6 +273,11 @@ void HTMLSlotElement::ClearAssignedNodesAndFlatTreeChildren() {
 void HTMLSlotElement::UpdateFlatTreeNodeDataForAssignedNodes() {
   Node* previous = nullptr;
   for (auto& current : assigned_nodes_) {
+    bool flat_tree_parent_changed = false;
+    if (!current->NeedsStyleRecalc() && !current->GetComputedStyle()) {
+      if (auto* node_data = current->GetFlatTreeNodeData())
+        flat_tree_parent_changed = !node_data->AssignedSlot();
+    }
     FlatTreeNodeData& flat_tree_node_data = current->EnsureFlatTreeNodeData();
     flat_tree_node_data.SetAssignedSlot(this);
     flat_tree_node_data.SetPreviousInAssignedNodes(previous);
@@ -281,6 +286,8 @@ void HTMLSlotElement::UpdateFlatTreeNodeDataForAssignedNodes() {
       previous->GetFlatTreeNodeData()->SetNextInAssignedNodes(current);
     }
     previous = current;
+    if (flat_tree_parent_changed)
+      current->FlatTreeParentChanged();
   }
   if (previous) {
     DCHECK(previous->GetFlatTreeNodeData());

third_party/blink/web_tests/external/wpt/html/rendering/replaced-elements/the-select-element/select-multiple-re-add-option-via-document-fragment-ref.html

+<!doctype html>
+<title>Reference: Move option from select[multiple] into DocumentFragment and back</title>
+<p>You should see the word PASS below.</p>
+<select multiple><option>PASS</option></select>

third_party/blink/web_tests/external/wpt/html/rendering/replaced-elements/the-select-element/select-multiple-re-add-option-via-document-fragment.html

+<!doctype html>
+<title>Test: Move option from select[multiple] into DocumentFragment and back</title>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/rendering.html#the-select-element-2">
+<link rel="match" href="select-multiple-re-add-option-via-document-fragment-ref.html">
+<p>You should see the word PASS below.</p>
+<select multiple id="sel"><option id="opt">PASS</option></select>
+<script>
+  document.body.offsetTop;
+  let rm = opt;
+  document.createDocumentFragment().appendChild(rm);
+  sel.appendChild(rm);
+</script>