ScriptPromise should check the constructor paraemter.

ScriptPromise::ScriptPromise(v8::Handle<v8::Value>, v8::Isolate*) expects
that the first parameter is a Promise object, but no one checks that.
This CL fixes that.

Rename ScriptPromise::ScriptPromise(ScriptValue) to ScriptPromise::cast to
avoid the confusion caused by the diferrent behaviors with a non-Promise parameter.

BUG=347047

Review URL: https://codereview.chromium.org/181173002

git-svn-id: svn://svn.chromium.org/blink/trunk@168462 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/bindings/v8/ScriptPromise.cpp

@@ -39,6 +39,16 @@
 
 namespace WebCore {
 
+ScriptPromise::ScriptPromise(v8::Handle<v8::Value> value, v8::Isolate* isolate)
+{
+    if (value.IsEmpty() || !V8PromiseCustom::isPromise(value, isolate)) {
+        m_promise = ScriptValue(v8::Handle<v8::Value>(), isolate);
+        V8ThrowException::throwTypeError("the given value is not a Promise", isolate);
+        return;
+    }
+    m_promise = ScriptValue(value, isolate);
+}
+
 ScriptPromise ScriptPromise::then(PassOwnPtr<ScriptFunction> onFulfilled, PassOwnPtr<ScriptFunction> onRejected)
 {
     if (m_promise.hasNoValue() || !m_promise.isObject())
@@ -47,17 +57,12 @@ ScriptPromise ScriptPromise::then(PassOwnPtr<ScriptFunction> onFulfilled, PassOw
     return ScriptPromise(V8PromiseCustom::then(promise, adoptByGarbageCollector(onFulfilled), adoptByGarbageCollector(onRejected), isolate()), isolate());
 }
 
-ScriptPromise::ScriptPromise(const ScriptValue& value)
+ScriptPromise ScriptPromise::createPending()
 {
-    if (value.hasNoValue())
-        return;
-    v8::Local<v8::Value> v8Value(value.v8Value());
-    v8::Isolate* isolate = value.isolate();
-    if (V8PromiseCustom::isPromise(v8Value, isolate)) {
-        m_promise = value;
-        return;
-    }
-    m_promise = ScriptValue(V8PromiseCustom::toPromise(v8Value, isolate), isolate);
+    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+    ASSERT(isolate->InContext());
+    v8::Handle<v8::Object> promise = V8PromiseCustom::createPromise(v8::Object::New(isolate), isolate);
+    return ScriptPromise(promise, isolate);
 }
 
 ScriptPromise ScriptPromise::createPending(ExecutionContext* context)
@@ -71,12 +76,16 @@ ScriptPromise ScriptPromise::createPending(ExecutionContext* context)
     return ScriptPromise(promise, isolate);
 }
 
-ScriptPromise ScriptPromise::createPending()
+ScriptPromise ScriptPromise::cast(const ScriptValue& value)
 {
-    v8::Isolate* isolate = v8::Isolate::GetCurrent();
-    ASSERT(isolate->InContext());
-    v8::Handle<v8::Object> promise = V8PromiseCustom::createPromise(v8::Object::New(isolate), isolate);
-    return ScriptPromise(promise, isolate);
+    if (value.hasNoValue())
+        return ScriptPromise();
+    v8::Local<v8::Value> v8Value(value.v8Value());
+    v8::Isolate* isolate = value.isolate();
+    if (V8PromiseCustom::isPromise(v8Value, isolate)) {
+        return ScriptPromise(v8Value, isolate);
+    }
+    return ScriptPromise(V8PromiseCustom::toPromise(v8Value, isolate), isolate);
 }
 
 } // namespace WebCore

third_party/WebKit/Source/bindings/v8/ScriptPromise.h

@@ -52,17 +52,9 @@ public:
     // Constructs an empty promise.
     ScriptPromise() { }
 
-    // Constructs a ScriptPromise from |value|.
-    // i.e. the constructed ScriptPromise holds |Promise.cast(value)|.
-    // Note: if |value| is a non-Promise value, two ScriptPromises constructed
-    // with it hold different Promises each other.
-    explicit ScriptPromise(const ScriptValue& /* value */);
-
-    ScriptPromise(v8::Handle<v8::Value> promise, v8::Isolate* isolate)
-        : m_promise(promise, isolate)
-    {
-        ASSERT(!m_promise.hasNoValue());
-    }
+    // Constructs a ScriptPromise from |promise|.
+    // If |promise| is not a Promise object, throws a v8 TypeError.
+    ScriptPromise(v8::Handle<v8::Value> promise, v8::Isolate*);
 
     ScriptPromise then(PassOwnPtr<ScriptFunction> onFulfilled, PassOwnPtr<ScriptFunction> onRejected = PassOwnPtr<ScriptFunction>());
 
