[js weak refs] Implement spec change: cleanup must be callable

See https://github.com/tc39/proposal-weakrefs/issues/37

Drive-by: fix error messages.

BUG=v8:8179

Change-Id: I8608d09ec5a58c8b62eea4580be9415f6bb41586
Reviewed-on: https://chromium-review.googlesource.com/c/1319758
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57347}

src/builtins/builtins-weak-refs.cc

@@ -20,13 +20,16 @@ BUILTIN(WeakFactoryConstructor) {
   Handle<JSReceiver> new_target = Handle<JSReceiver>::cast(args.new_target());
   Handle<Object> cleanup = args.atOrUndefined(isolate, 1);
 
+  if (!cleanup->IsCallable()) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kWeakRefsCleanupMustBeCallable));
+  }
+
   Handle<JSObject> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result,
       JSObject::New(target, new_target, Handle<AllocationSite>::null()));
 
-  // TODO(marja): (spec) here we could return an error if cleanup is not a
-  // function, if the spec said so.
   Handle<JSWeakFactory> weak_factory = Handle<JSWeakFactory>::cast(result);
   weak_factory->set_native_context(*isolate->native_context());
   weak_factory->set_cleanup(*cleanup);
@@ -44,9 +47,8 @@ BUILTIN(WeakFactoryMakeCell) {
   Handle<Object> target = args.atOrUndefined(isolate, 1);
   if (!target->IsJSReceiver()) {
     THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kMakeCellTargetMustBeObject,
-                              isolate->factory()->NewStringFromAsciiChecked(
-                                  method_name)));
+        isolate,
+        NewTypeError(MessageTemplate::kWeakRefsMakeCellTargetMustBeObject));
   }
   Handle<JSReceiver> target_receiver = Handle<JSReceiver>::cast(target);
   Handle<Object> holdings = args.atOrUndefined(isolate, 2);
@@ -54,8 +56,7 @@ BUILTIN(WeakFactoryMakeCell) {
     THROW_NEW_ERROR_RETURN_FAILURE(
         isolate,
         NewTypeError(
-            MessageTemplate::kMakeCellTargetAndHoldingsMustNotBeSame,
-            isolate->factory()->NewStringFromAsciiChecked(method_name)));
+            MessageTemplate::kWeakRefsMakeCellTargetAndHoldingsMustNotBeSame));
   }
 
   // TODO(marja): Realms.
@@ -84,9 +85,8 @@ BUILTIN(WeakFactoryMakeRef) {
   Handle<Object> target = args.atOrUndefined(isolate, 1);
   if (!target->IsJSReceiver()) {
     THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kMakeRefTargetMustBeObject,
-                              isolate->factory()->NewStringFromAsciiChecked(
-                                  method_name)));
+        isolate,
+        NewTypeError(MessageTemplate::kWeakRefsMakeRefTargetMustBeObject));
   }
   Handle<JSReceiver> target_receiver = Handle<JSReceiver>::cast(target);
   Handle<Object> holdings = args.atOrUndefined(isolate, 2);
@@ -94,8 +94,7 @@ BUILTIN(WeakFactoryMakeRef) {
     THROW_NEW_ERROR_RETURN_FAILURE(
         isolate,
         NewTypeError(
-            MessageTemplate::kMakeRefTargetAndHoldingsMustNotBeSame,
-            isolate->factory()->NewStringFromAsciiChecked(method_name)));
+            MessageTemplate::kWeakRefsMakeRefTargetAndHoldingsMustNotBeSame));
   }
 
   // TODO(marja): Realms.

src/message-template.h

