Repository: servo/servo
File: components/script/dom/node.rs (Rust)

Code before:
            node.clean_up_style_and_layout_data();

            // Step 11 & 14.1. Run the removing steps.
            // This needs to be in its own loop, because unbind_from_tree may
            // rely on the state of IS_IN_DOC of the context node's descendants,
            // e.g. when removing a <form>.
            vtable_for(&node).unbind_from_tree(context, can_gc);

            // Step 12 & 14.2. Enqueue disconnected custom element reactions.
            if is_parent_connected {
                if let Some(element) = node.as_custom_element() {
                    custom_element_reaction_stack.enqueue_callback_reaction(
                        &element,
                        CallbackReaction::Disconnected,
                        None,
                    );
                }
            }
        }

        // Clear containing shadow roots after `unbind_from_tree`. That's because
        // some elements (most notable <style>) need to retrieve information from
        // the containing root to clear up data.
        for node in root.traverse_preorder(ShadowIncluding::No) {
            node.set_containing_shadow_root(None);
        }
    }

    /// Removes the given child from this node's list of children.
    ///
    /// Fails unless `child` is a child of this node.
    fn remove_child(&self, child: &Node, cached_index: Option<u32>, can_gc: CanGc) {
        assert!(child.parent_node.get().as_deref() == Some(self));
        self.note_dirty_descendants();

        let prev_sibling = child.GetPreviousSibling();
        match prev_sibling {
            None => {
                self.first_child.set(child.next_sibling.get().as_deref());
            },
            Some(ref prev_sibling) => {
                prev_sibling
                    .next_sibling
                    .set(child.next_sibling.get().as_deref());
            },
        }
        let next_sibling = child.GetNextSibling();
        match next_sibling {
            None => {
                self.last_child.set(child.prev_sibling.get().as_deref());
            },

Review comment (performance):
This seems very expensive. Is there a reason this can't be in `Node::unbind_from_tree`? You would need to check if `NodeFlags::IS_IN_SHADOW_TREE` is set, to cover the case where the entire shadow tree is removed.

Code after:
        // Since both the initial traversal in light dom and the inner traversal
        // in shadow DOM share the same code, we define a closure to prevent omissions.
        let cleanup_node = |node: &Node| {
            node.clean_up_style_and_layout_data();

            // Step 11 & 14.1. Run the removing steps.
            // This needs to be in its own loop, because unbind_from_tree may
            // rely on the state of IS_IN_DOC of the context node's descendants,
            // e.g. when removing a <form>.
            vtable_for(node).unbind_from_tree(context, can_gc);

            // Step 12 & 14.2. Enqueue disconnected custom element reactions.
            if is_parent_connected {
                if let Some(element) = node.as_custom_element() {
                    custom_element_reaction_stack.enqueue_callback_reaction(
                        &element,
                        CallbackReaction::Disconnected,
                        None,
                    );
                }
            }
        };

        for node in root.traverse_preorder(ShadowIncluding::No) {
            cleanup_node(&node);

            // Make sure that we don't accidentally initialize the rare data for this node
            // by setting it to None
            if node.containing_shadow_root().is_some() {
                // Reset the containing shadowRoot after we unbind the node, since some elements
                // require the containing shadowRoot for cleanup logic (e.g. <style>).
                node.set_containing_shadow_root(None);
            }

            // If the element has a shadow root attached to it then we traverse that as well,
            // but without resetting the contained shadow root
            if let Some(shadow_root) = node.downcast::<Element>().and_then(Element::shadow_root) {
                for node in shadow_root
                    .upcast::<Node>()
                    .traverse_preorder(ShadowIncluding::Yes)
                {
                    cleanup_node(&node);
                }
            }
        }
    }

    /// Removes the given child from this node's list of children.
    ///
    /// Fails unless `child` is a child of this node.
    fn remove_child(&self, child: &Node, cached_index: Option<u32>, can_gc: CanGc) {