Add a way to mark a rejected promise as handled
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Mar 2020 22:27:22 +0000 (22:27 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Mar 2020 22:27:22 +0000 (22:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209241

Reviewed by Michael Saboff.

JSTests:

* stress/reject-as-handled.js: Added.
(shouldBe):
(set new):

Source/JavaScriptCore:

Some of WebCore promise implementations (WebAnimation etc.) want to reject promise
as handled state to suppress UnhandledPromiseRejection tracking. For example, a
lot of WebCore implementations expose Promise DOM attributes which will be rejected
at some conditions. But we do not want to force users setting a handler for each such an
attribute.

This patch adds `JSPromise::rejectAsHandled` C++ function. This simply sets isHandledFlag
before executing `JSPromise::reject` if we are not calling a reject function yet.

* runtime/JSPromise.cpp:
(JSC::JSPromise::rejectAsHandled):
* runtime/JSPromise.h:
* tools/JSDollarVM.cpp:
(JSC::functionRejectPromiseAsHandled):
(JSC::JSDollarVM::finishCreation):

Source/WebCore:

This adds an interface using JSPromise::rejectAsHandled to DOMPromise classes.

* bindings/js/DOMPromiseProxy.h:
(WebCore::DOMPromiseProxy<IDLType>::reject):
(WebCore::DOMPromiseProxy<IDLVoid>::reject):
(WebCore::DOMPromiseProxyWithResolveCallback<IDLType>::reject):
* bindings/js/JSDOMPromiseDeferred.cpp:
(WebCore::DeferredPromise::callFunction):
(WebCore::DeferredPromise::reject):
* bindings/js/JSDOMPromiseDeferred.h:
(WebCore::DeferredPromise::reject):
(WebCore::DeferredPromise::rejectWithCallback):
(WebCore::DOMPromiseDeferredBase::reject):
(WebCore::DOMPromiseDeferredBase::rejectType):

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

JSTests/ChangeLog
JSTests/stress/reject-as-handled.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSPromise.cpp
Source/JavaScriptCore/runtime/JSPromise.h
Source/JavaScriptCore/tools/JSDollarVM.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/DOMPromiseProxy.h
Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
Source/WebCore/bindings/js/JSDOMPromiseDeferred.h

index af2514b..e1ab3b9 100644 (file)
@@ -1,3 +1,14 @@
+2020-03-18  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Add a way to mark a rejected promise as handled
+        https://bugs.webkit.org/show_bug.cgi?id=209241
+
+        Reviewed by Michael Saboff.
+
+        * stress/reject-as-handled.js: Added.
+        (shouldBe):
+        (set new):
+
 2020-03-16  Keith Miller  <keith_miller@apple.com>
 
         JavaScript identifier grammar supports unescaped astral symbols, but JSC doesn’t
diff --git a/JSTests/stress/reject-as-handled.js b/JSTests/stress/reject-as-handled.js
new file mode 100644 (file)
index 0000000..e20c534
--- /dev/null
@@ -0,0 +1,38 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var set = new Set;
+var count = 0;
+setUnhandledRejectionCallback(function (promise) {
+    shouldBe(set.has(promise), true);
+    ++count;
+});
+var promise1 = Promise.reject(42);
+set.add(promise1);
+var promise2 = new Promise(function () { });
+$vm.rejectPromiseAsHandled(promise2, 42);
+
+// If it is already rejected, rejectPromiseAsHandled is no-op: not marking the promise as handled.
+var promise3 = Promise.reject(43);
+set.add(promise3);
+$vm.rejectPromiseAsHandled(promise3, 43);
+
+drainMicrotasks();
+shouldBe(count, 2);
+
+promise1.then($vm.abort, function (value) {
+    shouldBe(value, 42);
+    ++count;
+});
+promise2.then($vm.abort, function (value) {
+    shouldBe(value, 42);
+    ++count;
+});
+promise3.then($vm.abort, function (value) {
+    shouldBe(value, 43);
+    ++count;
+});
+drainMicrotasks();
+shouldBe(count, 5);
index ca27142..2776e8d 100644 (file)
@@ -1,3 +1,26 @@
+2020-03-18  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Add a way to mark a rejected promise as handled
+        https://bugs.webkit.org/show_bug.cgi?id=209241
+
+        Reviewed by Michael Saboff.
+
+        Some of WebCore promise implementations (WebAnimation etc.) want to reject promise
+        as handled state to suppress UnhandledPromiseRejection tracking. For example, a
+        lot of WebCore implementations expose Promise DOM attributes which will be rejected
+        at some conditions. But we do not want to force users setting a handler for each such an
+        attribute.
+
+        This patch adds `JSPromise::rejectAsHandled` C++ function. This simply sets isHandledFlag
+        before executing `JSPromise::reject` if we are not calling a reject function yet.
+
+        * runtime/JSPromise.cpp:
+        (JSC::JSPromise::rejectAsHandled):
+        * runtime/JSPromise.h:
+        * tools/JSDollarVM.cpp:
+        (JSC::functionRejectPromiseAsHandled):
+        (JSC::JSDollarVM::finishCreation):
+
 2020-03-17  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] DeleteIC patchpoint in FTL should require tag and mask registers
index 3e6999f..fe53095 100644 (file)
@@ -182,9 +182,24 @@ void JSPromise::reject(JSGlobalObject* lexicalGlobalObject, JSValue value)
     vm.promiseTimer->cancelPendingPromise(this);
 }
 
