Bug 218361 - JS Promises in detached iframes do not settle
Summary: JS Promises in detached iframes do not settle
Status: RESOLVED DUPLICATE of bug 218363
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-29 15:52 PDT by Chris Dumez
Modified: 2020-10-30 16:59 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-10-29 15:52:29 PDT
JS Promises in detached iframes do not settle:

===
<!doctype html>
<title>Test that the promise from detached iframes do get resolved.</title>
<body></body>
<script>
frame = document.createElement('iframe');
document.body.appendChild(frame);
frame.contentWindow.Promise.resolve("test").then(() => {
    alert("Resolved! URL is: " + document.URL);
});
frame.remove();
</script>
===

I see an alert in Chrome and Firefox (with the URL being the top document URL). No alert in Safari.

This is causing issues with our DOM API returning promises as well.
Comment 1 Keith Miller 2020-10-29 15:55:46 PDT
Do we check that the frame is still detached in the WebCore micro task queue? Seems likely that is the source of the problem here.
Comment 2 Keith Miller 2020-10-29 15:56:18 PDT
(In reply to Keith Miller from comment #1)
> Do we check that the frame is still detached in the WebCore micro task
> queue? Seems likely that is the source of the problem here.

/detached/attached/s
Comment 3 Chris Dumez 2020-10-29 16:06:48 PDT
Looks like this is the source of the issue:

diff --git a/Source/WebCore/dom/Microtasks.cpp b/Source/WebCore/dom/Microtasks.cpp
index 2e4aa4b56c10..73cb09642adf 100644
--- a/Source/WebCore/dom/Microtasks.cpp
+++ b/Source/WebCore/dom/Microtasks.cpp
@@ -56,7 +56,7 @@ void MicrotaskQueue::performMicrotaskCheckpoint()
         Vector<std::unique_ptr<EventLoopTask>> queue = WTFMove(m_microtaskQueue);
         for (auto& task : queue) {
             auto* group = task->group();
-            if (!group || group->isStoppedPermanently())
+            if (!group/* || group->isStoppedPermanently()*/)
                 continue;
             if (group->isSuspended())
                 toKeep.append(WTFMove(task));
Comment 4 Chris Dumez 2020-10-29 16:15:11 PDT
As Geoff pointed out on Slack:
"""
A browsing context group holds a browsing context set (a set of top-level browsing contexts).
A top-level browsing context is added to the group when the group is created. All subsequent top-level browsing contexts added to the group will be auxiliary browsing contexts.
"""

Seems wrong that we use different groups for documents in the same top-level browsing context.
Comment 5 Yusuke Suzuki 2020-10-29 16:19:45 PDT
Is it related to https://bugs.webkit.org/show_bug.cgi?id=216149 ?
Comment 6 Chris Dumez 2020-10-29 16:37:09 PDT
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index c9e3be11872a..6f4ffa18644b 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -2711,8 +2711,8 @@ void Document::resumeActiveDOMObjects(ReasonForSuspension why)
 
 void Document::stopActiveDOMObjects()
 {
-    if (m_documentTaskGroup)
-        m_documentTaskGroup->stopAndDiscardAllTasks();
+    /*if (m_documentTaskGroup)
+        m_documentTaskGroup->stopAndDiscardAllTasks();*/
     ScriptExecutionContext::stopActiveDOMObjects();
     platformSuspendOrStopActiveDOMObjects();
 }
diff --git a/Source/WebCore/dom/Microtasks.cpp b/Source/WebCore/dom/Microtasks.cpp
index 2e4aa4b56c10..73cb09642adf 100644
--- a/Source/WebCore/dom/Microtasks.cpp
+++ b/Source/WebCore/dom/Microtasks.cpp
@@ -56,7 +56,7 @@ void MicrotaskQueue::performMicrotaskCheckpoint()
         Vector<std::unique_ptr<EventLoopTask>> queue = WTFMove(m_microtaskQueue);
         for (auto& task : queue) {
             auto* group = task->group();
-            if (!group || group->isStoppedPermanently())
+            if (!group/* || group->isStoppedPermanently()*/)
                 continue;
             if (group->isSuspended())
                 toKeep.append(WTFMove(task));

Had to do both these changes to make WebAudio tests pass (Bug 218363).
Comment 7 Chris Dumez 2020-10-30 16:59:59 PDT

*** This bug has been marked as a duplicate of bug 218363 ***