@@ -534,14 +534,15 @@ namespace internal {
   T(TraceEventPhaseError, "Trace event phase must be a number.")               \
   T(TraceEventIDError, "Trace event id must be a number.")                     \
   /* Weak refs */                                                              \
-  T(MakeCellTargetMustBeObject,                                                \
-    "WeakFactory.makeCell: target must be an object")                          \
-  T(MakeCellTargetAndHoldingsMustNotBeSame,                                    \
-    "WeakFactory.makeCell: target and holdings must not be same")              \
-  T(MakeRefTargetMustBeObject,                                                 \
-    "WeakFactory.makeRef: target must be an object")                           \
-  T(MakeRefTargetAndHoldingsMustNotBeSame,                                     \
-    "WeakFactory.makeRef: target and holdings must not be same")
+  T(WeakRefsCleanupMustBeCallable, "WeakFactory: cleanup must be callable")    \
+  T(WeakRefsMakeCellTargetMustBeObject,                                        \
+    "WeakFactory.prototype.makeCell: target must be an object")                \
+  T(WeakRefsMakeCellTargetAndHoldingsMustNotBeSame,                            \
+    "WeakFactory.prototype.makeCell: target and holdings must not be same")    \
+  T(WeakRefsMakeRefTargetMustBeObject,                                         \
+    "WeakFactory.prototype.makeRef: target must be an object")                 \
+  T(WeakRefsMakeRefTargetAndHoldingsMustNotBeSame,                             \
+    "WeakFactory.prototype.makeRef: target and holdings must not be same")
 
 enum class MessageTemplate {
 #define TEMPLATE(NAME, STRING) k##NAME,

test/message/fail/weak-refs-makecell1.js

@@ -4,5 +4,5 @@
 
 // Flags: --harmony-weak-refs
 
-let wf = new WeakFactory();
+let wf = new WeakFactory(() => {});
 wf.makeCell(1);

test/message/fail/weak-refs-makecell1.out

-*%(basename)s:8: TypeError: WeakFactory.makeCell: target must be an object
+*%(basename)s:8: TypeError: WeakFactory.prototype.makeCell: target must be an object
 wf.makeCell(1);
    ^
-TypeError: WeakFactory.makeCell: target must be an object
+TypeError: WeakFactory.prototype.makeCell: target must be an object
     at WeakFactory.makeCell (<anonymous>)
     at *%(basename)s:8:4

test/message/fail/weak-refs-makecell2.js

@@ -4,6 +4,6 @@
 
 // Flags: --harmony-weak-refs
 
-let wf = new WeakFactory();
+let wf = new WeakFactory(() => {});
 let o = {};
 wf.makeCell(o, o);

test/message/fail/weak-refs-makecell2.out

-*%(basename)s:9: TypeError: WeakFactory.makeCell: target and holdings must not be same
+*%(basename)s:9: TypeError: WeakFactory.prototype.makeCell: target and holdings must not be same
 wf.makeCell(o, o);
    ^
-TypeError: WeakFactory.makeCell: target and holdings must not be same
+TypeError: WeakFactory.prototype.makeCell: target and holdings must not be same
     at WeakFactory.makeCell (<anonymous>)
     at *%(basename)s:9:4

test/message/fail/weak-refs-weakfactory1.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-weak-refs
+
+let wf = new WeakFactory();

test/message/fail/weak-refs-weakfactory1.out

+*%(basename)s:7: TypeError: WeakFactory: cleanup must be callable
+let wf = new WeakFactory();
+         ^
+TypeError: WeakFactory: cleanup must be callable
+    at new WeakFactory (<anonymous>)
+    at *%(basename)s:7:10

test/message/fail/weak-refs-weakfactory2.js

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-weak-refs
+
+let wf = new WeakFactory({});

test/message/fail/weak-refs-weakfactory2.out

+*%(basename)s:7: TypeError: WeakFactory: cleanup must be callable
+let wf = new WeakFactory({});
+         ^
+TypeError: WeakFactory: cleanup must be callable
+    at new WeakFactory (<anonymous>)
+    at *%(basename)s:7:10

test/mjsunit/harmony/weakrefs/basics.js

@@ -5,7 +5,7 @@
 // Flags: --harmony-weak-refs
 
 (function TestConstructWeakFactory() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   assertEquals(wf.toString(), "[object WeakFactory]");
   assertNotSame(wf.__proto__, Object.prototype);
   assertSame(wf.__proto__.__proto__, Object.prototype);
@@ -15,7 +15,7 @@
   let caught = false;
   let message = "";
   try {
-    let f = WeakFactory();
+    let f = WeakFactory(() => {});
   } catch (e) {
     message = e.message;
     caught = true;
@@ -25,8 +25,30 @@
   }
 })();
 
+(function TestConstructWeakFactoryCleanupNotCallable() {
+  let message = "WeakFactory: cleanup must be callable";
+  assertThrows(() => { let wf = new WeakFactory(); }, TypeError, message);
+  assertThrows(() => { let wf = new WeakFactory(1); }, TypeError, message);
+  assertThrows(() => { let wf = new WeakFactory(null); }, TypeError, message);
+})();
+
+(function TestConstructWeakFactoryWithCallableProxyAsCleanup() {
+  let handler = {};
+  let obj = () => {};
+  let proxy = new Proxy(obj, handler);
+  let wf = new WeakFactory(proxy);
+})();
+
+(function TestConstructWeakFactoryWithNonCallableProxyAsCleanup() {
+  let message = "WeakFactory: cleanup must be callable";
+  let handler = {};
+  let obj = {};
+  let proxy = new Proxy(obj, handler);
+  assertThrows(() => { let wf = new WeakFactory(proxy); }, TypeError, message);
+})();
+
 (function TestMakeCell() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});
   assertEquals(wc.toString(), "[object WeakCell]");
   assertNotSame(wc.__proto__, Object.prototype);