+void JSPromise::rejectAsHandled(JSGlobalObject* lexicalGlobalObject, JSValue value)
+{
+    // Setting isHandledFlag before calling reject since this removes round-trip between JSC and PromiseRejectionTracker, and it does not show an user-observable behavior.
+    VM& vm = lexicalGlobalObject->vm();
+    uint32_t flags = this->flags();
+    if (!(flags & isFirstResolvingFunctionCalledFlag))
+        internalField(static_cast<unsigned>(Field::Flags)).set(vm, this, jsNumber(flags | isHandledFlag));
+    reject(lexicalGlobalObject, value);
+}
+
 void JSPromise::reject(JSGlobalObject* lexicalGlobalObject, Exception* reason)
 {
     reject(lexicalGlobalObject, reason->value());
 }
 
+void JSPromise::rejectAsHandled(JSGlobalObject* lexicalGlobalObject, Exception* reason)
+{
+    rejectAsHandled(lexicalGlobalObject, reason->value());
+}
+
 } // namespace JSC
index 16c2691..d3b0af8 100644 (file)
@@ -68,7 +68,9 @@ public:
 
     JS_EXPORT_PRIVATE void resolve(JSGlobalObject*, JSValue);
     JS_EXPORT_PRIVATE void reject(JSGlobalObject*, JSValue);
+    JS_EXPORT_PRIVATE void rejectAsHandled(JSGlobalObject*, JSValue);
     JS_EXPORT_PRIVATE void reject(JSGlobalObject*, Exception*);
