-
Notifications
You must be signed in to change notification settings - Fork 77
Description
Description
We have observed persistent issues with RemoteMutationObserver when used in combination with the iframe sandbox. Specifically, problems arise when mutation records contain interleaving removals and additions of nodes, leading to incorrect behavior where nodes are not properly added or removed. Additionally, under certain conditions, nodes may be inserted multiple times on the host side.
Steps to Reproduce
A reproducible scenario for the index issue can be found in the kitchen-sink demo app of this fork, where a mutation replaces two child nodes with two new child nodes.
Initial DOM State
<ui-stack>
<ui-text>Loading 1</ui-text>
<ui-text>Loading 2</ui-text>
</ui-stack>After State Update
<ui-stack>
<ui-button>Button 1</ui-button>
<ui-button>Button 2</ui-button>
</ui-stack>Detected Mutations by MutationObserver
| Action | Previous Sibling | Node | Next Sibling |
|---|---|---|---|
| Remove | null | <ui-text>Loading 1</ui-text> |
<ui-text>Loading 2</ui-text> |
| Insert | <ui-text>Loading 2</ui-text> |
<ui-button>Button 1</ui-button> |
null |
| Remove | null | <ui-text>Loading 2</ui-text> |
<ui-button>Button 1</ui-button> |
| Insert | <ui-button>Button 1</ui-button> |
<ui-button>Button 2</ui-button> |
null |
Mutation Records Generated by RemoteMutationObserver
| Action | Node | Index |
|---|---|---|
| Remove | null | 0 |
| Insert | <ui-button>Button 1</ui-button> |
0 |
| Remove | null | 0 |
| Insert | <ui-button>Button 2</ui-button> |
1 |
Incorrect Final DOM State
<ui-stack>
<ui-button>Loading 2</ui-button>
<ui-button>Button 2</ui-button>
</ui-stack>In this state, <ui-text>Loading 2</ui-text> was incorrectly retained instead of being removed, and <ui-button>Button 1</ui-button> is missing.
A small side node
Prior to this bugfix, the second example here even caused a crash, as described in this issue. However, this fix merely acts as a temporary patch rather than addressing the root problem. I even would suggest to be more strict (throwing errors) when invariants are not met, like a node that could not be found in the receivers lookup registries. This is how errors in the algorithm or logic are first noticed and subsequent errors are avoided.
Root Cause Analysis
1. Unreliable Index Calculation
- The issue stems from how
RemoteMutationObserverdetermines the index of inserted and removed nodes. - When processing multiple mutation records within the same mutation event, the index calculation is unreliable.
- This is because the mutation event occurs after the mutation has already been applied.
- The removed node
<ui-text>Loading 2</ui-text>no longer exists in the parent’s child list at the time of index determination. - As a result, the algorithm incorrectly assumes an index of
0, leading to incorrect mutations.
2. Duplicate Insertions
- In rare cases, nodes are inserted multiple times on the host side.
- This typically occurs when replacing a node with a subtree of deeply nested nodes.
- Although
RemoteMutationObservertries to avoid duplicate insertions usingaddedNode.contains(node), this check is insufficient because it only detects direct children, not deep descendants.
Expected Behavior
RemoteMutationObserver should ensure correct node insertions and removals, even when multiple mutation records are involved or deeply nested structures are inserted. It should also prevent duplicate insertions of the same node.
Proposed Solution
1. Use Reference Nodes Instead of Indices
A more reliable approach is to replace index-based positioning with remote IDs of reference nodes:
- For removals: The removed node itself serves as the reference.
- For insertions: The reference should be the next or previous sibling node, with the next sibling recommended, as this simplifies processing on the receiving end:
- If the successor is
null, the node should be appended. - Otherwise, the node should be inserted before the reference.
- If the successor is
2. Prevent Duplicate Insertions on the Host Side
- Rather than implementing recursive descendant checks in
RemoteMutationObserver(which would be costly), the host/receiver now checks before inserting a node whether it already exists under the expected parent. - Since the parent is known, this check is efficient and avoids unnecessary overhead.
This combination ensures both correctness and performance across a range of complex mutation scenarios.