@@ -46,7 +68,7 @@
 })();
 
 (function TestMakeCellWithHoldings() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wc = wf.makeCell(obj, holdings);
@@ -54,7 +76,7 @@
 })();
 
 (function TestMakeCellWithHoldingsSetHoldings() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wc = wf.makeCell(obj, holdings);
@@ -65,7 +87,7 @@
 
 (function TestMakeCellWithHoldingsSetHoldingsStrict() {
   "use strict";
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wc = wf.makeCell(obj, holdings);
@@ -75,8 +97,8 @@
 })();
 
 (function TestMakeCellWithNonObject() {
-  let wf = new WeakFactory();
-  let message = "WeakFactory.makeCell: target must be an object";
+  let wf = new WeakFactory(() => {});
+  let message = "WeakFactory.prototype.makeCell: target must be an object";
   assertThrows(() => wf.makeCell(), TypeError, message);
   assertThrows(() => wf.makeCell(1), TypeError, message);
   assertThrows(() => wf.makeCell(false), TypeError, message);
@@ -90,16 +112,16 @@
   let handler = {};
   let obj = {};
   let proxy = new Proxy(obj, handler);
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell(proxy);
 })();
 
 (function TestMakeCellTargetAndHoldingsSameValue() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   // SameValue(target, holdings) not ok
   assertThrows(() => wf.makeCell(obj, obj), TypeError,
-               "WeakFactory.makeCell: target and holdings must not be same");
+               "WeakFactory.prototype.makeCell: target and holdings must not be same");
   let holdings = {a: 1};
   let wc = wf.makeCell(obj, holdings);
 })();
@@ -107,12 +129,12 @@
 (function TestMakeCellWithoutWeakFactory() {
   assertThrows(() => WeakFactory.prototype.makeCell.call({}, {}), TypeError);
   // Does not throw:
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   WeakFactory.prototype.makeCell.call(wf, {});
 })();
 
 (function TestHoldingsWithoutWeakCell() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});
   let holdings_getter = Object.getOwnPropertyDescriptor(wc.__proto__, "holdings").get;
   assertThrows(() => holdings_getter.call({}), TypeError);
@@ -121,7 +143,7 @@
 })();
 
 (function TestClearWithoutWeakCell() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});
   let clear = Object.getOwnPropertyDescriptor(wc.__proto__, "clear").value;
   assertThrows(() => clear.call({}), TypeError);
@@ -130,7 +152,7 @@
 })();
 
 (function TestMakeRef() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wr = wf.makeRef({});
   let wc = wf.makeCell({});
   assertEquals(wr.toString(), "[object WeakRef]");
@@ -145,7 +167,7 @@
 })();
 
 (function TestMakeRefWithHoldings() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wr = wf.makeRef(obj, holdings);
@@ -153,7 +175,7 @@
 })();
 
 (function TestMakeRefWithHoldingsSetHoldings() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wr = wf.makeRef(obj, holdings);
@@ -164,7 +186,7 @@
 
 (function TestMakeRefWithHoldingsSetHoldingsStrict() {
   "use strict";
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   let holdings = {b: 2};
   let wr = wf.makeRef(obj, holdings);
@@ -174,8 +196,8 @@
 })();
 
 (function TestMakeRefWithNonObject() {
-  let wf = new WeakFactory();
-  let message = "WeakFactory.makeRef: target must be an object";
+  let wf = new WeakFactory(() => {});
+  let message = "WeakFactory.prototype.makeRef: target must be an object";
   assertThrows(() => wf.makeRef(), TypeError, message);
   assertThrows(() => wf.makeRef(1), TypeError, message);
   assertThrows(() => wf.makeRef(false), TypeError, message);
@@ -189,16 +211,16 @@
   let handler = {};
   let obj = {};
   let proxy = new Proxy(obj, handler);
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wr = wf.makeRef(proxy);
 })();
 
 (function TestMakeRefTargetAndHoldingsSameValue() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let obj = {a: 1};
   // SameValue(target, holdings) not ok
   assertThrows(() => wf.makeRef(obj, obj), TypeError,
-               "WeakFactory.makeRef: target and holdings must not be same");
+               "WeakFactory.prototype.makeRef: target and holdings must not be same");
   let holdings = {a: 1};
   let wr = wf.makeRef(obj, holdings);
 })();
@@ -206,12 +228,12 @@
 (function TestMakeRefWithoutWeakFactory() {
   assertThrows(() => WeakFactory.prototype.makeRef.call({}, {}), TypeError);
   // Does not throw:
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   WeakFactory.prototype.makeRef.call(wf, {});
 })();
 
 (function TestDerefWithoutWeakRef() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});
   let wr = wf.makeRef({});
   let deref = Object.getOwnPropertyDescriptor(wr.__proto__, "deref").value;
@@ -222,7 +244,7 @@
 })();
 
 (function TestWeakRefClearAfterProtoChange() {
-  let wf = new WeakFactory();
+  let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});
   let wr = wf.makeRef({});
   // Does not throw: