imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2018 23:11:36 +0000 (23:11 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2018 23:11:36 +0000 (23:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167652

Reviewed by Saam Barati.

Source/WebCore:

The bug appears to be caused by the JS wrappers of slot elements getting prematurely collected.
Deployed GCReachableRef introduced in r236376 to fix the bug.

Test: fast/shadow-dom/signal-slot-list-retains-js-wrappers.html

* dom/MutationObserver.cpp:
(WebCore::signalSlotList):
(WebCore::MutationObserver::enqueueSlotChangeEvent):
(WebCore::MutationObserver::notifyMutationObservers):

LayoutTests:

Added a regression test for signaling a lot of slot elements.

* fast/shadow-dom/signal-slot-list-retains-js-wrappers-expected.txt: Added.
* fast/shadow-dom/signal-slot-list-retains-js-wrappers.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236440 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/MutationObserver.cpp

index 8814ae0..830e62c 100644 (file)
@@ -1,5 +1,17 @@
 2018-09-24  Ryosuke Niwa  <rniwa@webkit.org>
 
+        imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=167652
+
+        Reviewed by Saam Barati.
+
+        Added a regression test for signaling a lot of slot elements.
+
+        * fast/shadow-dom/signal-slot-list-retains-js-wrappers-expected.txt: Added.
+        * fast/shadow-dom/signal-slot-list-retains-js-wrappers.html: Added.
+
+2018-09-24  Ryosuke Niwa  <rniwa@webkit.org>
+
         Release assert when using paper-textarea due to autocorrect IDL attribute missing CEReactions
         https://bugs.webkit.org/show_bug.cgi?id=174629
         <rdar://problem/33407620>
diff --git a/LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers-expected.txt b/LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers-expected.txt
new file mode 100644 (file)
index 0000000..d9eaa16
--- /dev/null
@@ -0,0 +1,9 @@
+This tests signaling a slot change on lots of slot elements in a single microtask.
+WebKit should retain the JS wrappers of those elements and fire the slotchange event.
+
+PASS
+PASS
+PASS
+PASS
+PASS
+
diff --git a/LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers.html b/LayoutTests/fast/shadow-dom/signal-slot-list-retains-js-wrappers.html
new file mode 100644 (file)
index 0000000..54c7cfe
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests signaling a slot change on lots of slot elements in a single microtask.<br>
+WebKit should retain the JS wrappers of those elements and fire the slotchange event.</p>
+<pre id="log"></pre>
+<script>
+
+function triggerSlotChange(host, shadowRoot, slotchangeListener) {
+    shadowRoot.innerHTML = '<div><slot></slot></div>';
+    shadowRoot.querySelector('slot').addEventListener('slotchange', slotchangeListener);
+    host.innerHTML = 'hello';
+    shadowRoot.innerHTML = '';
+    host.innerHTML = '';
+}
+
+function runTest() {
+    let slotchangeCount = 0;
+    const iteration = window.GCController ? 5 : 500;
+    const host = document.createElement('div');
+    const shadowRoot = host.attachShadow({mode: 'closed'});
+    for (let i = 0; i < iteration; i++) {
+        triggerSlotChange(host, shadowRoot, () => slotchangeCount++);
+        if (window.GCController)
+            GCController.collect();
+        else {
+            const x = new Array(100);
+            for (let j = 0; j < 100; j++)
+                x[j] = new Array;
+        }
+    }
+    return new Promise((resolve) => {
+        setTimeout(() => {
+            log.textContent += slotchangeCount === iteration ? 'PASS' : `FAIL - expected ${iteration} slotchange events but got ${slotchangeCount}`;
+            log.textContent += '\n';
+            resolve();
+        }, 0);
+    });
+}
+
+const promises = [];
+const outerRuns = window.GCController ? 5 : 20;
+for (let i = 0; i < outerRuns; i++)
+    promises[i] = runTest();
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    Promise.all(promises).then(() => testRunner.notifyDone());
+}
+
+</script>
+</body>
+</html>
index 6e095f6..c1e5321 100644 (file)
@@ -1,5 +1,22 @@
 2018-09-24  Ryosuke Niwa  <rniwa@webkit.org>
 
+        imported/w3c/web-platform-tests/shadow-dom/slotchange.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=167652
+
+        Reviewed by Saam Barati.
+
+        The bug appears to be caused by the JS wrappers of slot elements getting prematurely collected.
+        Deployed GCReachableRef introduced in r236376 to fix the bug.
+
+        Test: fast/shadow-dom/signal-slot-list-retains-js-wrappers.html
+
+        * dom/MutationObserver.cpp:
+        (WebCore::signalSlotList):
+        (WebCore::MutationObserver::enqueueSlotChangeEvent):
+        (WebCore::MutationObserver::notifyMutationObservers):
+
+2018-09-24  Ryosuke Niwa  <rniwa@webkit.org>
+
         Release assert when using paper-textarea due to autocorrect IDL attribute missing CEReactions
         https://bugs.webkit.org/show_bug.cgi?id=174629
         <rdar://problem/33407620>
index 680c42a..1cbb09b 100644 (file)
@@ -34,6 +34,7 @@
 #include "MutationObserver.h"
 
 #include "Document.h"
+#include "GCReachableRef.h"
 #include "HTMLSlotElement.h"
 #include "Microtasks.h"
 #include "MutationCallback.h"
@@ -151,9 +152,9 @@ static MutationObserverSet& suspendedMutationObservers()
 }
 
 // https://dom.spec.whatwg.org/#signal-slot-list
-static Vector<RefPtr<HTMLSlotElement>>& signalSlotList()
+static Vector<GCReachableRef<HTMLSlotElement>>& signalSlotList()
 {
-    static NeverDestroyed<Vector<RefPtr<HTMLSlotElement>>> list;
+    static NeverDestroyed<Vector<GCReachableRef<HTMLSlotElement>>> list;
     return list;
 }
 
@@ -189,8 +190,8 @@ void MutationObserver::enqueueMutationRecord(Ref<MutationRecord>&& mutation)
 void MutationObserver::enqueueSlotChangeEvent(HTMLSlotElement& slot)
 {
     ASSERT(isMainThread());
-    ASSERT(!signalSlotList().contains(&slot));
-    signalSlotList().append(&slot);
+    ASSERT(signalSlotList().findMatching([&slot](auto& entry) { return entry.ptr() == &slot; }) == notFound);
+    signalSlotList().append(slot);
 
     queueMutationObserverCompoundMicrotask();
 }
@@ -273,7 +274,7 @@ void MutationObserver::notifyMutationObservers()
 
         // 3. Let signalList be a copy of unit of related similar-origin browsing contexts' signal slot list.
         // 4. Empty unit of related similar-origin browsing contexts' signal slot list.
-        Vector<RefPtr<HTMLSlotElement>> slotList;
+        Vector<GCReachableRef<HTMLSlotElement>> slotList;
         if (!signalSlotList().isEmpty()) {
             slotList.swap(signalSlotList());
             for (auto& slot : slotList)