(WONT COMMIT) Pass a boPass a bool object allocated on heap to CallbackPromiseAdapter::onSuccess (2)

Source/bindings/core/v8/CallbackPromiseAdapter.h

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 18 matching lines @@
  */
 
 #ifndef CallbackPromiseAdapter_h
 #define CallbackPromiseAdapter_h
 
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "public/platform/WebCallbacks.h"
 
 namespace blink {
 
+// TODO(nhiroki): Remove ifdef hack after Chromium-side patch is landed.
+// http://crbug.com/493531
+#define CRBUG_493531
+
 // This class provides an easy way to convert from a Script-exposed
 // class (i.e. a class that has a toV8() overload) that uses Promises
 // to a WebKit API class that uses WebCallbacks. You can define
 // separate Success and Error classes, but this example just uses one
 // object for both.
 //
 // To use:
 //
 // class MyClass ... {
 //    typedef blink::WebMyClass WebType;
@@ skipping 126 matching lines @@
 class CallbackPromiseAdapter<bool, T> final : public WebCallbacks<bool, typename T::WebType> {
     WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
 public:
     explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
         : m_resolver(resolver)
     {
         ASSERT(m_resolver);
     }
     virtual ~CallbackPromiseAdapter() { }
 
-    // TODO(nhiroki): onSuccess should take ownership of a bool object for
-    // consistency. (http://crbug.com/493531)
+    // Takes ownership of |result|.
     virtual void onSuccess(bool* result) override
     {
+        OwnPtr<bool> ownPtr = adoptPtr(result);
         if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
             return;
-        m_resolver->resolve(*result);
+        m_resolver->resolve(*ownPtr);
     }
 
     // Takes ownership of |error|.
     virtual void onError(typename T::WebType* error) override
     {
         OwnPtr<typename T::WebType> ownPtr = adoptPtr(error);
         if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
             return;
         m_resolver->reject(T::take(m_resolver.get(), ownPtr.release()));
     }
@@ skipping 35 matching lines @@
 class CallbackPromiseAdapter<bool, void> final : public WebCallbacks<bool, void> {
     WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
 public:
     explicit CallbackPromiseAdapter(PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
         : m_resolver(resolver)
     {
         ASSERT(m_resolver);
     }
     virtual ~CallbackPromiseAdapter() { }
 
-    // TODO(nhiroki): onSuccess should take ownership of a bool object for
-    // consistency. (http://crbug.com/493531)
+    // Takes ownership of |result|.
     virtual void onSuccess(bool* result) override
     {
+        OwnPtr<bool> ownPtr = adoptPtr(result);
         if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
             return;
-        m_resolver->resolve(*result);
+        m_resolver->resolve(*ownPtr);

[haraken, 2015/07/03 05:57:49]

This will end up with calling resolveOrReject(OwnPtr<bool>, ...), which will
call toV8(OwnPtr<bool>, ...). This seems not quite right to me.

hirano-san?

[yhirano, 2015/07/03 06:04:12]

This statement passes |*ownPtr| of type |bool|, so it looks good. Am I missing
anything?

[haraken, 2015/07/03 06:09:06]

ah, OK. Then what's the merit of using an OwnPtr? It looks a bit strange to use
OwnPtr<bool>.

[nhiroki, 2015/07/03 06:59:39]

Sorry, I'm not really sure why you feel this a bit strange... which solution are
you thinking?

(1) manually deleting an object,
(2) passing an object w/o ownership as the original implementation, or
(3) passing an object by value

Re (1): I'd prefer a scoped pointer to avoid accidental leak.
Re (2): This looks a bit awkward to me because other types are passed with
ownership.
Re (3): This should be feasible, but we need to add more WebCallbacks interfaces
for bool type.

     }
 
     virtual void onError() override
     {
         if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
             return;
         m_resolver->reject();
     }
 
 private:
     RefPtrWillBePersistent<ScriptPromiseResolver> m_resolver;
 };
 
 } // namespace blink
 
 #endif