Fine-grained child insert/remove invalidation based on ElementSelectorFlags - #802
Open
nicoburns wants to merge 10 commits into
Open
Fine-grained child insert/remove invalidation based on ElementSelectorFlags#802nicoburns wants to merge 10 commits into
nicoburns wants to merge 10 commits into
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the blanket parent
RestyleHint::restyle_subtree()on every child insertion/removal (and theTODO: make this fine grained / conditional based on ElementSelectorFlagscomments) with flag-driven invalidation mirroring Gecko'sRestyleManager::RestyleForInsertOrChange/ContentAppended/ContentWillBeRemoved, and fixes two invalidation gaps the new tests exposed.New
BaseDocument::restyle_for_child_insert_or_remove(parent_id, change_idx)(called after the child list is updated, with the insertion index or the removed child's old index):HAS_EMPTY_SELECTOR→restyle_for_empty_change: restyle the container's subtree, plus the container's later siblings when the grandparent hasHAS_SLOW_SELECTOR_LATER_SIBLINGS(:empty + E/:empty ~ E), mirroring Gecko'sRestyleForEmptyChangeHAS_SLOW_SELECTOR(:nth-last-childfamily) → singlerestyle_subtree()on the container, like Gecko'sRestyleWholeContainerHAS_SLOW_SELECTOR_LATER_SIBLINGS(:nth-childfamily) → restyle only element children at/afterchange_idx; a change at index 0 escalates to a whole-container restyle to avoid quadratic behavior when clearing a container from the start (Gecko'sContentWillBeRemovedoptimization)HAS_EDGE_CHILD_SELECTOR(:first/last/only-child) → restyle the first/last element children and the nearest element children on either side of the change (covers the previously-last child on append / previously-first on prepend, like Gecko'sMaybeRestyleForEdgeChildChange)Repeated mutations within one style flush are deduped: if the container already carries a pending
restyle_subtree()hint, the helper early-outs (the equivalent of Gecko'smRestyledAsWholeContainerset — Blitz can use the hint itself since it lives on the node and is cleared by the traversal).All
DocumentMutatorchild-list mutation paths route through it (add_children_to_parent,remove_node,remove_and_drop_node_with,remove_and_drop_all_children), which also fixes a pre-existing gap whereremove_nodeinserted damage but never posted a restyle hint, so removals could leave stale sibling styles. The blanket parent restyle inset_attributeis removed entirely: attribute changes can't shift sibling indices, and stylo's snapshot invalidation covers attribute-dependent selectors (matching Gecko, which does no container-level work for attribute changes).Two further fixes found while verifying against Gecko:
restyle_subtree()themselves — previously masked by the blanket parent restyle, a moved node kept styles matched against its old ancestors (e.g..red span).:emptytext handling:set_node_text/append_text_to_nodenow trigger the empty-change restyle when a child text node transitions empty↔non-empty (Gecko'sCharacterDataChangedpath), andis_emptynow ignores zero-length text nodes and comments per spec instead of treating any child as significant.Known conservative divergence from Gecko (correctness-preserving): the
:emptypath restyles unconditionally when the flag is set rather than checking was/is-empty first.Includes a new
child_invalidation.rstest suite (7 tests) covering:nth-child,:nth-last-child,:last-child,:empty(structural, sibling-combinator, and text-content cases) and moved-node restyling.Extracted from #800 so it can land first; the
:nth-child(An+B of S)work will be rebased on top.WPT results
No changes in test results compared to
main.Generated by the WPT workflow.
Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/65287500bfb04b2589b9fc6a08bc2d14
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/65287500bfb04b2589b9fc6a08bc2d14?variant=devin-insiders
Requested by: @nicoburns