Attachment 346355 Details for Bug 188212 – Patch

[patch] Patch

bug-188212-20180802122125.patch (text/plain), 11.79 KB, created by Yusuke Suzuki on 2018-08-01 20:21:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-08-01 20:21:27 PDT

Size: 11.79 KB

patch

obsolete

>Subversion Revision: 234455
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 0e811c4c14d036830517799867114553a970dff6..f31b15946c88496db6153cbb8dd2934d2f137bfa 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-08-01  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Add self.queueMicrotask(f) on DOMWindow
>+        https://bugs.webkit.org/show_bug.cgi?id=188212
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * CMakeLists.txt:
>+        * runtime/JSJob.cpp:
>+        (JSC::createJSJob):
>+        Export them to WebCore.
>+
>+        (JSC::JSJobMicrotask::run):
>+        * runtime/JSJob.h:
>+        Add another version of JSJobMicrotask which does not have arguments.
>+
> 2018-07-31  Keith Miller  <keith_miller@apple.com>
> 
>         Unreviewed 32-bit build fix...
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index cae1db4e0edbd7eca3e7a9846623130a6211b0e6..770dbaf93ea38aa9a5aa7d0053a36077b0398f60 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-08-01  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Add self.queueMicrotask(f) on DOMWindow
>+        https://bugs.webkit.org/show_bug.cgi?id=188212
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adds self.queueMicrotask(f) in DOMWindow, which takes a function and enqueue it into microtask queue.
>+        We do not add this to Worker's global scope since our worker does not support microtasks correctly.
>+
>+        Tests: js/dom/queue-microtask-window.html
>+
>+        * bindings/js/JSDOMWindowCustom.cpp:
>+        (WebCore::JSDOMWindow::queueMicrotask):
>+        Post a microtask to JSC's microtask mechanism. This will eventually go to WebCore's MicrotaskQueue code.
>+
>+        * page/DOMWindow.idl:
>+
> 2018-08-01  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [CoordGraphics] Move CoordinatedBackingStore to WebCore
>diff --git a/Source/JavaScriptCore/CMakeLists.txt b/Source/JavaScriptCore/CMakeLists.txt
>index 349f17d07df2979a44faf051da30d86dc3f3ccd3..52875609dcae98ff4f12fd10a162068321cb4834 100644
>--- a/Source/JavaScriptCore/CMakeLists.txt
>+++ b/Source/JavaScriptCore/CMakeLists.txt
>@@ -794,6 +794,7 @@ set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS
>     runtime/JSGlobalObjectInlines.h
>     runtime/JSInternalPromise.h
>     runtime/JSInternalPromiseDeferred.h
>+    runtime/JSJob.h
>     runtime/JSLock.h
>     runtime/JSMap.h
>     runtime/JSMapIterator.h
>diff --git a/Source/JavaScriptCore/runtime/JSJob.cpp b/Source/JavaScriptCore/runtime/JSJob.cpp
>index 7ac663a48d0144a669e97333a55cb9da613675b9..83076edb1648202c7c2455bb5f83b64f37f4601a 100644
>--- a/Source/JavaScriptCore/runtime/JSJob.cpp
>+++ b/Source/JavaScriptCore/runtime/JSJob.cpp
>@@ -45,8 +45,9 @@ class JSJobMicrotask final : public Microtask {
>         m_arguments.set(vm, arguments);
>     }
> 
>-    virtual ~JSJobMicrotask()
>+    JSJobMicrotask(VM& vm, JSValue job)
>     {
>+        m_job.set(vm, job);
>     }
> 
> private:
>@@ -56,6 +57,11 @@ class JSJobMicrotask final : public Microtask {
>     Strong<JSArray> m_arguments;
> };
> 
>+Ref<Microtask> createJSJob(VM& vm, JSValue job)
>+{
>+    return adoptRef(*new JSJobMicrotask(vm, job));
>+}
>+
> Ref<Microtask> createJSJob(VM& vm, JSValue job, JSArray* arguments)
> {
>     return adoptRef(*new JSJobMicrotask(vm, job, arguments));
>@@ -71,13 +77,15 @@ void JSJobMicrotask::run(ExecState* exec)
>     ASSERT(handlerCallType != CallType::None);
> 
>     MarkedArgumentBuffer handlerArguments;
>-    for (unsigned index = 0, length = m_arguments->length(); index < length; ++index) {
>-        JSValue arg = m_arguments->JSArray::get(exec, index);
>-        CLEAR_AND_RETURN_IF_EXCEPTION(scope, handlerArguments.overflowCheckNotNeeded());
>-        handlerArguments.append(arg);
>+    if (m_arguments) {
>+        for (unsigned index = 0, length = m_arguments->length(); index < length; ++index) {
>+            JSValue arg = m_arguments->JSArray::get(exec, index);
>+            CLEAR_AND_RETURN_IF_EXCEPTION(scope, handlerArguments.overflowCheckNotNeeded());
>+            handlerArguments.append(arg);
>+        }
>+        if (UNLIKELY(handlerArguments.hasOverflowed()))
>+            return;
>     }
>-    if (UNLIKELY(handlerArguments.hasOverflowed()))
>-        return;
>     profiledCall(exec, ProfilingReason::Microtask, m_job.get(), handlerCallType, handlerCallData, jsUndefined(), handlerArguments);
>     scope.clearException();
> }
>diff --git a/Source/JavaScriptCore/runtime/JSJob.h b/Source/JavaScriptCore/runtime/JSJob.h
>index 0a9e636d9ee5de7e5c011731d48c55d5ea3b9229..3f879acde951809f82f37ccb1d5e75edbc413598 100644
>--- a/Source/JavaScriptCore/runtime/JSJob.h
>+++ b/Source/JavaScriptCore/runtime/JSJob.h
>@@ -33,6 +33,7 @@ namespace JSC {
> class Microtask;
> class JSArray;
> 
>-Ref<Microtask> createJSJob(VM&, JSValue job, JSArray* arguments);
>+JS_EXPORT_PRIVATE Ref<Microtask> createJSJob(VM&, JSValue job);
>+JS_EXPORT_PRIVATE Ref<Microtask> createJSJob(VM&, JSValue job, JSArray* arguments);
> 
> } // namespace JSC
>diff --git a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>index 1405015cf13fdfe653b0fd46711b87f3f509c5b2..c46a38120475e383fa0f0d978aae500c8d9adb01 100644
>--- a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>@@ -46,6 +46,7 @@
> #include "Settings.h"
> #include "WebCoreJSClientData.h"
> #include <JavaScriptCore/JSCInlines.h>
>+#include <JavaScriptCore/JSJob.h>
> #include <JavaScriptCore/Lookup.h>
> 
> #if ENABLE(USER_MESSAGE_HANDLERS)
>@@ -521,6 +522,23 @@ JSValue JSDOMWindow::showModalDialog(ExecState& state)
>     return handler.returnValue();
> }
> 
>+JSValue JSDOMWindow::queueMicrotask(ExecState& state)
>+{
>+    VM& vm = state.vm();
>+    auto scope = DECLARE_THROW_SCOPE(vm);
>+
>+    if (UNLIKELY(state.argumentCount() < 1))
>+        return throwException(&state, scope, createNotEnoughArgumentsError(&state));
>+
>+    JSValue functionValue = state.uncheckedArgument(0);
>+    if (UNLIKELY(!functionValue.isFunction(vm)))
>+        return JSValue::decode(throwArgumentMustBeFunctionError(state, scope, 0, "callback", "Window", "queueMicrotask"));
>+
>+    scope.release();
>+    Base::queueMicrotask(JSC::createJSJob(vm, functionValue));
>+    return jsUndefined();
>+}
>+
> DOMWindow* JSDOMWindow::toWrapped(VM& vm, JSValue value)
> {
>     if (!value.isObject())
>diff --git a/Source/WebCore/page/DOMWindow.idl b/Source/WebCore/page/DOMWindow.idl
>index 14c392d343ec6025da155d75509c31a84c0a68d7..ff4d9575290b7643c7b99705d745ba9f2bbdba35 100644
>--- a/Source/WebCore/page/DOMWindow.idl
>+++ b/Source/WebCore/page/DOMWindow.idl
>@@ -89,6 +89,8 @@ typedef USVString CSSOMString;
>     DOMString? prompt(optional DOMString message = "", optional DOMString defaultValue = "");
>     void print();
> 
>+    [Custom] void queueMicrotask(VoidCallback callback);
>+
>     long requestAnimationFrame(RequestAnimationFrameCallback callback); // FIXME: Should return an unsigned long.
>     void cancelAnimationFrame(long handle); // FIXME: handle should be an unsigned long.
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 22e95870bae2806684ef92d0f20b005b1cb2a817..19bfe88250895fad4bf8c9abcadfc52ec85a38cd 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-01  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Add self.queueMicrotask(f) on DOMWindow
>+        https://bugs.webkit.org/show_bug.cgi?id=188212
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * js/dom/queue-microtask-window-expected.txt: Added.
>+        * js/dom/queue-microtask-window.html: Added.
>+
> 2018-07-31  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         [WinCairo] Unreviewed test gardening.
>diff --git a/LayoutTests/js/dom/queue-microtask-window-expected.txt b/LayoutTests/js/dom/queue-microtask-window-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..cc06f58ccb84fdcf7324071bc04d06d30d3bafad
>--- /dev/null
>+++ b/LayoutTests/js/dom/queue-microtask-window-expected.txt
>@@ -0,0 +1,7 @@
>+
>+
>+PASS Queued microtasks should be drained before executing macrotasks 
>+PASS queueMicrotask's callback has zero arguments and self as |this| 
>+PASS queueMicrotask and Promise uses the same Microtask queue 
>+PASS queueMicrotask should reject non-function arguments 
>+
>diff --git a/LayoutTests/js/dom/queue-microtask-window.html b/LayoutTests/js/dom/queue-microtask-window.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..4b5e4f9453b51e63669957c60c5f1056a02ce8d4
>--- /dev/null
>+++ b/LayoutTests/js/dom/queue-microtask-window.html
>@@ -0,0 +1,110 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>self.queueMicrotask works in Window</title>
>+<script src="../../resources/testharness.js"></script>
>+<script src="../../resources/testharnessreport.js"></script>
>+</head>
>+<body>
>+<iframe></iframe>
>+<script>
>+promise_test(() => {
>+    var counter = 0;
>+    self.queueMicrotask(() => {
>+        assert_equals(counter++, 1);
>+        self.queueMicrotask(() => {
>+            assert_equals(counter++, 2);
>+        });
>+    });
>+    var promise = new Promise((resolve, reject) => {
>+        setTimeout(() => {
>+            assert_equals(counter++, 3);
>+            resolve();
>+        }, 0);
>+    });
>+    assert_equals(counter++, 0);
>+    return promise;
>+}, `Queued microtasks should be drained before executing macrotasks`);
>+
>+promise_test(() => {
>+    return new Promise((resolve, reject) => {
>+        self.queueMicrotask(function () {
>+            try {
>+                assert_equals(arguments.length, 0);
>+                assert_equals(this, self);
>+                assert_equals(frames.length, 1);
>+                assert_not_equals(frames[0], self);
>+                var iframe = frames[0];
>+                iframe.queueMicrotask(function () {
>+                    try {
>+                        assert_equals(this, self);
>+                        assert_not_equals(this, iframe);
>+                        self.queueMicrotask(function () {
>+                            'use strict';
>+                            try {
>+                                assert_equals(this, undefined);
>+                                resolve();
>+                            } catch (e) {
>+                                reject(e);
>+                            }
>+                        });
>+                    } catch (e) {
>+                        reject(e);
>+                    }
>+                });
>+            } catch (e) {
>+                reject(e);
>+            }
>+        });
>+    });
>+}, `queueMicrotask's callback has zero arguments and self as |this|`);
>+
>+promise_test(() => {
>+    return new Promise((resolve ,reject) => {
>+        var counter = 0;
>+        Promise.resolve().then(() => {
>+            assert_equals(counter++, 1);
>+            self.queueMicrotask(() => {
>+                assert_equals(counter++, 3);
>+                resolve();
>+            });
>+        });
>+        self.queueMicrotask(() => {
>+            assert_equals(counter++, 2);
>+        });
>+        assert_equals(counter++, 0);
>+    });
>+}, `queueMicrotask and Promise uses the same Microtask queue`);
>+
>+test(() => {
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask();
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask(null);
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask(undefined);
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask(42);
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask("42");
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask(true);
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask(Symbol("42"));
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask({});
>+    });
>+    assert_throws(new TypeError, () => {
>+        self.queueMicrotask({ handleEvent() { } });
>+    });
>+}, `queueMicrotask should reject non-function arguments`);
>+</script>
>+</body>
>+</html>

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188212: 346275 | 346353 | 346354 | 346355