@@ -103,6 +95,10 @@ public:
 
     static ScriptPromise createPending();
     static ScriptPromise createPending(ExecutionContext*);
+    // Constructs and returns a ScriptPromise from |value|.
+    // if |value| is not a Promise object, returns a Promise object
+    // resolved with |value|.
+    static ScriptPromise cast(const ScriptValue& /*value*/);
 
 private:
     ScriptValue m_promise;

third_party/WebKit/Source/bindings/v8/ScriptPromiseTest.cpp

@@ -43,21 +43,20 @@ namespace WebCore {
 
 namespace {
 
+void callback(const v8::FunctionCallbackInfo<v8::Value>& info) { }
+
 class ScriptPromiseTest : public testing::Test {
 public:
     ScriptPromiseTest()
         : m_isolate(v8::Isolate::GetCurrent())
-    {
-    }
-
-    void SetUp()
     {
         m_scope = V8BindingTestScope::create(m_isolate);
     }
 
-    void TearDown()
+    ~ScriptPromiseTest()
     {
-        m_scope.clear();
+        // FIXME: We put this statement here to clear an exception from the isolate.
+        createClosure(callback, v8::Undefined(m_isolate), m_isolate);
     }
 
     V8PromiseCustom::PromiseState state(ScriptPromise promise)
@@ -72,10 +71,18 @@ private:
     OwnPtr<V8BindingTestScope> m_scope;
 };
 
+TEST_F(ScriptPromiseTest, constructFromNonPromise)
+{
+    v8::TryCatch trycatch;
+    ScriptPromise promise(v8::Undefined(m_isolate), m_isolate);
+    ASSERT_TRUE(trycatch.HasCaught());
+    ASSERT_TRUE(promise.hasNoValue());
+}
+
 TEST_F(ScriptPromiseTest, castPromise)
 {
     ScriptPromise promise = ScriptPromise::createPending();
-    ScriptPromise newPromise(ScriptValue(promise.v8Value(), m_isolate));
+    ScriptPromise newPromise = ScriptPromise::cast(ScriptValue(promise.v8Value(), m_isolate));
 
     ASSERT_FALSE(promise.hasNoValue());
     EXPECT_EQ(V8PromiseCustom::Pending, state(promise));
@@ -85,8 +92,8 @@ TEST_F(ScriptPromiseTest, castPromise)
 TEST_F(ScriptPromiseTest, castNonPromise)
 {
     ScriptValue value = ScriptValue(v8String(m_isolate, "hello"), m_isolate);
-    ScriptPromise promise1(ScriptValue(value.v8Value(), m_isolate));
-    ScriptPromise promise2(ScriptValue(value.v8Value(), m_isolate));
+    ScriptPromise promise1 = ScriptPromise::cast(ScriptValue(value.v8Value(), m_isolate));
+    ScriptPromise promise2 = ScriptPromise::cast(ScriptValue(value.v8Value(), m_isolate));
 
     ASSERT_FALSE(promise1.hasNoValue());
     ASSERT_FALSE(promise2.hasNoValue());

third_party/WebKit/Source/modules/serviceworkers/RespondWithObserver.cpp

@@ -81,7 +81,7 @@ void RespondWithObserver::respondWith(const ScriptValue& value)
         return;
 
     m_state = Pending;
-    ScriptPromise(value).then(
+    ScriptPromise::cast(value).then(
         ThenFunction::create(this, ThenFunction::Fulfilled),
         ThenFunction::create(this, ThenFunction::Rejected));
 }

third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp

@@ -75,7 +75,7 @@ void WaitUntilObserver::didDispatchEvent()
 void WaitUntilObserver::waitUntil(const ScriptValue& value)
 {
     incrementPendingActivity();
-    ScriptPromise(value).then(
+    ScriptPromise::cast(value).then(
         ThenFunction::create(this, ThenFunction::Fulfilled),
         ThenFunction::create(this, ThenFunction::Rejected));
 }