Merge r220620 - [Bindings] Simplify DOMPromiseProxy now that WTF::Fun…

…ction can return references https://bugs.webkit.org/show_bug.cgi?id=175394 Patch by Sam Weinig <sam@webkit.org> on 2017-08-11 Reviewed by Chris Dumez. * bindings/IDLTypes.h: (WebCore::IDLWrapper::convertToParameterType): Deleted. Remove no longer used convertToParameterType. * bindings/js/DOMPromiseProxy.h: - Replace Variant<Value, Exception> with ExceptionOr<Value> / ExceptionOr<void>. - Update ResolveCallback to have a return type of IDLType::ParameterType, rather than IDLType::ImplementationType, now that WTF::Function supports references as the return type. This is needed, since the IDLType::ParameterType for an interface T is T&. * css/FontFace.cpp: * css/FontFace.h: * css/FontFaceSet.cpp: * css/FontFaceSet.h: Update resolve callbacks to return a reference rather than a RefPtr, matching the new signature requirement.
WebKit · Aug 14, 2017 · ec55b9b · ec55b9b
1 parent 30ef731
commit ec55b9b
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 48 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2017-08-11  Sam Weinig  <sam@webkit.org>
+
+        [Bindings] Simplify DOMPromiseProxy now that WTF::Function can return references
+        https://bugs.webkit.org/show_bug.cgi?id=175394
+
+        Reviewed by Chris Dumez.
+
+        * bindings/IDLTypes.h:
+        (WebCore::IDLWrapper::convertToParameterType): Deleted.
+
+            Remove no longer used convertToParameterType.
+
+        * bindings/js/DOMPromiseProxy.h:
+
+            - Replace Variant<Value, Exception> with ExceptionOr<Value> / ExceptionOr<void>.
+            - Update ResolveCallback to have a return type of IDLType::ParameterType, rather than
+              IDLType::ImplementationType, now that WTF::Function supports references as the 
+              return type. This is needed, since the IDLType::ParameterType for an interface T
+              is T&.
+
+        * css/FontFace.cpp:
+        * css/FontFace.h:
+        * css/FontFaceSet.cpp:
+        * css/FontFaceSet.h:
+
+            Update resolve callbacks to return a reference rather than a RefPtr, matching
+            the new signature requirement.
+
 2017-08-11  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Replace JSCryptoKeyCustom and JSCryptoAlgorithmBuilder with generated code

diff --git a/Source/WebCore/bindings/IDLTypes.h b/Source/WebCore/bindings/IDLTypes.h
@@ -170,8 +170,6 @@ template<typename T> struct IDLWrapper : IDLType<RefPtr<T>> {
     static inline std::nullptr_t nullValue() { return nullptr; }
     template<typename U> static inline bool isNullValue(U&& value) { return !value; }
     template<typename U> static inline U&& extractValueFromNullable(U&& value) { return std::forward<U>(value); }
-
-    static ParameterType convertToParameterType(RefPtr<T> value) { ASSERT(value); return *value.get(); }
 };
 
 template<typename T> struct IDLInterface : IDLWrapper<T> { };

diff --git a/Source/WebCore/bindings/js/DOMPromiseProxy.h b/Source/WebCore/bindings/js/DOMPromiseProxy.h
@@ -25,12 +25,11 @@
 
 #pragma once
 
-#include "Exception.h"
+#include "ExceptionOr.h"
 #include "JSDOMGlobalObject.h"
 #include "JSDOMPromiseDeferred.h"
 #include <wtf/Function.h>
 #include <wtf/Optional.h>
