Attachment 369669 Details for Bug 197172 – Patch

[patch] Patch

bug-197172-20190512000940.patch (text/plain), 17.70 KB, created by Yusuke Suzuki on 2019-05-12 00:09:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-05-12 00:09:41 PDT

Size: 17.70 KB

patch

obsolete

>Subversion Revision: 245204
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index b3ecb66cabd844cd19a132cc938e16ed48a62234..56c0a5d5c69282843f6c32c2275f12124d8886ee 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-05-12  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        Uncaught Promise Exceptions in JavaScriptCore
>+        https://bugs.webkit.org/show_bug.cgi?id=197172
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * runtime/JSGlobalObjectFunctions.cpp:
>+        (JSC::globalFuncHostPromiseRejectionTracker):
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        (JSC::VM::didMicrotaskQueueExhausted):
>+        (JSC::VM::drainMicrotasks):
>+        * runtime/VM.h:
>+
> 2019-05-10  Saam barati  <sbarati@apple.com>
> 
>         Call to JSToWasmICCallee::createStructure passes in wrong prototype value
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 08adb420af8088e38b126d1a77469a9b92bc60c4..62761c2d80e3ad91ba0bc11dc58b853d2049e002 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-05-12  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        Uncaught Promise Exceptions in JavaScriptCore
>+        https://bugs.webkit.org/show_bug.cgi?id=197172
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests (OOPS!).
>+
>+        * bindings/js/JSDOMGlobalObject.cpp:
>+        (WebCore::JSDOMGlobalObject::promiseRejectionTracker):
>+        * bindings/js/JSExecState.cpp:
>+        (WebCore::JSExecState::didLeaveScriptContext):
>+        * dom/RejectedPromiseTracker.cpp:
>+        (WebCore::RejectedPromiseTracker::RejectedPromiseTracker):
>+        (WebCore::RejectedPromiseTracker::promiseRejected):
>+        (WebCore::RejectedPromiseTracker::promiseHandled):
>+        (WebCore::RejectedPromiseTracker::processQueueSoon):
>+        (WebCore::RejectedPromiseTracker::reportUnhandledRejections):
>+        (WebCore::RejectedPromiseTracker::reportRejectionHandled):
>+        * dom/RejectedPromiseTracker.h:
>+        * dom/ScriptExecutionContext.cpp:
>+        (WebCore::ScriptExecutionContext::ensureRejectedPromiseTrackerSlow):
>+
> 2019-05-10  Youenn Fablet  <youenn@apple.com>
> 
>         A service worker instance should be terminated when its SWServer is destroyed
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp b/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>index 4c37285306798642418117be69a316b0fc15f29d..628f4d972175fa3614a1646f38cb188369ee164b 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>@@ -761,10 +761,12 @@ EncodedJSValue JSC_HOST_CALL globalFuncHostPromiseRejectionTracker(ExecState* ex
>     VM& vm = globalObject->vm();
>     auto scope = DECLARE_THROW_SCOPE(vm);
> 
>-    if (!globalObject->globalObjectMethodTable()->promiseRejectionTracker)
>+    JSPromise* promise = jsCast<JSPromise*>(exec->argument(0));
>+
>+    // InternalPromises should not be exposed to user scripts.
>+    if (promise->inherits<JSInternalPromise>(vm))
>         return JSValue::encode(jsUndefined());
> 
>-    JSPromise* promise = jsCast<JSPromise*>(exec->argument(0));
>     JSValue operationValue = exec->argument(1);
> 
>     ASSERT(operationValue.isNumber());
>@@ -772,8 +774,32 @@ EncodedJSValue JSC_HOST_CALL globalFuncHostPromiseRejectionTracker(ExecState* ex
>     ASSERT(operation == JSPromiseRejectionOperation::Reject || operation == JSPromiseRejectionOperation::Handle);
>     scope.assertNoException();
> 
>-    globalObject->globalObjectMethodTable()->promiseRejectionTracker(globalObject, exec, promise, operation);
>-    RETURN_IF_EXCEPTION(scope, { });
>+    if (globalObject->globalObjectMethodTable()->promiseRejectionTracker) {
>+        globalObject->globalObjectMethodTable()->promiseRejectionTracker(globalObject, exec, promise, operation);
>+        RETURN_IF_EXCEPTION(scope, { });
>+    } else {
>+        ([&] {
>+            switch (operation) {
>+            case JSPromiseRejectionOperation::Reject: {
>+                vm.m_aboutToBeNotifiedRejectedPromises.constructAndAppend(vm, promise);
>+                return;
>+            }
>+            case JSPromiseRejectionOperation::Handle: {
>+                bool removed = vm.m_aboutToBeNotifiedRejectedPromises.removeFirstMatching([&] (Strong<JSPromise>& unhandledPromise) {
>+                    return unhandledPromise.get() == promise;
>+                });
>+                if (removed)
>+                    return;
>+
>+                if (!vm.m_outstandingRejectedPromises.remove(promise))
>+                    return;
>+
>+                vm.m_rejectedPromises.constructAndAppend(vm, promise);
>+                return;
>+            }
>+            }
>+        }());
>+    }
> 
>     return JSValue::encode(jsUndefined());
> }
>diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
>index a630116cc2ca027404b00180f0a6549a8674ec8b..05eed131b650587b160c35733556c078d359cc19 100644
>--- a/Source/JavaScriptCore/runtime/VM.cpp
>+++ b/Source/JavaScriptCore/runtime/VM.cpp
>@@ -321,6 +321,7 @@ VM::VM(VMType vmType, HeapType heapType)
>     , m_typeProfilerEnabledCount(0)
>     , m_primitiveGigacageEnabled(IsWatched)
>     , m_controlFlowProfilerEnabledCount(0)
>+    , m_outstandingRejectedPromises(*this)
> {
>     if (UNLIKELY(vmCreationShouldCrash))
>         CRASH_WITH_INFO(0x4242424220202020, 0xbadbeef0badbeef, 0x1234123412341234, 0x1337133713371337);
>@@ -1095,15 +1096,40 @@ void VM::queueMicrotask(JSGlobalObject& globalObject, Ref<Microtask>&& task)
>     m_microtaskQueue.append(std::make_unique<QueuedTask>(*this, &globalObject, WTFMove(task)));
> }
> 
>-void VM::drainMicrotasks()
>+
>+inline void VM::didMicrotaskQueueExhausted()
> {
>-    while (!m_microtaskQueue.isEmpty()) {
>-        m_microtaskQueue.takeFirst()->run();
>-        if (m_onEachMicrotaskTick)
>-            m_onEachMicrotaskTick(*this);
>+    auto rejectedPromises = WTFMove(m_rejectedPromises);
>+    for (auto& promise : rejectedPromises)
>+        dataLogLn(JSValue(promise.get()));
>+
>+    auto unhandledPromises = WTFMove(m_aboutToBeNotifiedRejectedPromises);
>+    for (auto& promise : unhandledPromises) {
>+        ExecState* state = promise->globalObject()->globalExec();
>+        if (promise->isHandled(*this))
>+            continue;
>+
>+        dataLogLn(JSValue(promise.get()));
>+
>+        // m_context.reportUnhandledPromiseRejection(state, promise, unhandledPromise.callStack());
>+
>+        if (!promise->isHandled(*this))
>+            m_outstandingRejectedPromises.set(promise.get(), promise.get());
>     }
> }
> 
>+void VM::drainMicrotasks()
>+{
>+    do {
>+        while (!m_microtaskQueue.isEmpty()) {
>+            m_microtaskQueue.takeFirst()->run();
>+            if (m_onEachMicrotaskTick)
>+                m_onEachMicrotaskTick(*this);
>+        }
>+        didMicrotaskQueueExhausted();
>+    } while (!m_microtaskQueue.isEmpty());
>+}
>+
> void QueuedTask::run()
> {
>     m_microtask->run(m_globalObject->globalExec());
>diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
>index b985d47ea4d95edddc9c0cf36689653376f7d1df..006ae69c0f381a8341df60d976fe93c64287f146 100644
>--- a/Source/JavaScriptCore/runtime/VM.h
>+++ b/Source/JavaScriptCore/runtime/VM.h
>@@ -123,6 +123,7 @@ class JSCustomGetterSetterFunction;
> class JSDestructibleObjectHeapCellType;
> class JSGlobalObject;
> class JSObject;
>+class JSPromise;
> class JSRunLoopTimer;
> class JSStringHeapCellType;
> class JSWebAssemblyCodeBlockHeapCellType;
>@@ -807,6 +808,10 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>     ALWAYS_INLINE HasOwnPropertyCache* hasOwnPropertyCache() { return m_hasOwnPropertyCache.get(); }
>     HasOwnPropertyCache* ensureHasOwnPropertyCache();
> 
>+    Vector<Strong<JSPromise>> m_aboutToBeNotifiedRejectedPromises;
>+    Vector<Strong<JSPromise>> m_rejectedPromises;
>+    WeakGCMap<JSPromise*, JSPromise> m_outstandingRejectedPromises;
>+
> #if ENABLE(REGEXP_TRACING)
>     typedef ListHashSet<RegExp*> RTTraceList;
>     RTTraceList* m_rtTraceList;
>@@ -981,6 +986,8 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>     static void primitiveGigacageDisabledCallback(void*);
>     void primitiveGigacageDisabled();
> 
>+    void didMicrotaskQueueExhausted();
>+
> #if ENABLE(GC_VALIDATION)
>     const ClassInfo* m_initializingObjectClass;
> #endif
>diff --git a/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp b/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp
>index 788e2e677f7f20cd39d21712cf867460b56f5b4b..628fbd680d8ae2f3ee880f3c3e390193a4e53f66 100644
>--- a/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMGlobalObject.cpp
>@@ -222,19 +222,15 @@ void JSDOMGlobalObject::promiseRejectionTracker(JSGlobalObject* jsGlobalObject,
>     if (!context)
>         return;
> 
>-    // InternalPromises should not be exposed to user scripts.
>-    if (JSC::jsDynamicCast<JSC::JSInternalPromise*>(vm, promise))
>-        return;
>-
>     // FIXME: If script has muted errors (cross origin), terminate these steps.
>     // <https://webkit.org/b/171415> Implement the `muted-errors` property of Scripts to avoid onerror/onunhandledrejection for cross-origin scripts
> 
>     switch (operation) {
>     case JSPromiseRejectionOperation::Reject:
>-        context->ensureRejectedPromiseTracker().promiseRejected(*exec, globalObject, *promise);
>+        context->ensureRejectedPromiseTracker().promiseRejected(*context, *exec, globalObject, *promise);
>         break;
>     case JSPromiseRejectionOperation::Handle:
>-        context->ensureRejectedPromiseTracker().promiseHandled(*exec, globalObject, *promise);
>+        context->ensureRejectedPromiseTracker().promiseHandled(*context, *exec, globalObject, *promise);
>         break;
>     }
> }
>diff --git a/Source/WebCore/bindings/js/JSExecState.cpp b/Source/WebCore/bindings/js/JSExecState.cpp
>index 20dd255816c0f72d1b9a63f5faf387bd0e1a987a..40954c47fe95563c13b4604fee502913dab775a6 100644
>--- a/Source/WebCore/bindings/js/JSExecState.cpp
>+++ b/Source/WebCore/bindings/js/JSExecState.cpp
>@@ -40,7 +40,7 @@ void JSExecState::didLeaveScriptContext(JSC::ExecState* exec)
>     if (!context)
>         return;
>     MicrotaskQueue::contextQueue(*context).performMicrotaskCheckpoint();
>-    context->ensureRejectedPromiseTracker().processQueueSoon();
>+    context->ensureRejectedPromiseTracker().processQueueSoon(*context);
> }
> 
> JSC::JSValue functionCallHandlerFromAnyThread(JSC::ExecState* exec, JSC::JSValue functionObject, JSC::CallType callType, const JSC::CallData& callData, JSC::JSValue thisValue, const JSC::ArgList& args, NakedPtr<JSC::Exception>& returnedException)
>diff --git a/Source/WebCore/dom/RejectedPromiseTracker.cpp b/Source/WebCore/dom/RejectedPromiseTracker.cpp
>index ab77306f27f336c7e4e89ff8821e59713ce79805..5f5be0ff5b3124980c67702fe0e65f25a42ed4af 100644
>--- a/Source/WebCore/dom/RejectedPromiseTracker.cpp
>+++ b/Source/WebCore/dom/RejectedPromiseTracker.cpp
>@@ -77,9 +77,8 @@ class UnhandledPromise {
> };
> 
> 
>-RejectedPromiseTracker::RejectedPromiseTracker(ScriptExecutionContext& context, JSC::VM& vm)
>-    : m_context(context)
>-    , m_outstandingRejectedPromises(vm)
>+RejectedPromiseTracker::RejectedPromiseTracker(JSC::VM& vm)
>+    : m_outstandingRejectedPromises(vm)
> {
> }
> 
>@@ -102,7 +101,7 @@ static RefPtr<ScriptCallStack> createScriptCallStackFromReason(ExecState& state,
>     return nullptr;
> }
> 
>-void RejectedPromiseTracker::promiseRejected(ExecState& state, JSDOMGlobalObject& globalObject, JSPromise& promise)
>+void RejectedPromiseTracker::promiseRejected(ScriptExecutionContext& context, ExecState& state, JSDOMGlobalObject& globalObject, JSPromise& promise)
> {
>     // https://html.spec.whatwg.org/multipage/webappapis.html#the-hostpromiserejectiontracker-implementation
> 
>@@ -110,7 +109,7 @@ void RejectedPromiseTracker::promiseRejected(ExecState& state, JSDOMGlobalObject
>     m_aboutToBeNotifiedRejectedPromises.append(UnhandledPromise { globalObject, promise, createScriptCallStackFromReason(state, reason) });
> }
> 
>-void RejectedPromiseTracker::promiseHandled(ExecState&, JSDOMGlobalObject& globalObject, JSPromise& promise)
>+void RejectedPromiseTracker::promiseHandled(ScriptExecutionContext& context, ExecState&, JSDOMGlobalObject& globalObject, JSPromise& promise)
> {
>     // https://html.spec.whatwg.org/multipage/webappapis.html#the-hostpromiserejectiontracker-implementation
> 
>@@ -126,12 +125,12 @@ void RejectedPromiseTracker::promiseHandled(ExecState&, JSDOMGlobalObject& globa
>     if (!m_outstandingRejectedPromises.remove(&promise))
>         return;
> 
>-    m_context.postTask([this, rejectedPromise = DOMPromise::create(globalObject, promise)] (ScriptExecutionContext&) mutable {
>-        reportRejectionHandled(WTFMove(rejectedPromise));
>+    context.postTask([this, rejectedPromise = DOMPromise::create(globalObject, promise)] (ScriptExecutionContext& context) mutable {
>+        reportRejectionHandled(context, WTFMove(rejectedPromise));
>     });
> }
> 
>-void RejectedPromiseTracker::processQueueSoon()
>+void RejectedPromiseTracker::processQueueSoon(ScriptExecutionContext& context)
> {
>     // https://html.spec.whatwg.org/multipage/webappapis.html#notify-about-rejected-promises
> 
>@@ -139,16 +138,16 @@ void RejectedPromiseTracker::processQueueSoon()
>         return;
> 
>     Vector<UnhandledPromise> items = WTFMove(m_aboutToBeNotifiedRejectedPromises);
>-    m_context.postTask([this, items = WTFMove(items)] (ScriptExecutionContext&) mutable {
>-        reportUnhandledRejections(WTFMove(items));
>+    context.postTask([this, items = WTFMove(items)] (ScriptExecutionContext& context) mutable {
>+        reportUnhandledRejections(context, WTFMove(items));
>     });
> }
> 
>-void RejectedPromiseTracker::reportUnhandledRejections(Vector<UnhandledPromise>&& unhandledPromises)
>+void RejectedPromiseTracker::reportUnhandledRejections(ScriptExecutionContext& context, Vector<UnhandledPromise>&& unhandledPromises)
> {
>     // https://html.spec.whatwg.org/multipage/webappapis.html#unhandled-promise-rejections
> 
>-    VM& vm = m_context.vm();
>+    VM& vm = context.vm();
>     JSC::JSLockHolder lock(vm);
> 
>     for (auto& unhandledPromise : unhandledPromises) {
>@@ -167,22 +166,22 @@ void RejectedPromiseTracker::reportUnhandledRejections(Vector<UnhandledPromise>&
>         initializer.reason = promise.result(vm);
> 
>         auto event = PromiseRejectionEvent::create(eventNames().unhandledrejectionEvent, initializer);
>-        auto target = m_context.errorEventTarget();
>+        auto target = context.errorEventTarget();
>         target->dispatchEvent(event);
> 
>         if (!event->defaultPrevented())
>-            m_context.reportUnhandledPromiseRejection(state, promise, unhandledPromise.callStack());
>+            context.reportUnhandledPromiseRejection(state, promise, unhandledPromise.callStack());
> 
>         if (!promise.isHandled(vm))
>             m_outstandingRejectedPromises.set(&promise, &promise);
>     }
> }
> 
>-void RejectedPromiseTracker::reportRejectionHandled(Ref<DOMPromise>&& rejectedPromise)
>+void RejectedPromiseTracker::reportRejectionHandled(ScriptExecutionContext& context, Ref<DOMPromise>&& rejectedPromise)
> {
>     // https://html.spec.whatwg.org/multipage/webappapis.html#the-hostpromiserejectiontracker-implementation
> 
>-    VM& vm = m_context.vm();
>+    VM& vm = context.vm();
>     JSC::JSLockHolder lock(vm);
> 
>     if (rejectedPromise->isSuspended())
>@@ -195,7 +194,7 @@ void RejectedPromiseTracker::reportRejectionHandled(Ref<DOMPromise>&& rejectedPr
>     initializer.reason = promise.result(vm);
> 
>     auto event = PromiseRejectionEvent::create(eventNames().rejectionhandledEvent, initializer);
>-    auto target = m_context.errorEventTarget();
>+    auto target = context.errorEventTarget();
>     target->dispatchEvent(event);
> }
> 
>diff --git a/Source/WebCore/dom/RejectedPromiseTracker.h b/Source/WebCore/dom/RejectedPromiseTracker.h
>index 87fd556bfc585b97820214cc944adab0c1db04e4..ca03ab96002acbee25b3621e829c8d1ea3e17969 100644
>--- a/Source/WebCore/dom/RejectedPromiseTracker.h
>+++ b/Source/WebCore/dom/RejectedPromiseTracker.h
>@@ -47,19 +47,18 @@ class RejectedPromiseTracker {
>     WTF_MAKE_FAST_ALLOCATED;
>     WTF_MAKE_NONCOPYABLE(RejectedPromiseTracker);
> public:
>-    explicit RejectedPromiseTracker(ScriptExecutionContext&, JSC::VM&);
>+    explicit RejectedPromiseTracker(JSC::VM&);
>     ~RejectedPromiseTracker();
> 
>-    void promiseRejected(JSC::ExecState&, JSDOMGlobalObject&, JSC::JSPromise&);
>-    void promiseHandled(JSC::ExecState&, JSDOMGlobalObject&, JSC::JSPromise&);
>+    void promiseRejected(ScriptExecutionContext&, JSC::ExecState&, JSDOMGlobalObject&, JSC::JSPromise&);
>+    void promiseHandled(ScriptExecutionContext&, JSC::ExecState&, JSDOMGlobalObject&, JSC::JSPromise&);
> 
>-    void processQueueSoon();
>+    void processQueueSoon(ScriptExecutionContext&);
> 
> private:
>-    void reportUnhandledRejections(Vector<UnhandledPromise>&&);
>-    void reportRejectionHandled(Ref<DOMPromise>&&);
>+    void reportUnhandledRejections(ScriptExecutionContext&, Vector<UnhandledPromise>&&);
>+    void reportRejectionHandled(ScriptExecutionContext&, Ref<DOMPromise>&&);
> 
>-    ScriptExecutionContext& m_context;
>     Vector<UnhandledPromise> m_aboutToBeNotifiedRejectedPromises;
>     JSC::WeakGCMap<JSC::JSPromise*, JSC::JSPromise> m_outstandingRejectedPromises;
> };
>diff --git a/Source/WebCore/dom/ScriptExecutionContext.cpp b/Source/WebCore/dom/ScriptExecutionContext.cpp
>index 261bad80999ac6a5b97121ba3213aaee0931bb6f..c87843af6873228414d25464c3d033d1bd19ab7a 100644
>--- a/Source/WebCore/dom/ScriptExecutionContext.cpp
>+++ b/Source/WebCore/dom/ScriptExecutionContext.cpp
>@@ -505,7 +505,7 @@ RejectedPromiseTracker& ScriptExecutionContext::ensureRejectedPromiseTrackerSlow
>     // When initializing ScriptExecutionContext, vm() is not ready.
> 
>     ASSERT(!m_rejectedPromiseTracker);
>-    m_rejectedPromiseTracker = std::make_unique<RejectedPromiseTracker>(*this, vm());
>+    m_rejectedPromiseTracker = std::make_unique<RejectedPromiseTracker>(vm());
>     return *m_rejectedPromiseTracker.get();
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 197172: 369669 | 373561 | 373564 | 373962 | 374097 | 374102 | 375197 | 375965 | 376286 | 376833 | 376931 | 376949 | 377136