[Bindings] Standardize on DOMPromise as the way to store passed in promises
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 23:31:03 +0000 (23:31 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 23:31:03 +0000 (23:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178533

Reviewed by Youenn Fablet.

This standardizes on RefPtr<DOMPromise> as the canonical way to store a promise
that has been passed in from JS. This does not change promises that start off in
WebCore and are passed to JS; they remain using DOMPromiseDeferred and DOMPromiseProxy.

* Modules/paymentrequest/PaymentRequestUpdateEvent.cpp:
* Modules/paymentrequest/PaymentRequestUpdateEvent.h:
* dom/PromiseRejectionEvent.cpp:
* dom/PromiseRejectionEvent.h:
* dom/RejectedPromiseTracker.cpp:

    Use a RefPtr<DOMPromise> rather than a JSPromise* to hold onto the promise.

* bindings/IDLTypes.h:

    Use IDLWrapper to get better defaults, since DOMPromise is refcounted.

* bindings/js/JSDOMConvertPromise.h:
(WebCore::Converter<IDLPromise<T>>::convert):

    Switch default conversion to return a RefPtr<DOMPromise> rather than a JSPromise*

(WebCore::JSConverter<IDLPromise<T>>::convert):

    Add support for converting from a DOMPromise to a JSValue.

* bindings/js/JSDOMPromise.cpp:
* bindings/js/JSDOMPromise.h:
(WebCore::DOMPromise::create): Deleted.

    Remove now unused constructor.

* workers/service/ExtendableEvent.cpp:
(WebCore::ExtendableEvent::waitUntil):
* workers/service/ExtendableEvent.h:
* workers/service/ExtendableEvent.idl:
* workers/service/FetchEvent.cpp:
(WebCore::FetchEvent::respondWith):
(WebCore::FetchEvent::promiseIsSettled):
* workers/service/FetchEvent.h:
* workers/service/FetchEvent.idl:

    Address FIXMEs and remove need for passing an ExecState to ExtendableEvent
    and FetchEvent by using the new default conversion to DOMPromise.

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.cpp
Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.h
Source/WebCore/bindings/IDLTypes.h
Source/WebCore/bindings/js/JSDOMConvertPromise.h
Source/WebCore/bindings/js/JSDOMPromise.cpp
Source/WebCore/bindings/js/JSDOMPromise.h
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/dom/PromiseRejectionEvent.cpp
Source/WebCore/dom/PromiseRejectionEvent.h
Source/WebCore/dom/RejectedPromiseTracker.cpp
Source/WebCore/workers/service/ExtendableEvent.cpp
Source/WebCore/workers/service/ExtendableEvent.h
Source/WebCore/workers/service/ExtendableEvent.idl
Source/WebCore/workers/service/FetchEvent.cpp
Source/WebCore/workers/service/FetchEvent.h
Source/WebCore/workers/service/FetchEvent.idl

index 7094ec9..42170b3 100644 (file)
@@ -1,5 +1,56 @@
 2017-10-19  Sam Weinig  <sam@webkit.org>
 
+        [Bindings] Standardize on DOMPromise as the way to store passed in promises
+        https://bugs.webkit.org/show_bug.cgi?id=178533
+
+        Reviewed by Youenn Fablet.
+
+        This standardizes on RefPtr<DOMPromise> as the canonical way to store a promise
+        that has been passed in from JS. This does not change promises that start off in
+        WebCore and are passed to JS; they remain using DOMPromiseDeferred and DOMPromiseProxy.
+
+        * Modules/paymentrequest/PaymentRequestUpdateEvent.cpp:
+        * Modules/paymentrequest/PaymentRequestUpdateEvent.h:
+        * dom/PromiseRejectionEvent.cpp:
+        * dom/PromiseRejectionEvent.h:
+        * dom/RejectedPromiseTracker.cpp:
+
+            Use a RefPtr<DOMPromise> rather than a JSPromise* to hold onto the promise.
+
+        * bindings/IDLTypes.h:
+
+            Use IDLWrapper to get better defaults, since DOMPromise is refcounted.
+
+        * bindings/js/JSDOMConvertPromise.h:
+        (WebCore::Converter<IDLPromise<T>>::convert):
+
+            Switch default conversion to return a RefPtr<DOMPromise> rather than a JSPromise*
+
+        (WebCore::JSConverter<IDLPromise<T>>::convert):
+
+            Add support for converting from a DOMPromise to a JSValue.
+
+        * bindings/js/JSDOMPromise.cpp:
+        * bindings/js/JSDOMPromise.h:
+        (WebCore::DOMPromise::create): Deleted.
+
+            Remove now unused constructor.    
+
+        * workers/service/ExtendableEvent.cpp:
+        (WebCore::ExtendableEvent::waitUntil):
+        * workers/service/ExtendableEvent.h:
+        * workers/service/ExtendableEvent.idl:
+        * workers/service/FetchEvent.cpp:
+        (WebCore::FetchEvent::respondWith):
+        (WebCore::FetchEvent::promiseIsSettled):
+        * workers/service/FetchEvent.h:
+        * workers/service/FetchEvent.idl:
+
+            Address FIXMEs and remove need for passing an ExecState to ExtendableEvent
+            and FetchEvent by using the new default conversion to DOMPromise.
+
+2017-10-19  Sam Weinig  <sam@webkit.org>
+
         [Settings] Move global settings into their own file
         https://bugs.webkit.org/show_bug.cgi?id=178512
 
index a9aff7d..ad64132 100644 (file)
@@ -34,7 +34,7 @@ PaymentRequestUpdateEvent::~PaymentRequestUpdateEvent()
 {
 }
 
-void PaymentRequestUpdateEvent::updateWith(JSC::JSPromise*)
+void PaymentRequestUpdateEvent::updateWith(Ref<DOMPromise>&&)
 {
 }
 
index 4c4a05c..616ef8b 100644 (file)
 
 #include "Event.h"
 
-namespace JSC {
-class JSPromise;
-}
-
 namespace WebCore {
 
+class DOMPromise;
+
 class PaymentRequestUpdateEvent final : public Event {
 public:
     ~PaymentRequestUpdateEvent();
-    void updateWith(JSC::JSPromise*);
+    void updateWith(Ref<DOMPromise>&&);
 };
 
 } // namespace WebCore
index befba58..6b7bb22 100644 (file)
@@ -220,7 +220,7 @@ template<typename K, typename V> struct IDLRecord : IDLType<Vector<WTF::KeyValue
     using NullableParameterType = const std::optional<Vector<WTF::KeyValuePair<typename K::ImplementationType, typename V::ImplementationType>>>&;
 };
 
-template<typename T> struct IDLPromise : IDLType<DOMPromise> {
+template<typename T> struct IDLPromise : IDLWrapper<DOMPromise> {
     using InnerType = T;
 };
 
index 555c38e..7d34df8 100644 (file)
@@ -32,7 +32,7 @@
 namespace WebCore {
 
 template<typename T> struct Converter<IDLPromise<T>> : DefaultConverter<IDLPromise<T>> {
-    using ReturnType = JSC::JSPromise*;
+    using ReturnType = RefPtr<DOMPromise>;
 
     // https://heycam.github.io/webidl/#es-promise
     template<typename ExceptionThrower = DefaultExceptionThrower>
@@ -54,7 +54,7 @@ template<typename T> struct Converter<IDLPromise<T>> : DefaultConverter<IDLPromi
         ASSERT(promise);
 
         // 3. Return the IDL promise type value that is a reference to the same object as promise.
-        return promise;
+        return DOMPromise::create(*globalObject, *promise);
     }
 };
 
@@ -62,9 +62,9 @@ template<typename T> struct JSConverter<IDLPromise<T>> {
     static constexpr bool needsState = true;
     static constexpr bool needsGlobalObject = true;
 
-    static JSC::JSValue convert(JSC::ExecState&, JSDOMGlobalObject&, JSC::JSPromise& promise)
+    static JSC::JSValue convert(JSC::ExecState&, JSDOMGlobalObject&, DOMPromise& promise)
     {
-        return &promise;
+        return promise.promise();
     }
 
     template<template<typename> class U>
index 5943cf9..080e539 100644 (file)
@@ -49,24 +49,6 @@ static inline JSC::JSValue callFunction(JSC::ExecState& state, JSC::JSValue jsFu
     return result;
 }
 
-Ref<DOMPromise> DOMPromise::create(JSC::ExecState& state, JSC::JSValue value)
-{
-    auto& globalObject = *JSC::jsCast<JSDOMGlobalObject*>(state.lexicalGlobalObject());
-
-    auto promiseConstructor = globalObject.promiseConstructor();
-    auto resolveFunction = promiseConstructor->get(&state, state.vm().propertyNames->builtinNames().resolvePrivateName());
-    ASSERT(resolveFunction.isFunction());
-
-    JSC::MarkedArgumentBuffer arguments;
-    arguments.append(value);
-    auto result = callFunction(state, resolveFunction, promiseConstructor, arguments);
-
-    auto* promise = JSC::jsCast<JSC::JSPromise*>(result);
-    ASSERT(promise);
-
-    return create(globalObject, *promise);
-}
-
 void DOMPromise::whenSettled(std::function<void()>&& callback)
 {
     auto& state = *globalObject()->globalExec();
index a368507..b310a1b 100644 (file)
@@ -33,7 +33,6 @@ namespace WebCore {
 
 class DOMPromise : public DOMGuarded<JSC::JSPromise> {
 public:
-    static Ref<DOMPromise> create(JSC::ExecState&, JSC::JSValue);
     static Ref<DOMPromise> create(JSDOMGlobalObject& globalObject, JSC::JSPromise& promise)
     {
         return adoptRef(*new DOMPromise(globalObject, promise));
index f143d74..ffbf5ed 100644 (file)
@@ -1544,7 +1544,7 @@ sub PassArgumentExpression
         return "${name}.releaseNonNull()";
     }
 
-    return "${name}.releaseNonNull()" if $codeGenerator->IsCallbackInterface($type) || $codeGenerator->IsCallbackFunction($type);
+    return "${name}.releaseNonNull()" if $codeGenerator->IsCallbackInterface($type) || $codeGenerator->IsCallbackFunction($type) || $codeGenerator->IsPromiseType($type);
     return "*${name}" if $codeGenerator->IsWrapperType($type);
     return "WTFMove(${name})";
 }
index cb3fd7f..5cc6af1 100644 (file)
 #include "PromiseRejectionEvent.h"
 
 #include "DOMWrapperWorld.h"
+#include "JSDOMPromise.h"
 #include <heap/HeapInlines.h>
 #include <heap/StrongInlines.h>
 
-
 namespace WebCore {
 using namespace JSC;
 
 PromiseRejectionEvent::PromiseRejectionEvent(ExecState& state, const AtomicString& type, const Init& initializer, IsTrusted isTrusted)
     : Event(type, initializer, isTrusted)
-    , m_promise(state.vm(), initializer.promise)
+    , m_promise(*(initializer.promise))
     , m_reason(state.vm(), initializer.reason)
 {
 }
index d0f4297..d3af5e6 100644 (file)
 
 #include "Event.h"
 #include <heap/Strong.h>
-#include <runtime/JSPromise.h>
 
 namespace WebCore {
 
+class DOMPromise;
+
 class PromiseRejectionEvent final : public Event {
 public:
     struct Init : EventInit {
-        JSC::JSPromise* promise;
+        RefPtr<DOMPromise> promise;
         JSC::JSValue reason;
     };
 
@@ -45,7 +46,7 @@ public:
 
     virtual ~PromiseRejectionEvent();
 
-    JSC::JSPromise& promise() const { return *m_promise.get(); }
+    DOMPromise& promise() const { return m_promise.get(); }
     JSC::JSValue reason() const { return m_reason.get(); }
 
     EventInterface eventInterface() const override { return PromiseRejectionEventInterfaceType; }
@@ -53,7 +54,7 @@ public:
 private:
     PromiseRejectionEvent(JSC::ExecState&, const AtomicString&, const Init&, IsTrusted);
 
-    JSC::Strong<JSC::JSPromise> m_promise;
+    Ref<DOMPromise> m_promise;
     JSC::Strong<JSC::Unknown> m_reason;
 };
 
index 083b89c..2d93d07 100644 (file)
@@ -166,7 +166,7 @@ void RejectedPromiseTracker::reportUnhandledRejections(Vector<UnhandledPromise>&
 
         PromiseRejectionEvent::Init initializer;
         initializer.cancelable = true;
-        initializer.promise = &promise;
+        initializer.promise = &domPromise;
         initializer.reason = promise.result(vm);
 
         auto event = PromiseRejectionEvent::create(state, eventNames().unhandledrejectionEvent, initializer);
@@ -195,7 +195,7 @@ void RejectedPromiseTracker::reportRejectionHandled(Ref<DOMPromise>&& rejectedPr
     auto& promise = *rejectedPromise->promise();
 
     PromiseRejectionEvent::Init initializer;
-    initializer.promise = &promise;
+    initializer.promise = rejectedPromise.ptr();
     initializer.reason = promise.result(vm);
 
     auto event = PromiseRejectionEvent::create(state, eventNames().rejectionhandledEvent, initializer);
index d761003..97c67c4 100644 (file)
@@ -28,6 +28,8 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "JSDOMPromise.h"
+
 namespace WebCore {
 
 ExtendableEvent::ExtendableEvent(const AtomicString& type, const ExtendableEventInit& initializer, IsTrusted isTrusted)
@@ -35,7 +37,7 @@ ExtendableEvent::ExtendableEvent(const AtomicString& type, const ExtendableEvent
 {
 }
 
-ExceptionOr<void> ExtendableEvent::waitUntil(JSC::ExecState& state, JSC::JSValue promise)
+ExceptionOr<void> ExtendableEvent::waitUntil(Ref<DOMPromise>&& promise)
 {
     if (!isTrusted())
         return Exception { InvalidStateError, ASCIILiteral("Event is not trusted") };
@@ -43,8 +45,7 @@ ExceptionOr<void> ExtendableEvent::waitUntil(JSC::ExecState& state, JSC::JSValue
     if (m_pendingPromises.isEmpty() && isBeingDispatched())
         return Exception { InvalidStateError, ASCIILiteral("Event is being dispatched") };
 
-    addPendingPromise(DOMPromise::create(state, promise));
-
+    addPendingPromise(WTFMove(promise));
     return { };
 }
 
index 6ea39f8..7074805 100644 (file)
 
 #include "Event.h"
 #include "ExtendableEventInit.h"
-#include "JSDOMPromise.h"
 #include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
+class DOMPromise;
+
 class ExtendableEvent : public Event {
 public:
     static Ref<ExtendableEvent> create(const AtomicString& type, const ExtendableEventInit& initializer, IsTrusted isTrusted = IsTrusted::No)
@@ -43,7 +44,7 @@ public:
 
     EventInterface eventInterface() const override { return ExtendableEventInterfaceType; }
 
-    ExceptionOr<void> waitUntil(JSC::ExecState&, JSC::JSValue);
+    ExceptionOr<void> waitUntil(Ref<DOMPromise>&&);
 
     WEBCORE_EXPORT void onFinishedWaitingForTesting(WTF::Function<void()>&&);
 
index 57ef5d0..c3918a3 100644 (file)
@@ -23,7 +23,7 @@
 * THE POSSIBILITY OF SUCH DAMAGE.
 */
 
-//FIXME: Should be exposed on ServiceWorker only.
+// FIXME: Should be exposed on ServiceWorker only.
 [
     Constructor(DOMString type, optional ExtendableEventInit eventInitDict),
     Conditional=SERVICE_WORKER,
@@ -31,8 +31,6 @@
     Exposed=(ServiceWorker,Worker,Window),
     ExportMacro=WEBCORE_EXPORT,
     JSGenerateToNativeObject,
-]
-interface ExtendableEvent : Event {
-    // FIXME: Binding generator should be able to wrap any non Promise value into a promise and pass it to waitUntil.
-    [CallWith=ScriptState, MayThrowException] void waitUntil(any f);
+] interface ExtendableEvent : Event {
+    [MayThrowException] void waitUntil(Promise<any> f);
 };
index 7bf6d77..38bf016 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "FetchEvent.h"
 
+#include "JSDOMPromise.h"
 #include "JSFetchResponse.h"
 
 #if ENABLE(SERVICE_WORKER)
@@ -41,7 +42,7 @@ FetchEvent::FetchEvent(const AtomicString& type, Init&& initializer, IsTrusted i
 {
 }
 
-ExceptionOr<void> FetchEvent::respondWith(JSC::ExecState& state, JSC::JSValue promise)
+ExceptionOr<void> FetchEvent::respondWith(Ref<DOMPromise>&& promise)
 {
     if (isBeingDispatched())
         return Exception { InvalidStateError, ASCIILiteral("Event is being dispatched") };
@@ -49,7 +50,7 @@ ExceptionOr<void> FetchEvent::respondWith(JSC::ExecState& state, JSC::JSValue pr
     if (m_respondWithEntered)
         return Exception { InvalidStateError, ASCIILiteral("Event respondWith flag is set") };
 
-    m_respondPromise = DOMPromise::create(state, promise);
+    m_respondPromise = WTFMove(promise);
     addPendingPromise(*m_respondPromise);
 
     m_respondPromise->whenSettled([this, weakThis = createWeakPtr()] () {
@@ -133,12 +134,16 @@ void FetchEvent::promiseIsSettled()
     }
 
     auto body = m_response->consumeBody();
-    WTF::switchOn(body, [] (Ref<FormData>&) {
-        // FIXME: Support FormData response bodies.
-    }, [this] (Ref<SharedBuffer>& buffer) {
-        m_responseBody = WTFMove(buffer);
-    }, [] (std::nullptr_t&) {
-    });
+    WTF::switchOn(body, 
+        [] (Ref<FormData>&) {
+            // FIXME: Support FormData response bodies.
+        },
+        [this] (Ref<SharedBuffer>& buffer) {
+            m_responseBody = WTFMove(buffer);
+        }, 
+        [] (std::nullptr_t&) {
+        }
+    );
 
     processResponse();
 }
index 57a174f..c231b5b 100644 (file)
@@ -49,7 +49,7 @@ public:
 
     EventInterface eventInterface() const final { return FetchEventInterfaceType; }
 
-    ExceptionOr<void> respondWith(JSC::ExecState&, JSC::JSValue);
+    ExceptionOr<void> respondWith(Ref<DOMPromise>&&);
 
     WEBCORE_EXPORT void onResponse(WTF::Function<void()>&&);
 
index d428d45..c90ceff 100644 (file)
@@ -23,7 +23,7 @@
 * THE POSSIBILITY OF SUCH DAMAGE.
 */
 
-//FIXME: Should be exposed on ServiceWorker only.
+// FIXME: Should be exposed on ServiceWorker only.
 [
     Constructor(DOMString type, FetchEventInit eventInitDict),
     Conditional=SERVICE_WORKER,
     Exposed=(ServiceWorker,Worker,Window),
     ExportToWrappedFunction,
     JSGenerateToNativeObject
-]
-interface FetchEvent : ExtendableEvent {
+] interface FetchEvent : ExtendableEvent {
     [SameObject] readonly attribute FetchRequest request;
     readonly attribute DOMString clientId;
     readonly attribute DOMString reservedClientId;
     readonly attribute DOMString targetClientId;
 
-    // FIXME: Binding generator should be able to wrap any non Promise value into a promise and pass it to respondWith.
-    [CallWith=ScriptState, MayThrowException] void respondWith(any r);
+    [MayThrowException] void respondWith(Promise<FetchResponse> r);
 };
 
 dictionary FetchEventInit : ExtendableEventInit {