Perform microtask checkpoint after each task as spec'ed
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Dec 2019 05:22:03 +0000 (05:22 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Dec 2019 05:22:03 +0000 (05:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204546
<rdar://problem/57449333>

Reviewed by Chris Dumez.

Source/WebCore:

This patch makes EventLoop perform a microtask checkpoint after each task as spec'ed:
https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
and removes the timer in MicrotaskQueue.

Because we always perform a microtask checkpoint after executing JavaScript in
JSExecState::didLeaveScriptContext, there isn't a good way of testing this code change
from the existing API and the infrastructure. This patch, therefore, also introduces
internals.queueMicrotaskInTask, which schedules a new task within which a new microtask
to invoke the callback is scheduled.

This patch also stops including Microtasks.h from most places as only event loop
implementations need to access MicrotaskQueue object now after r252820.

Test: http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html

* animation/DocumentTimeline.cpp:
* animation/WebAnimation.cpp:
* bindings/js/JSExecState.cpp:
* dom/CustomElementReactionQueue.cpp:
* dom/Document.cpp:
* dom/EventLoop.cpp:
(WebCore::EventLoop::queueMicrotask):
(WebCore::EventLoop::run):
* dom/Microtasks.cpp:
(WebCore::MicrotaskQueue::MicrotaskQueue):
(WebCore::MicrotaskQueue::append):
(WebCore::MicrotaskQueue::timerFired): Deleted.
* dom/Microtasks.h:
* dom/MutationObserver.cpp:
* testing/Internals.cpp:
(WebCore::taskSourceFromString): Extracted from Internals::queueTask.
(WebCore::Internals::queueTask):
(WebCore::Internals::queueMicrotaskInTask): Added.
* testing/Internals.h:
* testing/Internals.idl:
* workers/WorkerGlobalScope.cpp:
* workers/service/SWClientConnection.cpp:

LayoutTests:

Added a regresion test using newly added internals methods.

* http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task-expected.txt: Added.
* http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html: Added.

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

18 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/WebAnimation.cpp
Source/WebCore/bindings/js/JSExecState.cpp
Source/WebCore/dom/CustomElementReactionQueue.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/EventLoop.cpp
Source/WebCore/dom/Microtasks.cpp
Source/WebCore/dom/Microtasks.h
Source/WebCore/dom/MutationObserver.cpp
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebCore/workers/WorkerGlobalScope.cpp
Source/WebCore/workers/service/SWClientConnection.cpp

index 2646878..b793f1e 100644 (file)
@@ -1,3 +1,16 @@
+2019-11-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perform microtask checkpoint after each task as spec'ed
+        https://bugs.webkit.org/show_bug.cgi?id=204546
+        <rdar://problem/57449333>
+
+        Reviewed by Chris Dumez.
+
+        Added a regresion test using newly added internals methods.
+
+        * http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task-expected.txt: Added.
+        * http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html: Added.
+
 2019-12-03  Simon Fraser  <simon.fraser@apple.com>
 
         Unreviewed test gardening.
diff --git a/LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task-expected.txt b/LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task-expected.txt
new file mode 100644 (file)
index 0000000..6db2c65
--- /dev/null
@@ -0,0 +1,10 @@
+A microtask checkpoint should be performed after executing each task.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS logs.join(", ") is "1.task, 2.microtask, 3.microtask, 4.task"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html b/LayoutTests/http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html
new file mode 100644 (file)
index 0000000..cf942da
--- /dev/null
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../resources/js-test-pre.js"></script>
+<script>
+
+description('A microtask checkpoint should be performed after executing each task.');
+
+const logs = [];
+
+if (!window.internals)
+    testFailed('This test requires internals');
+else {
+    jsTestIsAsync = true;
+
+    internals.queueTask('DOMManipulation', () => {
+        logs.push('1.task');
+    });
+    internals.queueTaskToQueueMicrotask('DOMManipulation', () => {
+        logs.push('2.microtask');
+    });
+    internals.queueTaskToQueueMicrotask('DOMManipulation', () => {
+        logs.push('3.microtask');
+    });
+    internals.queueTask('DOMManipulation', () => {
+        logs.push('4.task');
+    });
+
+    internals.queueTask('DOMManipulation', () => {
+        shouldBeEqualToString('logs.join(", ")', '1.task, 2.microtask, 3.microtask, 4.task');
+        finishJSTest();
+    });
+}
+
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index df2441b..db79f25 100644 (file)
@@ -1,3 +1,49 @@
+2019-11-22  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perform microtask checkpoint after each task as spec'ed
+        https://bugs.webkit.org/show_bug.cgi?id=204546
+        <rdar://problem/57449333>
+
+        Reviewed by Chris Dumez.
+
+        This patch makes EventLoop perform a microtask checkpoint after each task as spec'ed:
+        https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
+        and removes the timer in MicrotaskQueue.
+
+        Because we always perform a microtask checkpoint after executing JavaScript in
+        JSExecState::didLeaveScriptContext, there isn't a good way of testing this code change
+        from the existing API and the infrastructure. This patch, therefore, also introduces
+        internals.queueMicrotaskInTask, which schedules a new task within which a new microtask
+        to invoke the callback is scheduled.
+
+        This patch also stops including Microtasks.h from most places as only event loop
+        implementations need to access MicrotaskQueue object now after r252820.
+
+        Test: http/tests/eventloop/perform-microtask-checkpoint-at-end-of-task.html
+
+        * animation/DocumentTimeline.cpp:
+        * animation/WebAnimation.cpp:
+        * bindings/js/JSExecState.cpp:
+        * dom/CustomElementReactionQueue.cpp:
+        * dom/Document.cpp:
+        * dom/EventLoop.cpp:
+        (WebCore::EventLoop::queueMicrotask):
+        (WebCore::EventLoop::run):
+        * dom/Microtasks.cpp:
+        (WebCore::MicrotaskQueue::MicrotaskQueue):
+        (WebCore::MicrotaskQueue::append):
+        (WebCore::MicrotaskQueue::timerFired): Deleted.
+        * dom/Microtasks.h:
+        * dom/MutationObserver.cpp:
+        * testing/Internals.cpp:
+        (WebCore::taskSourceFromString): Extracted from Internals::queueTask.
+        (WebCore::Internals::queueTask):
+        (WebCore::Internals::queueMicrotaskInTask): Added.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        * workers/WorkerGlobalScope.cpp:
+        * workers/service/SWClientConnection.cpp:
+
 2019-12-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         Replace customJavaScriptUserAgentAsSiteSpecificQuirks with customUserAgentAsSiteSpecificQuirks
index 630c2fa..4fe9980 100644 (file)
@@ -36,7 +36,6 @@
 #include "EventNames.h"
 #include "GraphicsLayer.h"
 #include "KeyframeEffect.h"
-#include "Microtasks.h"
 #include "Node.h"
 #include "Page.h"
 #include "PseudoElement.h"
index 29b627e..515d369 100644 (file)
@@ -40,7 +40,6 @@
 #include "JSWebAnimation.h"
 #include "KeyframeEffect.h"
 #include "KeyframeEffectStack.h"
-#include "Microtasks.h"
 #include "RenderElement.h"
 #include "StyledElement.h"
 #include "WebAnimationUtilities.h"
index 0b3a490..93a2605 100644 (file)
@@ -27,7 +27,6 @@
 #include "JSExecState.h"
 
 #include "EventLoop.h"
-#include "Microtasks.h"
 #include "RejectedPromiseTracker.h"
 #include "ScriptExecutionContext.h"
 #include "ScriptState.h"
index 47309c7..f9aa9a7 100644 (file)
@@ -34,7 +34,6 @@
 #include "HTMLNames.h"
 #include "JSCustomElementInterface.h"
 #include "JSDOMBinding.h"
-#include "Microtasks.h"
 #include <JavaScriptCore/CatchScope.h>
 #include <JavaScriptCore/Heap.h>
 #include <wtf/NeverDestroyed.h>
index fba005e..abe4cfe 100644 (file)
 #include "MediaQueryMatcher.h"
 #include "MediaStream.h"
 #include "MessageEvent.h"
-#include "Microtasks.h"
 #include "MouseEventWithHitTestResults.h"
 #include "MutationEvent.h"
 #include "NameNodeList.h"
index eab6366..1d79fe8 100644 (file)
@@ -43,6 +43,7 @@ void EventLoop::queueMicrotask(std::unique_ptr<EventLoopTask>&& microtask)
 {
     ASSERT(microtask->taskSource() == TaskSource::Microtask);
     microtaskQueue().append(WTFMove(microtask));
+    scheduleToRunIfNeeded(); // FIXME: Remove this once everything is integrated with the event loop.
 }
 
 void EventLoop::performMicrotaskCheckpoint()
@@ -77,28 +78,35 @@ void EventLoop::scheduleToRunIfNeeded()
 void EventLoop::run()
 {
     m_isScheduledToRun = false;
-    if (m_tasks.isEmpty())
-        return;
-
-    auto tasks = std::exchange(m_tasks, { });
-    m_groupsWithSuspenedTasks.clear();
-    Vector<std::unique_ptr<EventLoopTask>> remainingTasks;
-    for (auto& task : tasks) {
-        auto* group = task->group();
-        if (!group || group->isStoppedPermanently())
-            continue;
-
-        if (group->isSuspended()) {
-            m_groupsWithSuspenedTasks.add(group);
-            remainingTasks.append(WTFMove(task));
-            continue;
+    bool didPerformMicrotaskCheckpoint = false;
+
+    if (!m_tasks.isEmpty()) {
+        auto tasks = std::exchange(m_tasks, { });
+        m_groupsWithSuspenedTasks.clear();
+        Vector<std::unique_ptr<EventLoopTask>> remainingTasks;
+        for (auto& task : tasks) {
+            auto* group = task->group();
+            if (!group || group->isStoppedPermanently())
+                continue;
+
+            if (group->isSuspended()) {
+                m_groupsWithSuspenedTasks.add(group);
+                remainingTasks.append(WTFMove(task));
+                continue;
+            }
+
+            task->execute();
+            didPerformMicrotaskCheckpoint = true;
+            microtaskQueue().performMicrotaskCheckpoint();
         }
-
-        task->execute();
+        for (auto& task : m_tasks)
+            remainingTasks.append(WTFMove(task));
+        m_tasks = WTFMove(remainingTasks);
     }
-    for (auto& task : m_tasks)
-        remainingTasks.append(WTFMove(task));
-    m_tasks = WTFMove(remainingTasks);
+
+    // FIXME: Remove this once everything is integrated with the event loop.
+    if (!didPerformMicrotaskCheckpoint)
+        microtaskQueue().performMicrotaskCheckpoint();
 }
 
 void EventLoop::clearAllTasks()
index 1299294..2e4aa4b 100644 (file)
@@ -33,7 +33,6 @@ namespace WebCore {
 
 MicrotaskQueue::MicrotaskQueue(JSC::VM& vm)
     : m_vm(makeRef(vm))
-    , m_timer(*this, &MicrotaskQueue::timerFired)
 {
 }
 
@@ -42,13 +41,6 @@ MicrotaskQueue::~MicrotaskQueue() = default;
 void MicrotaskQueue::append(std::unique_ptr<EventLoopTask>&& task)
 {
     m_microtaskQueue.append(WTFMove(task));
-
-    m_timer.startOneShot(0_s);
-}
-
-void MicrotaskQueue::timerFired()
-{
-    performMicrotaskCheckpoint();
 }
 
 void MicrotaskQueue::performMicrotaskCheckpoint()
index 3713bc9..90c7740 100644 (file)
@@ -21,7 +21,6 @@
 
 #pragma once
 
-#include "Timer.h"
 #include <wtf/Forward.h>
 #include <wtf/Vector.h>
 
@@ -32,7 +31,6 @@ class VM;
 namespace WebCore {
 
 class EventLoopTask;
-class ScriptExecutionContext;
 
 class MicrotaskQueue final {
     WTF_MAKE_FAST_ALLOCATED;
@@ -43,19 +41,13 @@ public:
     WEBCORE_EXPORT void append(std::unique_ptr<EventLoopTask>&&);
     WEBCORE_EXPORT void performMicrotaskCheckpoint();
 
-    JSC::VM& vm() const { return m_vm.get(); }
-
 private:
-    void timerFired();
+    JSC::VM& vm() const { return m_vm.get(); }
 
     bool m_performingMicrotaskCheckpoint { false };
     Vector<std::unique_ptr<EventLoopTask>> m_microtaskQueue;
     // For the main thread the VM lives forever. For workers it's lifetime is tied to our owning WorkerGlobalScope. Regardless, we retain the VM here to be safe.
     Ref<JSC::VM> m_vm;
-
-    // FIXME: Instead of a Timer, we should have a centralized Event Loop that calls performMicrotaskCheckpoint()
-    // on every iteration, implementing https://html.spec.whatwg.org/multipage/webappapis.html#processing-model-9
-    Timer m_timer;
 };
 
 } // namespace WebCore
index af0f71f..ebfee24 100644 (file)
@@ -38,7 +38,6 @@
 #include "GCReachableRef.h"
 #include "HTMLSlotElement.h"
 #include "InspectorInstrumentation.h"
-#include "Microtasks.h"
 #include "MutationCallback.h"
 #include "MutationObserverRegistration.h"
 #include "MutationRecord.h"
index 5285bde..1e3d8f4 100644 (file)
@@ -4650,21 +4650,43 @@ void Internals::postTask(RefPtr<VoidCallback>&& callback)
     });
 }
 
-ExceptionOr<void> Internals::queueTask(ScriptExecutionContext& context, const String& taskSourceName, RefPtr<VoidCallback>&& callback)
+static Optional<TaskSource> taskSourceFromString(const String& taskSourceName)
 {
-    TaskSource source;
     if (taskSourceName == "DOMManipulation")
-        source = TaskSource::DOMManipulation;
-    else
+        return TaskSource::DOMManipulation;
+    return WTF::nullopt;
+}
+
+ExceptionOr<void> Internals::queueTask(ScriptExecutionContext& context, const String& taskSourceName, RefPtr<VoidCallback>&& callback)
+{
+    auto source = taskSourceFromString(taskSourceName);
+    if (!source)
         return Exception { NotSupportedError };
 
-    context.eventLoop().queueTask(source, [callback = WTFMove(callback)]() {
+    context.eventLoop().queueTask(*source, [callback = WTFMove(callback)] {
         callback->handleEvent();
     });
 
     return { };
 }
 
+ExceptionOr<void> Internals::queueTaskToQueueMicrotask(Document& document, const String& taskSourceName, RefPtr<VoidCallback>&& callback)
+{
+    auto source = taskSourceFromString(taskSourceName);
+    if (!source)
+        return Exception { NotSupportedError };
+
+    ScriptExecutionContext& context = document; // This avoids unnecessarily exporting Document::eventLoop.
+    context.eventLoop().queueTask(*source, [movedCallback = WTFMove(callback), protectedDocument = makeRef(document)]() mutable {
+        ScriptExecutionContext& context = protectedDocument.get();
+        context.eventLoop().queueMicrotask([callback = WTFMove(movedCallback)] {
+            callback->handleEvent();
+        });
+    });
+
+    return { };
+}
+
 Vector<String> Internals::accessKeyModifiers() const
 {
     Vector<String> accessKeyModifierStrings;
index a9b1b7d..9c7d253 100644 (file)
@@ -776,6 +776,7 @@ public:
 
     void postTask(RefPtr<VoidCallback>&&);
     ExceptionOr<void> queueTask(ScriptExecutionContext&, const String& source, RefPtr<VoidCallback>&&);
+    ExceptionOr<void> queueTaskToQueueMicrotask(Document&, const String& source, RefPtr<VoidCallback>&&);
     void markContextAsInsecure();
 
     bool usingAppleInternalSDK() const;
index 5492170..0b7252d 100644 (file)
@@ -765,6 +765,7 @@ enum CompositingPolicy {
 
     void postTask(VoidCallback callback);
     [CallWith=ScriptExecutionContext, MayThrowException] void queueTask(DOMString source, VoidCallback callback);
+    [CallWith=Document] void queueTaskToQueueMicrotask(DOMString source, VoidCallback callback);
     void markContextAsInsecure();
 
     void setMaxCanvasPixelMemory(unsigned long size);
index e2d5bc5..d5b2000 100644 (file)
@@ -35,7 +35,6 @@
 #include "IDBConnectionProxy.h"
 #include "ImageBitmapOptions.h"
 #include "InspectorInstrumentation.h"
-#include "Microtasks.h"
 #include "Performance.h"
 #include "RuntimeEnabledFeatures.h"
 #include "ScheduledAction.h"
index 9d999c7..13fd990 100644 (file)
@@ -31,7 +31,6 @@
 #include "Document.h"
 #include "ExceptionData.h"
 #include "MessageEvent.h"
-#include "Microtasks.h"
 #include "SWContextManager.h"
 #include "ServiceWorkerContainer.h"
 #include "ServiceWorkerFetchResult.h"