Occasional crash in CollectionIndexCache::~CollectionIndexCache while running moveBefore tests.

[rniwa]

958ef3d

Occasional crash in CollectionIndexCache::~CollectionIndexCache while running moveBefore tests.
https://bugs.webkit.org/show_bug.cgi?id=315034
rdar://177337493

Reviewed by Simon Fraser.

Notify the parent nodes for a child move so that things like HTMLCollectionCache gets invalidated.

No new tests since existing tests cover this scenario.

More tests progress with this change except script-move-before.html which regresses.

* LayoutTests/imported/w3c/web-platform-tests/dom/nodes/moveBefore/Node-moveBefore-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/dom/nodes/moveBefore/iframe-document-preserve.window-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/dom/nodes/moveBefore/moveBefore-name-map-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/dom/nodes/moveBefore/script-move-before-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/dom/nodes/moveBefore/slotchange-events-expected.txt:
* Source/WebCore/dom/ContainerNode.cpp:
(WebCore::ContainerNode::moveBefore):

Canonical link: https://commits.webkit.org/313444@main

a789fd4

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		❌ 🛠 vision-apple
	🧪 ios-wk2-wpt	✅ 🧪 api-mac-debug	✅ 🛠 gtk3-libwebrtc
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🛠 ios-safer-cpp	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim	✅ 🧪 mac-site-isolation
	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on current version of this PR (hash a789fd4)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		❌ 🛠 vision-apple
	🧪 ios-wk2-wpt	✅ 🧪 api-mac-debug	✅ 🛠 gtk3-libwebrtc
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🛠 ios-safer-cpp	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim	✅ 🧪 mac-site-isolation
	✅ 🛠 watch
	✅ 🛠 watch-sim

[lukewarlow]

Do you happen to know the iteration count that triggers this crash?

Either way this looks fine as a temporary fix I suspect in the end we don't want to call this and will handle it another way. (I might be wrong on that but the spec never fires these steps from my reading of it)

[rniwa]

Do you happen to know the iteration count that triggers this crash?

Either way this looks fine as a temporary fix I suspect in the end we don't want to call this and will handle it another way. (I might be wrong on that but the spec never fires these steps from my reading of it)

./Tools/Scripts/run-webkit-tests --debug --no-build --no-show-results --no-retry --exit-after-n-crashes-or-timeout 1 imported/w3c/web-platform-tests/dom/nodes/moveBefore/ --iterations 100

[rniwa]

Either way this looks fine as a temporary fix I suspect in the end we don't want to call this and will handle it another way. (I might be wrong on that but the spec never fires these steps from my reading of it)

The spec doesn't mention it because there is no equivalent step / concept in the spec. childrenChanged must be called on each parent whose child list has changed. We might want to add a new type for "move" but we absolutely must call it.

[lukewarlow]

Ah okay, I thought they were the same as https://dom.spec.whatwg.org/#concept-node-children-changed-ext

[rniwa]

Ah okay, I thought they were the same as https://dom.spec.whatwg.org/#concept-node-children-changed-ext

Oh, that's very confusing. We should probably rename childrenChanged to avoid confusion.

[smfr]

Is it OK that this PASS changes to a FAIL?

[rniwa]

Yes, we'd just need to follow up.

[webkit-commit-queue]

Committed 313444@main (958ef3d): https://commits.webkit.org/313444@main

Reviewed commits have been landed. Closing PR #65120 and removing active labels.