CallbackPromiseAdapter types should be more compatible with WebCallbacks (3/3).

Source/bindings/core/v8/CallbackPromiseAdapter.h

Index: Source/bindings/core/v8/CallbackPromiseAdapter.h
diff --git a/Source/bindings/core/v8/CallbackPromiseAdapter.h b/Source/bindings/core/v8/CallbackPromiseAdapter.h
index 3f37736c6237b6608748e1e3548b2519712220f0..07d8a962d5c4049ec00843ff73c3f49047d34a61 100644
--- a/Source/bindings/core/v8/CallbackPromiseAdapter.h
+++ b/Source/bindings/core/v8/CallbackPromiseAdapter.h
@@ -36,11 +36,57 @@
 #include "public/platform/WebPassOwnPtr.h"
 #include "wtf/OwnPtr.h"
 #include "wtf/PassOwnPtr.h"
+#include "wtf/TypeTraits.h"
 
 namespace blink {
 
-class CallbackPromiseAdapterBase {
-public:
+// CallbackPromiseAdapter is a WebCallbacks subclass and resolves / rejects the
+// stored resolver when onSuccess / onError is called, respectively.
+//
+// Basically CallbackPromiseAdapter<S, T> is a subclass of
+// WebCallbacks<S::WebType, T::WebType>. There are some exceptions:
+//  - If S or T don't have WebType (e.g. S = bool), the trivial WebType holder

[kinuko, 2015/07/27 05:59:00]

nit: 'the trivial WebType holder' -> 'a default WebType holder called
TrivialWebType holder' ?

(trivial webtype holder is not introduced / defined here yet)

[yhirano, 2015/07/27 06:10:27]

Done.

+//    is used. For example, CallbackPromiseAdapter<bool, void> is a subclass of

[kinuko, 2015/07/27 05:59:00]

'is a' -> 'can be a' ?

[yhirano, 2015/07/27 06:10:27]

Why? There is no way to have bool and void have their WebTypes, so CPA<bool,
void> is always a subclass of WC<bool, void>.

[kinuko, 2015/07/27 07:43:54]

Ok, this was impossible before so my mental model was biased (like 'yay, now we
can do this!'). Current wording is fine.

+//    WebCallbacks<bool, void>.
+//  - If a WebType is OwnPtr<T>, its corresponding type parameter on
+//    WebCallbacks is WebPassOwnPtr<T>, because WebCallbacks is exposed to

[kinuko, 2015/07/27 05:59:00]

'is' -> 'must be' ?

[yhirano, 2015/07/27 06:10:27]

Done.

+//    Chromium.
+//
+// When onSuccess is called with a S::WebType value, the value is passed to
+// S::take and the resolver is resolved with its return value. Ditto for
+// onError.
+//
+// ScriptPromiseResolver::resolve / reject will not be called when the execution
+// context is stopped.
+//
+// In order to implemented the above exceptions, we have template classes below.
+// OnSuccess and OnError provide onSuccess and onError implementation, and there
+// are utility templates that provide
+//  - OwnPtr - WebPassOwnPtr translation ([Web]PassType[Impl], adopt, pass),
+//  - trivial WebType holder (TrivialWebTypeHolder).
+
+namespace internal {
+
+// This template is placed outside of CallbackPromiseAdapterInternal because
+// explicit specialization is forbidden in a class scope.
+template <typename T>
+struct CallbackPromiseAdapterTrivialWebTypeHolder {
+    using WebType = T;
+    static T take(ScriptPromiseResolver*, const T& x) { return x; }
+};
+template <>
+struct CallbackPromiseAdapterTrivialWebTypeHolder<void> {
+    using WebType = void;
+};
+
+class CallbackPromiseAdapterInternal {
+private:
+    template <typename T> static T webTypeHolderMatcher(typename WTF::RemoveReference<typename T::WebType>::Type*);
+    template <typename T> static CallbackPromiseAdapterTrivialWebTypeHolder<T> webTypeHolderMatcher(...);
+    template <typename T> using WebTypeHolder = decltype(webTypeHolderMatcher<T>(nullptr));
+
+    // The following templates should be gone when the repositories are merged
+    // and we can use C++11 libraries.
     template <typename T>
     struct PassTypeImpl {
         using Type = T;
@@ -49,7 +95,6 @@ public:
     struct PassTypeImpl<OwnPtr<T>> {
         using Type = PassOwnPtr<T>;
     };
-
     template <typename T>
     struct WebPassTypeImpl {
         using Type = T;
@@ -58,206 +103,87 @@ public:
     struct WebPassTypeImpl<OwnPtr<T>> {
         using Type = WebPassOwnPtr<T>;
     };
+    template <typename T> using PassType = typename PassTypeImpl<T>::Type;
+    template <typename T> using WebPassType = typename WebPassTypeImpl<T>::Type;
+    template <typename T> static T& adopt(T& x) { return x; }
+    template <typename T> static PassOwnPtr<T> adopt(WebPassOwnPtr<T>& x) { return x.release(); }
+    template <typename T> static PassType<T> pass(T& x) { return x; }
+    template <typename T> static PassOwnPtr<T> pass(OwnPtr<T>& x) { return x.release(); }
+
+    template <typename S, typename T>
+    class Base : public WebCallbacks<WebPassType<typename S::WebType>, WebPassType<typename T::WebType>> {
+    public:
+        explicit Base(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : m_resolver(resolver) {}
+        ScriptPromiseResolver* resolver() { return m_resolver.get(); }
+
+    private:
+        RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
+    };
 
+    template <typename S, typename T>
+    class OnSuccess : public Base<S, T> {
+    public:
+        explicit OnSuccess(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : Base<S, T>(resolver) {}
+        void onSuccess(WebPassType<typename S::WebType> r) override
+        {
+            typename S::WebType result(adopt(r));
+            ScriptPromiseResolver* resolver = this->resolver();
+            if (!resolver->executionContext() || resolver->executionContext()->activeDOMObjectsAreStopped())
+                return;
+            resolver->resolve(S::take(resolver, pass(result)));
+        }
+    };
     template <typename T>
-    using PassType = typename PassTypeImpl<T>::Type;
-    template <typename T>
-    using WebPassType = typename WebPassTypeImpl<T>::Type;
-    template <typename T>
-    static T& adopt(T& x) { return x; }
-    template <typename T>
-    static PassOwnPtr<T> adopt(WebPassOwnPtr<T>& x) { return x.release(); }
-    template <typename T>
-    static PassType<T> pass(T& x) { return x; }
-    template <typename T>
-    static PassOwnPtr<T> pass(OwnPtr<T>& x) { return x.release(); }
-};
-
-
-// TODO(yhirano): Add comments.
-template<typename S, typename T>
-class CallbackPromiseAdapter final
-    : public WebCallbacks<typename CallbackPromiseAdapterBase::WebPassType<typename S::WebType>, typename CallbackPromiseAdapterBase::WebPassType<typename T::WebType>>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
-public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    void onSuccess(WebPassType<typename S::WebType> r) override
-    {
-        typename S::WebType result(adopt(r));
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve(S::take(m_resolver.get(), pass(result)));
-    }
-
-    void onError(WebPassType<typename T::WebType> e) override
-    {
-        typename T::WebType error(adopt(e));
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject(T::take(m_resolver.get(), pass(error)));
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
-};
-
-template<typename T>
-class CallbackPromiseAdapter<void, T> final : public WebCallbacks<void, typename CallbackPromiseAdapterBase::WebPassType<typename T::WebType>>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
-public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    void onSuccess() override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve();
-    }
-
-    void onError(WebPassType<typename T::WebType> e) override
-    {
-        typename T::WebType error(adopt(e));
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject(T::take(m_resolver.get(), pass(error)));
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
-};
-
-template<typename S>
-class CallbackPromiseAdapter<S, void> final : public WebCallbacks<typename CallbackPromiseAdapterBase::WebPassType<typename S::WebType>, void>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
-public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    void onSuccess(WebPassType<typename S::WebType> r) override
-    {
-        typename S::WebType result(adopt(r));
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve(S::take(m_resolver.get(), pass(result)));
-    }
-
-    void onError() override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject();
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
-};
-
-template<typename T>
-class CallbackPromiseAdapter<bool, T> final : public WebCallbacks<bool*, typename CallbackPromiseAdapterBase::WebPassType<typename T::WebType>>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
-public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    // TODO(nhiroki): onSuccess should take ownership of a bool object for
-    // consistency. (http://crbug.com/493531)
-    void onSuccess(bool* result) override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve(*result);
-    }
-
-    void onError(WebPassType<typename T::WebType> e) override
-    {
-        typename T::WebType error(adopt(e));
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject(T::take(m_resolver.get(), pass(error)));
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
-};
+    class OnSuccess<WebTypeHolder<void>, T> : public Base<WebTypeHolder<void>, T> {
+    public:
+        explicit OnSuccess(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : Base<WebTypeHolder<void>, T>(resolver) {}
+        void onSuccess() override
+        {
+            ScriptPromiseResolver* resolver = this->resolver();
+            if (!resolver->executionContext() || resolver->executionContext()->activeDOMObjectsAreStopped())
+                return;
+            resolver->resolve();
+        }
+    };
+    template <typename S, typename T>
+    class OnError : public OnSuccess<S, T> {
+    public:
+        explicit OnError(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : OnSuccess<S, T>(resolver) {}
+        void onError(WebPassType<typename T::WebType> e) override
+        {
+            typename T::WebType result(adopt(e));
+            ScriptPromiseResolver* resolver = this->resolver();
+            if (!resolver->executionContext() || resolver->executionContext()->activeDOMObjectsAreStopped())
+                return;
+            resolver->reject(T::take(resolver, pass(result)));
+        }
+    };
+    template <typename S>
+    class OnError<S, WebTypeHolder<void>> : public OnSuccess<S, WebTypeHolder<void>> {
+    public:
+        explicit OnError(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : OnSuccess<S, WebTypeHolder<void>>(resolver) {}
+        void onError() override
+        {
+            ScriptPromiseResolver* resolver = this->resolver();
+            if (!resolver->executionContext() || resolver->executionContext()->activeDOMObjectsAreStopped())
+                return;
+            resolver->reject();
+        }
+    };
 
-template<>
-class CallbackPromiseAdapter<void, void> final : public WebCallbacks<void, void>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
 public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    void onSuccess() override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve();
-    }
-
-    void onError() override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject();
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
+    template <typename S, typename T>
+    class CallbackPromiseAdapter final : public OnError<WebTypeHolder<S>, WebTypeHolder<T>> {
+        WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
+    public:
+        explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver) : OnError<WebTypeHolder<S>, WebTypeHolder<T>>(resolver) {}
+    };
 };
 
-template<>
-class CallbackPromiseAdapter<bool, void> final : public WebCallbacks<bool*, void>, public CallbackPromiseAdapterBase {
-    WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
-public:
-    explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
-        : m_resolver(resolver)
-    {
-        ASSERT(m_resolver);
-    }
-    ~CallbackPromiseAdapter() override { }
-
-    // TODO(nhiroki): onSuccess should take ownership of a bool object for
-    // consistency. (http://crbug.com/493531)
-    void onSuccess(bool* result) override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->resolve(*result);
-    }
+} // namespace internal
 
-    void onError() override
-    {
-        if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
-            return;
-        m_resolver->reject();
-    }
-
-private:
-    RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
-};
+template <typename S, typename T>
+using CallbackPromiseAdapter = internal::CallbackPromiseAdapterInternal::CallbackPromiseAdapter<S, T>;
 
 } // namespace blink