+    JS_EXPORT_PRIVATE void rejectAsHandled(JSGlobalObject*, Exception*);
 
     struct DeferredData {
         WTF_FORBID_HEAP_ALLOCATION;
index 9927606..7aacee6 100644 (file)
@@ -2929,6 +2929,14 @@ static EncodedJSValue JSC_HOST_CALL functionHasOwnLengthProperty(JSGlobalObject*
     return JSValue::encode(jsBoolean(function->canAssumeNameAndLengthAreOriginal(vm)));
 }
 
+static EncodedJSValue JSC_HOST_CALL functionRejectPromiseAsHandled(JSGlobalObject* globalObject, CallFrame* callFrame)
+{
+    JSPromise* promise = jsCast<JSPromise*>(callFrame->uncheckedArgument(0));
+    JSValue reason = callFrame->uncheckedArgument(1);
+    promise->rejectAsHandled(globalObject, reason);
+    return JSValue::encode(jsUndefined());
+}
+
 void JSDollarVM::finishCreation(VM& vm)
 {
     DollarVMAssertScope assertScope;
@@ -3063,6 +3071,7 @@ void JSDollarVM::finishCreation(VM& vm)
     addFunction(vm, "getConcurrently", functionGetConcurrently, 2);
 
     addFunction(vm, "hasOwnLengthProperty", functionHasOwnLengthProperty, 1);
+    addFunction(vm, "rejectPromiseAsHandled", functionRejectPromiseAsHandled, 1);
 
     m_objectDoingSideEffectPutWithoutCorrectSlotStatusStructure.set(vm, this, ObjectDoingSideEffectPutWithoutCorrectSlotStatus::createStructure(vm, globalObject, jsNull()));
 }
index fc518a2..c5b4997 100644 (file)
@@ -1,3 +1,25 @@
+2020-03-18  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Add a way to mark a rejected promise as handled
+        https://bugs.webkit.org/show_bug.cgi?id=209241
+
+        Reviewed by Michael Saboff.
+
+        This adds an interface using JSPromise::rejectAsHandled to DOMPromise classes.
+
+        * bindings/js/DOMPromiseProxy.h:
+        (WebCore::DOMPromiseProxy<IDLType>::reject):
+        (WebCore::DOMPromiseProxy<IDLVoid>::reject):
+        (WebCore::DOMPromiseProxyWithResolveCallback<IDLType>::reject):
+        * bindings/js/JSDOMPromiseDeferred.cpp:
+        (WebCore::DeferredPromise::callFunction):
+        (WebCore::DeferredPromise::reject):
+        * bindings/js/JSDOMPromiseDeferred.h:
+        (WebCore::DeferredPromise::reject):
+        (WebCore::DeferredPromise::rejectWithCallback):
+        (WebCore::DOMPromiseDeferredBase::reject):
+        (WebCore::DOMPromiseDeferredBase::rejectType):
+
 2020-03-18  youenn fablet  <youenn@apple.com>
 
         WebPage should own a Ref<WebFrame>
index 672f13c..1482d36 100644 (file)
@@ -51,7 +51,7 @@ public:
 
     void resolve(typename IDLType::ParameterType);
     void resolveWithNewlyCreated(typename IDLType::ParameterType);
-    void reject(Exception);
+    void reject(Exception, RejectAsHandled = RejectAsHandled::No);
     
 private:
     Optional<ExceptionOr<Value>> m_valueOrException;
@@ -72,7 +72,7 @@ public:
     bool isFulfilled() const;
 
     void resolve();
-    void reject(Exception);
+    void reject(Exception, RejectAsHandled = RejectAsHandled::No);
 
 private:
     Optional<ExceptionOr<void>> m_valueOrException;
@@ -102,7 +102,7 @@ public:
 
     void resolve(typename IDLType::ParameterType);
     void resolveWithNewlyCreated(typename IDLType::ParameterType);
-    void reject(Exception);
+    void reject(Exception, RejectAsHandled = RejectAsHandled::No);
     
 private:
     ResolveCallback m_resolveCallback;
@@ -173,13 +173,13 @@ inline void DOMPromiseProxy<IDLType>::resolveWithNewlyCreated(typename IDLType::
 }
 
 template<typename IDLType>
-inline void DOMPromiseProxy<IDLType>::reject(Exception exception)
+inline void DOMPromiseProxy<IDLType>::reject(Exception exception, RejectAsHandled rejectAsHandled)
 {
     ASSERT(!m_valueOrException);
 
     m_valueOrException = ExceptionOr<Value> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(m_valueOrException->exception());
+        deferredPromise->reject(m_valueOrException->exception(), rejectAsHandled);
 }
 
 
@@ -229,12 +229,12 @@ inline void DOMPromiseProxy<IDLVoid>::resolve()
         deferredPromise->resolve();
 }
 
-inline void DOMPromiseProxy<IDLVoid>::reject(Exception exception)
+inline void DOMPromiseProxy<IDLVoid>::reject(Exception exception, RejectAsHandled rejectAsHandled)
 {
     ASSERT(!m_valueOrException);
     m_valueOrException = ExceptionOr<void> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(m_valueOrException->exception());
+        deferredPromise->reject(m_valueOrException->exception(), rejectAsHandled);
 }
 
 // MARK: - DOMPromiseProxyWithResolveCallback<IDLType> implementation
@@ -312,13 +312,13 @@ inline void DOMPromiseProxyWithResolveCallback<IDLType>::resolveWithNewlyCreated
 }
 
 template<typename IDLType>
-inline void DOMPromiseProxyWithResolveCallback<IDLType>::reject(Exception exception)
+inline void DOMPromiseProxyWithResolveCallback<IDLType>::reject(Exception exception, RejectAsHandled rejectAsHandled)
 {
     ASSERT(!m_valueOrException);
 
     m_valueOrException = ExceptionOr<void> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(m_valueOrException->exception());
+        deferredPromise->reject(m_valueOrException->exception(), rejectAsHandled);
 }
 
 }
