Promise constructor should throw when not called with "new"
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Oct 2015 00:51:26 +0000 (00:51 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Oct 2015 00:51:26 +0000 (00:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149380

Reviewed by Darin Adler.

Source/JavaScriptCore:

Implement handling new.target in Promise constructor. And
prohibiting Promise constructor call without "new".

* runtime/JSPromiseConstructor.cpp:
(JSC::constructPromise):
(JSC::callPromise):
(JSC::JSPromiseConstructor::getCallData):
* tests/es6.yaml:
* tests/stress/promise-cannot-be-called.js: Added.
(shouldBe):
(shouldThrow):
(Deferred):
(super):

LayoutTests:

Fix js/dom/Promise-types.html. Before this change, it calls the Promise constructor without new and
expects it succeeds. And we move it from js/dom to js since we can execute this without DOM support.

* js/Promise-types-expected.txt: Renamed from LayoutTests/js/dom/Promise-types-expected.txt.
* js/Promise-types.html: Added.
* js/script-tests/Promise-types.js: Renamed from LayoutTests/js/dom/Promise-types.html.
(aPromise.new.Promise):
(debug.string_appeared_here.shouldThrow.Promise):
* resources/standalone-pre.js:
(shouldBeDefined):

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

LayoutTests/ChangeLog
LayoutTests/js/Promise-types-expected.txt [moved from LayoutTests/js/dom/Promise-types-expected.txt with 69% similarity]
LayoutTests/js/Promise-types.html [new file with mode: 0644]
LayoutTests/js/script-tests/Promise-types.js [moved from LayoutTests/js/dom/Promise-types.html with 66% similarity]
LayoutTests/resources/standalone-pre.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSPromiseConstructor.cpp
Source/JavaScriptCore/tests/es6.yaml
Source/JavaScriptCore/tests/stress/promise-cannot-be-called.js [new file with mode: 0644]

index e9ac285..b9c7bb9 100644 (file)
@@ -1,3 +1,21 @@
+2015-10-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Promise constructor should throw when not called with "new"
+        https://bugs.webkit.org/show_bug.cgi?id=149380
+
+        Reviewed by Darin Adler.
+
+        Fix js/dom/Promise-types.html. Before this change, it calls the Promise constructor without new and
+        expects it succeeds. And we move it from js/dom to js since we can execute this without DOM support.
+
+        * js/Promise-types-expected.txt: Renamed from LayoutTests/js/dom/Promise-types-expected.txt.
+        * js/Promise-types.html: Added.
+        * js/script-tests/Promise-types.js: Renamed from LayoutTests/js/dom/Promise-types.html.
+        (aPromise.new.Promise):
+        (debug.string_appeared_here.shouldThrow.Promise):
+        * resources/standalone-pre.js:
+        (shouldBeDefined):
+
 2015-10-18  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking fast/canvas/webgl/oes-texture-float-linear.html as flaky
similarity index 69%
rename from LayoutTests/js/dom/Promise-types-expected.txt
rename to LayoutTests/js/Promise-types-expected.txt
index 203e64c..b4a6c7f 100644 (file)
@@ -16,26 +16,25 @@ PASS aPromise.catch is defined.
 PASS aPromise.catch is an instance of Function
 PASS aPromise.catch.length is 1
 aPromise2 = Promise(...)
-PASS aPromise2 is an instance of Promise
-PASS String(aPromise2) is '[object Promise]'
+PASS Promise(function(resolve, reject) { resolve(1); }) threw exception TypeError: Type error.
 
 Promise constructor
 
 PASS Promise.length is 1
 PASS new Promise() threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise() threw exception TypeError: Promise constructor takes a function argument.
+PASS Promise() threw exception TypeError: Type error.
 PASS new Promise(1) threw exception TypeError: Promise constructor takes a function argument.
 PASS new Promise('hello') threw exception TypeError: Promise constructor takes a function argument.
 PASS new Promise([]) threw exception TypeError: Promise constructor takes a function argument.
 PASS new Promise({}) threw exception TypeError: Promise constructor takes a function argument.
 PASS new Promise(null) threw exception TypeError: Promise constructor takes a function argument.
 PASS new Promise(undefined) threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise(1) threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise('hello') threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise([]) threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise({}) threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise(null) threw exception TypeError: Promise constructor takes a function argument.
-PASS Promise(undefined) threw exception TypeError: Promise constructor takes a function argument.
+PASS Promise(1) threw exception TypeError: Type error.
+PASS Promise('hello') threw exception TypeError: Type error.
+PASS Promise([]) threw exception TypeError: Type error.
+PASS Promise({}) threw exception TypeError: Type error.
+PASS Promise(null) threw exception TypeError: Type error.
+PASS Promise(undefined) threw exception TypeError: Type error.
 
 Promise statics
 
diff --git a/LayoutTests/js/Promise-types.html b/LayoutTests/js/Promise-types.html
new file mode 100644 (file)
index 0000000..d067bea
--- /dev/null
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/Promise-types.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
similarity index 66%
rename from LayoutTests/js/dom/Promise-types.html
rename to LayoutTests/js/script-tests/Promise-types.js
index 2429fac..215ffac 100644 (file)
@@ -1,17 +1,5 @@
-<!DOCTYPE html>
-<html>
-<head>
-<meta charset="utf-8">
-<script src="../../resources/js-test-pre.js"></script>
-</head>
-<body>
-<script>
-
 description("Promises - Test basic types / exceptions.");
 
-// window.jsTestIsAsync = true;
-
-
 // Promise
 debug("");
 debug("Promises");
@@ -32,11 +20,8 @@ shouldBeDefined("aPromise.catch");
 shouldBeType("aPromise.catch", "Function");
 shouldBe("aPromise.catch.length", "1");
 
-var aPromise2 = Promise(function(resolve, reject) { resolve(1); });
-
 debug("aPromise2 = Promise(...)")
-shouldBeType("aPromise2", "Promise");
-shouldBe("String(aPromise2)", "'[object Promise]'");
+shouldThrow(`Promise(function(resolve, reject) { resolve(1); })`);
 
 // Promise constructor
 debug("");
@@ -56,12 +41,12 @@ shouldThrow("new Promise({})", "'TypeError: Promise constructor takes a function
 shouldThrow("new Promise(null)", "'TypeError: Promise constructor takes a function argument'");
 shouldThrow("new Promise(undefined)", "'TypeError: Promise constructor takes a function argument'");
 
-shouldThrow("Promise(1)", "'TypeError: Promise constructor takes a function argument'");
-shouldThrow("Promise('hello')", "'TypeError: Promise constructor takes a function argument'");
-shouldThrow("Promise([])", "'TypeError: Promise constructor takes a function argument'");
-shouldThrow("Promise({})", "'TypeError: Promise constructor takes a function argument'");
-shouldThrow("Promise(null)", "'TypeError: Promise constructor takes a function argument'");
-shouldThrow("Promise(undefined)", "'TypeError: Promise constructor takes a function argument'");
+shouldThrow("Promise(1)", "'TypeError: Type error'");
+shouldThrow("Promise('hello')", "'TypeError: Type error'");
+shouldThrow("Promise([])", "'TypeError: Type error'");
+shouldThrow("Promise({})", "'TypeError: Type error'");
+shouldThrow("Promise(null)", "'TypeError: Type error'");
+shouldThrow("Promise(undefined)", "'TypeError: Type error'");
 
 // Promise statics
 debug("");
@@ -82,7 +67,4 @@ shouldNotThrow("Promise.reject(1)");
 shouldBeType("Promise.resolve(1)", "Promise");
 shouldBeType("Promise.reject(1)", "Promise");
 
-</script>
-<script src="../../resources/js-test-post.js"></script>
-</body>
-</html>
+
index 8007a0d..7846b4a 100644 (file)
@@ -223,6 +223,24 @@ function shouldBeUndefined(_a)
     testFailed(_a + " should be undefined. Was " + _av);
 }
 
+function shouldBeDefined(_a)
+{
+  var exception;
+  var _av;
+  try {
+     _av = eval(_a);
+  } catch (e) {
+     exception = e;
+  }
+
+  if (exception)
+    testFailed(_a + " should be defined. Threw exception " + exception);
+  else if (_av !== undefined)
+    testPassed(_a + " is defined.");
+  else
+    testFailed(_a + " should be defined. Was " + _av);
+}
+
 function shouldNotThrow(_a) {
     try {
         eval(_a);
index f239fc6..8cbe9ed 100644 (file)
@@ -1,5 +1,26 @@
 2015-10-18  Yusuke Suzuki  <utatane.tea@gmail.com>
 
+        Promise constructor should throw when not called with "new"
+        https://bugs.webkit.org/show_bug.cgi?id=149380
+
+        Reviewed by Darin Adler.
+
+        Implement handling new.target in Promise constructor. And
+        prohibiting Promise constructor call without "new".
+
+        * runtime/JSPromiseConstructor.cpp:
+        (JSC::constructPromise):
+        (JSC::callPromise):
+        (JSC::JSPromiseConstructor::getCallData):
+        * tests/es6.yaml:
+        * tests/stress/promise-cannot-be-called.js: Added.
+        (shouldBe):
+        (shouldThrow):
+        (Deferred):
+        (super):
+
+2015-10-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
         [ES6] Handle asynchronous tests in tests/es6
         https://bugs.webkit.org/show_bug.cgi?id=150293
 
index 5351f50..1e0e693 100644 (file)
@@ -89,12 +89,25 @@ static EncodedJSValue JSC_HOST_CALL constructPromise(ExecState* exec)
     JSGlobalObject* globalObject = exec->callee()->globalObject();
     VM& vm = exec->vm();
 
+    JSValue newTarget = exec->newTarget();
+    if (!newTarget || newTarget.isUndefined())
+        return throwVMTypeError(exec);
+
     JSPromise* promise = JSPromise::create(vm, globalObject->promiseStructure());
+    if (!jsDynamicCast<JSPromiseConstructor*>(newTarget)) {
+        JSValue proto = asObject(newTarget)->getDirect(vm, vm.propertyNames->prototype);
+        asObject(promise)->setPrototypeWithCycleCheck(exec, proto);
+    }
     promise->initialize(exec, globalObject, exec->argument(0));
 
     return JSValue::encode(promise);
 }
 
+static EncodedJSValue JSC_HOST_CALL callPromise(ExecState* exec)
+{
+    return throwVMTypeError(exec);
+}
+
 ConstructType JSPromiseConstructor::getConstructData(JSCell*, ConstructData& constructData)
 {
     constructData.native.function = constructPromise;
@@ -103,7 +116,11 @@ ConstructType JSPromiseConstructor::getConstructData(JSCell*, ConstructData& con
 
 CallType JSPromiseConstructor::getCallData(JSCell*, CallData& callData)
 {
-    callData.native.function = constructPromise;
+    // FIXME: This is workaround. Since JSC does not expose @isConstructor to JS builtins,
+    // we use typeof function === "function" now. And since typeof constructorWithoutCallability
+    // returns "object", we need to define [[Call]] for now.
+    // https://bugs.webkit.org/show_bug.cgi?id=144093
+    callData.native.function = callPromise;
     return CallTypeHost;
 }
 
index 211294f..1d450d5 100644 (file)
   cmd: runES6 :normal
 - path: es6/Promise_is_subclassable_basic_functionality.js
   cmd: runES6 :normal
+- path: es6/Promise_constructor_requires_new.js
+  cmd: runES6 :normal
+- path: es6/Promise_is_subclassable_correct_prototype_chain.js
+  cmd: runES6 :normal
 - path: es6/Reflect_Reflect.apply.js
   cmd: runES6 :normal
 - path: es6/Reflect_Reflect.defineProperty.js
   cmd: runES6 :fail
 - path: es6/non-strict_function_semantics_hoisted_block-level_function_declaration.js
   cmd: runES6 :fail
-- path: es6/Promise_constructor_requires_new.js
-  cmd: runES6 :fail
-- path: es6/Promise_is_subclassable_correct_prototype_chain.js
-  cmd: runES6 :fail
 - path: es6/Promise_is_subclassable_Promise.all.js
   cmd: runES6 :fail
 - path: es6/Promise_is_subclassable_Promise.race.js
diff --git a/Source/JavaScriptCore/tests/stress/promise-cannot-be-called.js b/Source/JavaScriptCore/tests/stress/promise-cannot-be-called.js
new file mode 100644 (file)
index 0000000..c5f91ab
--- /dev/null
@@ -0,0 +1,44 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error(`bad value: {String(actual)}`);
+}
+
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+var executorCalled = false;
+shouldThrow(() => {
+    Promise(function (resolve, reject) { executorCalled = true; });
+}, `TypeError: Type error`);
+shouldBe(executorCalled, false);
+
+// But should accept inheriting Promise.
+class Deferred extends Promise {
+    constructor()
+    {
+        let resolve, reject;
+        super(function (aResolve, aReject) {
+            this.resolve = aResolve;
+            this.reject = aReject;
+        });
+    }
+}
+
+{
+    let deferred = new Deferred();
+    shouldBe(deferred.__proto__, Deferred.prototype);
+    shouldBe(deferred.__proto__.__proto__, Promise.prototype);
+    shouldBe(Deferred.__proto__, Promise);
+}