-#include <wtf/Variant.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -39,7 +38,6 @@ template<typename IDLType>
 class DOMPromiseProxy {
 public:
     using Value = typename IDLType::StorageType;
-    using ValueOrException = Variant<Value, Exception>;
 
     DOMPromiseProxy() = default;
     ~DOMPromiseProxy() = default;
@@ -55,16 +53,13 @@ class DOMPromiseProxy {
     void reject(Exception);
 
 private:
-    std::optional<ValueOrException> m_valueOrException;
+    std::optional<ExceptionOr<Value>> m_valueOrException;
     Vector<Ref<DeferredPromise>, 1> m_deferredPromises;
 };
 
 template<>
 class DOMPromiseProxy<IDLVoid> {
 public:
-    using Value = bool;
-    using ValueOrException = Variant<Value, Exception>;
-
     DOMPromiseProxy() = default;
     ~DOMPromiseProxy() = default;
 
@@ -78,7 +73,7 @@ class DOMPromiseProxy<IDLVoid> {
     void reject(Exception);
 
 private:
-    std::optional<ValueOrException> m_valueOrException;
+    std::optional<ExceptionOr<void>> m_valueOrException;
     Vector<Ref<DeferredPromise>, 1> m_deferredPromises;
 };
 
@@ -89,15 +84,10 @@ class DOMPromiseProxy<IDLVoid> {
 template<typename IDLType>
 class DOMPromiseProxyWithResolveCallback {
 public:
-    using Value = bool;
-    using ValueOrException = Variant<Value, Exception>;
-    // FIXME: This should return a IDLType::ParameterType, but WTF::Function does
-    //        not support returning references / non-default initializable types.
-    //        See https://webkit.org/b/175244.
-    using ResolveCallback = WTF::Function<typename IDLType::ImplementationType ()>;
+    using ResolveCallback = WTF::Function<typename IDLType::ParameterType ()>;
 
     template <typename Class, typename BaseClass>
-    DOMPromiseProxyWithResolveCallback(Class&, typename IDLType::ImplementationType (BaseClass::*)());
+    DOMPromiseProxyWithResolveCallback(Class&, typename IDLType::ParameterType (BaseClass::*)());
     DOMPromiseProxyWithResolveCallback(ResolveCallback&&);
     ~DOMPromiseProxyWithResolveCallback() = default;
 
@@ -113,8 +103,7 @@ class DOMPromiseProxyWithResolveCallback {
 
 private:
     ResolveCallback m_resolveCallback;
-
-    std::optional<ValueOrException> m_valueOrException;
+    std::optional<ExceptionOr<void>> m_valueOrException;
     Vector<Ref<DeferredPromise>, 1> m_deferredPromises;
 };
 
@@ -135,10 +124,10 @@ inline JSC::JSValue DOMPromiseProxy<IDLType>::promise(JSC::ExecState& state, JSD
         return JSC::jsUndefined();
 
     if (m_valueOrException) {
-        if (WTF::holds_alternative<Value>(*m_valueOrException))
-            deferredPromise->template resolve<IDLType>(WTF::get<Value>(*m_valueOrException));
+        if (m_valueOrException->hasException())
+            deferredPromise->reject(m_valueOrException->exception());
         else
-            deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+            deferredPromise->template resolve<IDLType>(m_valueOrException->returnValue());
     }
 
     auto result = deferredPromise->promise();
@@ -164,29 +153,29 @@ inline void DOMPromiseProxy<IDLType>::resolve(typename IDLType::ParameterType va
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { std::forward<typename IDLType::ParameterType>(value) };
+    m_valueOrException = ExceptionOr<Value> { std::forward<typename IDLType::ParameterType>(value) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->template resolve<IDLType>(WTF::get<Value>(*m_valueOrException));
+        deferredPromise->template resolve<IDLType>(m_valueOrException->returnValue());
 }
 
 template<typename IDLType>
 inline void DOMPromiseProxy<IDLType>::resolveWithNewlyCreated(typename IDLType::ParameterType value)
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { std::forward<typename IDLType::ParameterType>(value) };
+    m_valueOrException = ExceptionOr<Value> { std::forward<typename IDLType::ParameterType>(value) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->template resolveWithNewlyCreated<IDLType>(WTF::get<Value>(*m_valueOrException));
+        deferredPromise->template resolveWithNewlyCreated<IDLType>(m_valueOrException->returnValue());
 }
 
 template<typename IDLType>
 inline void DOMPromiseProxy<IDLType>::reject(Exception exception)
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { WTFMove(exception) };
+    m_valueOrException = ExceptionOr<Value> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+        deferredPromise->reject(m_valueOrException->exception());
 }
 
 
@@ -205,10 +194,10 @@ inline JSC::JSValue DOMPromiseProxy<IDLVoid>::promise(JSC::ExecState& state, JSD
         return JSC::jsUndefined();
 
     if (m_valueOrException) {
-        if (WTF::holds_alternative<Value>(*m_valueOrException))
-            deferredPromise->resolve();
+        if (m_valueOrException->hasException())
+            deferredPromise->reject(m_valueOrException->exception());
         else
-            deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+            deferredPromise->resolve();
     }
 
     auto result = deferredPromise->promise();
@@ -230,24 +219,24 @@ inline bool DOMPromiseProxy<IDLVoid>::isFulfilled() const
 inline void DOMPromiseProxy<IDLVoid>::resolve()
 {
     ASSERT(!m_valueOrException);
-    m_valueOrException = ValueOrException { true };
+    m_valueOrException = ExceptionOr<void> { };
     for (auto& deferredPromise : m_deferredPromises)
         deferredPromise->resolve();
 }
 
 inline void DOMPromiseProxy<IDLVoid>::reject(Exception exception)
 {
     ASSERT(!m_valueOrException);
-    m_valueOrException = ValueOrException { WTFMove(exception) };
+    m_valueOrException = ExceptionOr<void> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+        deferredPromise->reject(m_valueOrException->exception());
 }
 
 // MARK: - DOMPromiseProxyWithResolveCallback<IDLType> implementation
 
 template<typename IDLType>
 template <typename Class, typename BaseClass>
-inline DOMPromiseProxyWithResolveCallback<IDLType>::DOMPromiseProxyWithResolveCallback(Class& object, typename IDLType::ImplementationType (BaseClass::*function)())
+inline DOMPromiseProxyWithResolveCallback<IDLType>::DOMPromiseProxyWithResolveCallback(Class& object, typename IDLType::ParameterType (BaseClass::*function)())
     : m_resolveCallback(std::bind(function, &object))
 {
 }
@@ -272,10 +261,10 @@ inline JSC::JSValue DOMPromiseProxyWithResolveCallback<IDLType>::promise(JSC::Ex
         return JSC::jsUndefined();
 
     if (m_valueOrException) {
-        if (WTF::holds_alternative<Value>(*m_valueOrException))
-            deferredPromise->template resolve<IDLType>(IDLType::convertToParameterType(m_resolveCallback()));
+        if (m_valueOrException->hasException())
+            deferredPromise->reject(m_valueOrException->exception());
         else
-            deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+            deferredPromise->template resolve<IDLType>(m_resolveCallback());
     }
 
     auto result = deferredPromise->promise();
@@ -301,7 +290,7 @@ inline void DOMPromiseProxyWithResolveCallback<IDLType>::resolve(typename IDLTyp
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { true };
+    m_valueOrException = ExceptionOr<void> { };
     for (auto& deferredPromise : m_deferredPromises)
         deferredPromise->template resolve<IDLType>(value);
 }
@@ -311,7 +300,7 @@ inline void DOMPromiseProxyWithResolveCallback<IDLType>::resolveWithNewlyCreated
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { true };
+    m_valueOrException = ExceptionOr<void> { };
     for (auto& deferredPromise : m_deferredPromises)
         deferredPromise->template resolveWithNewlyCreated<IDLType>(value);
 }
@@ -321,9 +310,9 @@ inline void DOMPromiseProxyWithResolveCallback<IDLType>::reject(Exception except
 {
     ASSERT(!m_valueOrException);
 
-    m_valueOrException = ValueOrException { WTFMove(exception) };
+    m_valueOrException = ExceptionOr<void> { WTFMove(exception) };
     for (auto& deferredPromise : m_deferredPromises)
-        deferredPromise->reject(WTF::get<Exception>(*m_valueOrException));
+        deferredPromise->reject(m_valueOrException->exception());
 }
 
 }
diff --git a/Source/WebCore/css/FontFace.cpp b/Source/WebCore/css/FontFace.cpp
@@ -448,9 +448,9 @@ auto FontFace::load() -> LoadedPromise&
     return m_loadedPromise;
 }
 
-RefPtr<FontFace> FontFace::loadedPromiseResolve()
+FontFace& FontFace::loadedPromiseResolve()
 {
-    return this;
+    return *this;
 }
 
 }
diff --git a/Source/WebCore/css/FontFace.h b/Source/WebCore/css/FontFace.h
@@ -94,7 +94,8 @@ class FontFace final : public RefCounted<FontFace>, private CSSFontFace::Client
     explicit FontFace(CSSFontSelector&);
     explicit FontFace(CSSFontFace&);
 
-    RefPtr<FontFace> loadedPromiseResolve();
+    // Callback for LoadedPromise.
+    FontFace& loadedPromiseResolve();
 
     WeakPtrFactory<FontFace> m_weakPtrFactory;
     Ref<CSSFontFace> m_backing;

diff --git a/Source/WebCore/css/FontFaceSet.cpp b/Source/WebCore/css/FontFaceSet.cpp
@@ -224,9 +224,9 @@ void FontFaceSet::faceFinished(CSSFontFace& face, CSSFontFace::Status newStatus)
     m_pendingPromises.remove(iterator);
 }
 
-RefPtr<FontFaceSet> FontFaceSet::readyPromiseResolve()
+FontFaceSet& FontFaceSet::readyPromiseResolve()
 {
-    return this;
+    return *this;
 }
 
 }
diff --git a/Source/WebCore/css/FontFaceSet.h b/Source/WebCore/css/FontFaceSet.h
@@ -108,7 +108,8 @@ class FontFaceSet final : public RefCounted<FontFaceSet>, private CSSFontFaceSet
     void refEventTarget() final { ref(); }
     void derefEventTarget() final { deref(); }
 
-    RefPtr<FontFaceSet> readyPromiseResolve();
+    // Callback for ReadyPromise.
+    FontFaceSet& readyPromiseResolve();
 
     Ref<CSSFontFaceSet> m_backing;
     HashMap<RefPtr<FontFace>, Vector<Ref<PendingPromise>>> m_pendingPromises;