index 5570f4e..f55d9d1 100644 (file)
@@ -71,6 +71,9 @@ void DeferredPromise::callFunction(JSGlobalObject& lexicalGlobalObject, ResolveM
     case ResolveMode::Reject:
         deferred()->reject(&lexicalGlobalObject, resolution);
         break;
+    case ResolveMode::RejectAsHandled:
+        deferred()->rejectAsHandled(&lexicalGlobalObject, resolution);
+        break;
     }
 
     if (m_mode == Mode::ClearPromiseOnResolve)
@@ -92,7 +95,7 @@ void DeferredPromise::whenSettled(Function<void()>&& callback)
     DOMPromise::whenPromiseIsSettled(globalObject(), deferred(), WTFMove(callback));
 }
 
-void DeferredPromise::reject()
+void DeferredPromise::reject(RejectAsHandled rejectAsHandled)
 {
     if (shouldIgnoreRequestToFulfill())
         return;
@@ -101,10 +104,10 @@ void DeferredPromise::reject()
     ASSERT(m_globalObject);
     auto& lexicalGlobalObject = *m_globalObject;
     JSC::JSLockHolder locker(&lexicalGlobalObject);
-    reject(lexicalGlobalObject, JSC::jsUndefined());
+    reject(lexicalGlobalObject, JSC::jsUndefined(), rejectAsHandled);
 }
 
-void DeferredPromise::reject(std::nullptr_t)
+void DeferredPromise::reject(std::nullptr_t, RejectAsHandled rejectAsHandled)
 {
     if (shouldIgnoreRequestToFulfill())
         return;
@@ -113,10 +116,10 @@ void DeferredPromise::reject(std::nullptr_t)
     ASSERT(m_globalObject);
     auto& lexicalGlobalObject = *m_globalObject;
     JSC::JSLockHolder locker(&lexicalGlobalObject);
-    reject(lexicalGlobalObject, JSC::jsNull());
+    reject(lexicalGlobalObject, JSC::jsNull(), rejectAsHandled);
 }
 
-void DeferredPromise::reject(Exception exception)
+void DeferredPromise::reject(Exception exception, RejectAsHandled rejectAsHandled)
 {
     if (shouldIgnoreRequestToFulfill())
         return;
@@ -134,7 +137,7 @@ void DeferredPromise::reject(Exception exception)
         auto error = scope.exception()->value();
         scope.clearException();
 
-        reject<IDLAny>(error);
+        reject<IDLAny>(error, rejectAsHandled);
         return;
     }
 
@@ -145,10 +148,10 @@ void DeferredPromise::reject(Exception exception)
         return;
     }
 
-    reject(lexicalGlobalObject, error);
+    reject(lexicalGlobalObject, error, rejectAsHandled);
 }
 
-void DeferredPromise::reject(ExceptionCode ec, const String& message)
+void DeferredPromise::reject(ExceptionCode ec, const String& message, RejectAsHandled rejectAsHandled)
 {
     if (shouldIgnoreRequestToFulfill())
         return;
@@ -166,7 +169,7 @@ void DeferredPromise::reject(ExceptionCode ec, const String& message)
         auto error = scope.exception()->value();
         scope.clearException();
 
-        reject<IDLAny>(error);
+        reject<IDLAny>(error, rejectAsHandled);
         return;
     }
 
@@ -178,10 +181,10 @@ void DeferredPromise::reject(ExceptionCode ec, const String& message)
     }
 
 
-    reject(lexicalGlobalObject, error);
+    reject(lexicalGlobalObject, error, rejectAsHandled);
 }
 
-void DeferredPromise::reject(const JSC::PrivateName& privateName)
+void DeferredPromise::reject(const JSC::PrivateName& privateName, RejectAsHandled rejectAsHandled)
 {
     if (shouldIgnoreRequestToFulfill())
         return;
@@ -190,7 +193,7 @@ void DeferredPromise::reject(const JSC::PrivateName& privateName)
     ASSERT(m_globalObject);
     JSC::JSGlobalObject* lexicalGlobalObject = m_globalObject.get();
     JSC::JSLockHolder locker(lexicalGlobalObject);
-    reject(*lexicalGlobalObject, JSC::Symbol::create(lexicalGlobalObject->vm(), privateName.uid()));
+    reject(*lexicalGlobalObject, JSC::Symbol::create(lexicalGlobalObject->vm(), privateName.uid()), rejectAsHandled);
 }
 
 void rejectPromiseWithExceptionIfAny(JSC::JSGlobalObject& lexicalGlobalObject, JSDOMGlobalObject& globalObject, JSPromise& promise)
