Skip to content

Commit

Permalink
Safe slot reassigment
Browse files Browse the repository at this point in the history
1. Use GetWithoutInvalidation() instead of Get() in DCHECKs.
We should never call Get() inside of a DCHECK(), because this can
lead to a different code path depending on whether DCHECKs are enabled.

2. Get() should not cause immediate side effects. At most, it should
queue up an invalidation for later processing.

Fixing #1 and #2 were required in order to get past a first set of
errors introduced by the new test.

3. The actual fix -- avoid infinite loop by calling a special
new SlotAssignmentWillChange(), rather than ChildrenChanged(),
where a minimal GetWithoutInvalidation() is called that does not
lead to IsShadowContentRelevantForAccessibility() => FirstChild() =>
RecalcAssignedNodes() => ChildrenChanged() ... (infinite loop).

A simpler potential fix is in CL:2965317 but requires more
research. It's also mentioned in a TODO comment.

Bug: 1219311
Change-Id: Iafaa289f241a851404ce352715d2970172a2e5f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961158
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#892778}
  • Loading branch information
aleventhal authored and chromium-wpt-export-bot committed Jun 15, 2021
1 parent f93078f commit 7b9ca7d
Showing 1 changed file with 34 additions and 0 deletions.
34 changes: 34 additions & 0 deletions accessibility/crashtests/slot-assignment-lockup.html
@@ -0,0 +1,34 @@
<html class="test-wait">
<body>
<script>
customElements.define("my-detail", class extends HTMLElement {
constructor() {
super();
this.attachShadow({ mode: "open", slotAssignment: "manual" });
}
connectedCallback() {
const slot1 = document.createElement("slot");
const child1 = document.createElement("span");
this.appendChild(child1);
child1.innerHTML = "x";
this.shadowRoot.appendChild(slot1);
slot1.style.display = "block";
slot1.assign(child1);
}
});

function fuzz() {
document.designMode = 'on';
document.execCommand("selectAll");
document.execCommand("InsertText");
document.documentElement.className = '';
}
window.onload = () => {
requestAnimationFrame(() => {
requestAnimationFrame(fuzz);
});
};
</script>
<my-detail></my-detail>
</body>
</html>

0 comments on commit 7b9ca7d

Please sign in to comment.