index 23a1a4c..3abb6e6 100644 (file)
@@ -36,6 +36,7 @@
 namespace WebCore {
 
 class JSDOMWindow;
+enum class RejectAsHandled : uint8_t { No, Yes };
 
 class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
 public:
@@ -109,7 +110,7 @@ public:
     }
 
     template<class IDLType>
-    void reject(typename IDLType::ParameterType value)
+    void reject(typename IDLType::ParameterType value, RejectAsHandled rejectAsHandled = RejectAsHandled::No)
     {
         if (shouldIgnoreRequestToFulfill())
             return;
@@ -118,14 +119,14 @@ public:
         ASSERT(globalObject());
         JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
         JSC::JSLockHolder locker(lexicalGlobalObject);
-        reject(*lexicalGlobalObject, toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value)));
+        reject(*lexicalGlobalObject, toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value)), rejectAsHandled);
     }
 
-    void reject();
-    void reject(std::nullptr_t);
-    void reject(Exception);
-    WEBCORE_EXPORT void reject(ExceptionCode, const String& = { });
-    void reject(const JSC::PrivateName&);
+    void reject(RejectAsHandled = RejectAsHandled::No);
+    void reject(std::nullptr_t, RejectAsHandled = RejectAsHandled::No);
+    void reject(Exception, RejectAsHandled = RejectAsHandled::No);
+    WEBCORE_EXPORT void reject(ExceptionCode, const String& = { }, RejectAsHandled = RejectAsHandled::No);
+    void reject(const JSC::PrivateName&, RejectAsHandled = RejectAsHandled::No);
 
     template<typename Callback>
     void resolveWithCallback(Callback callback)
@@ -141,7 +142,7 @@ public:
     }
 
     template<typename Callback>
-    void rejectWithCallback(Callback callback)
+    void rejectWithCallback(Callback callback, RejectAsHandled rejectAsHandled = RejectAsHandled::No)
     {
         if (shouldIgnoreRequestToFulfill())
             return;
@@ -150,7 +151,7 @@ public:
         ASSERT(globalObject());
         JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
         JSC::JSLockHolder locker(lexicalGlobalObject);
-        reject(*lexicalGlobalObject, callback(*globalObject()));
+        reject(*lexicalGlobalObject, callback(*globalObject()), rejectAsHandled);
     }
 
     JSC::JSValue promise() const;
@@ -168,11 +169,14 @@ private:
 
     JSC::JSPromise* deferred() const { return guarded(); }
 
-    enum class ResolveMode { Resolve, Reject };
+    enum class ResolveMode { Resolve, Reject, RejectAsHandled };
     WEBCORE_EXPORT void callFunction(JSC::JSGlobalObject&, ResolveMode, JSC::JSValue resolution);
 
     void resolve(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSValue resolution) { callFunction(lexicalGlobalObject, ResolveMode::Resolve, resolution); }
-    void reject(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSValue resolution) { callFunction(lexicalGlobalObject, ResolveMode::Reject, resolution); }
+    void reject(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSValue resolution, RejectAsHandled rejectAsHandled)
+    {
+        callFunction(lexicalGlobalObject, rejectAsHandled == RejectAsHandled::Yes ? ResolveMode::RejectAsHandled : ResolveMode::Reject, resolution);
+    }
 
     Mode m_mode;
 };
@@ -207,9 +211,9 @@ public:
         return *this;
     }
 
-    void reject()
+    void reject(RejectAsHandled rejectAsHandled = RejectAsHandled::No)
     {
-        m_promise->reject();
+        m_promise->reject(rejectAsHandled);
     }
 
     template<typename... ErrorType> 
@@ -219,9 +223,9 @@ public:
     }
 
     template<typename IDLType>
-    void rejectType(typename IDLType::ParameterType value)
+    void rejectType(typename IDLType::ParameterType value, RejectAsHandled rejectAsHandled = RejectAsHandled::No)
     {
-        m_promise->reject<IDLType>(std::forward<typename IDLType::ParameterType>(value));
+        m_promise->reject<IDLType>(std::forward<typename IDLType::ParameterType>(value), rejectAsHandled);
     }
 
     JSC::JSValue promise() const { return m_promise->promise(); };