Bug 1617585: Remove the revoked proxy check from ProxyCreate. r=evilpie

Implements the changes from <tc39/ecma262#1814>. Depends on D70814 Differential Revision: https://phabricator.services.mozilla.com/D70815
xeonchen · Apr 14, 2020 · 6b058c9 · 6b058c9
1 parent 69a4d6a
commit 6b058c9
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 37 deletions.
diff --git a/js/src/proxy/ScriptedProxyHandler.cpp b/js/src/proxy/ScriptedProxyHandler.cpp
@@ -1464,12 +1464,8 @@ bool ScriptedProxyHandler::isConstructor(JSObject* obj) const {
 const char ScriptedProxyHandler::family = 0;
 const ScriptedProxyHandler ScriptedProxyHandler::singleton;
 
-bool IsRevokedScriptedProxy(JSObject* obj) {
-  obj = CheckedUnwrapStatic(obj);
-  return obj && IsScriptedProxy(obj) && !obj->as<ProxyObject>().target();
-}
-
-// ES8 rev 0c1bd3004329336774cbc90de727cd0cf5f11e93
+// ES2021 rev c21b280a2c46e92decf3efeca9e9da35d5b9f622
+// Including the changes from: https://github.com/tc39/ecma262/pull/1814
 // 9.5.14 ProxyCreate.
 static bool ProxyCreate(JSContext* cx, CallArgs& args, const char* callerName) {
   if (!args.requireAtLeast(cx, callerName, 2)) {
@@ -1484,48 +1480,34 @@ static bool ProxyCreate(JSContext* cx, CallArgs& args, const char* callerName) {
   }
 
   // Step 2.
-  if (IsRevokedScriptedProxy(target)) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_PROXY_ARG_REVOKED, "1");
-    return false;
-  }
-
-  // Step 3.
   RootedObject handler(cx,
                        RequireObjectArg(cx, "`handler`", callerName, args[1]));
   if (!handler) {
     return false;
   }
 
-  // Step 4.
-  if (IsRevokedScriptedProxy(handler)) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_PROXY_ARG_REVOKED, "2");
-    return false;
-  }
-
-  // Steps 5-6, 8.
+  // Steps 3-4, 6.
   RootedValue priv(cx, ObjectValue(*target));
   JSObject* proxy_ = NewProxyObject(cx, &ScriptedProxyHandler::singleton, priv,
                                     TaggedProto::LazyProto);
   if (!proxy_) {
     return false;
   }
 
-  // Step 9 (reordered).
+  // Step 7 (reordered).
   Rooted<ProxyObject*> proxy(cx, &proxy_->as<ProxyObject>());
   proxy->setReservedSlot(ScriptedProxyHandler::HANDLER_EXTRA,
                          ObjectValue(*handler));
 
-  // Step 7.
+  // Step 5.
   uint32_t callable =
       target->isCallable() ? ScriptedProxyHandler::IS_CALLABLE : 0;
   uint32_t constructor =
       target->isConstructor() ? ScriptedProxyHandler::IS_CONSTRUCTOR : 0;
   proxy->setReservedSlot(ScriptedProxyHandler::IS_CALLCONSTRUCT_EXTRA,
                          PrivateUint32Value(callable | constructor));
 
-  // Step 10.
+  // Step 8.
   args.rval().setObject(*proxy);
   return true;
 }

diff --git a/js/src/tests/jstests.list b/js/src/tests/jstests.list
@@ -468,12 +468,6 @@ skip script test262/language/statements/for-of/iterator-close-throw-get-method-a
 skip script test262/built-ins/AsyncFromSyncIteratorPrototype/return/absent-value-not-passed.js
 skip script test262/built-ins/AsyncFromSyncIteratorPrototype/next/absent-value-not-passed.js
 
-# https://github.com/tc39/ecma262/pull/1814
-# https://bugzilla.mozilla.org/show_bug.cgi?id=1617585
-skip script test262/built-ins/Proxy/create-handler-is-revoked-proxy.js
-skip script test262/built-ins/Proxy/create-target-is-revoked-proxy.js
-skip script test262/built-ins/Proxy/create-target-is-revoked-function-proxy.js
-
 
 ###########################################################
 # Tests disabled due to issues in test262 importer script #

diff --git a/js/src/tests/non262/Proxy/proxy-with-revoked-arguments.js b/js/src/tests/non262/Proxy/proxy-with-revoked-arguments.js
@@ -1,5 +1,5 @@
 var BUGNUMBER = 1151149;
-var summary = "Proxy constructor should throw if either the target or handler is a revoked proxy.";
+var summary = "Proxy constructor should not throw if either the target or handler is a revoked proxy.";
 
 print(BUGNUMBER + ": " + summary);
 
@@ -16,8 +16,8 @@ new Proxy({}, p);
 
 r.revoke();
 
-assertThrowsInstanceOf(() => new Proxy(p, {}), TypeError);
-assertThrowsInstanceOf(() => new Proxy({}, p), TypeError);
+new Proxy(p, {});
+new Proxy({}, p);
 
 
 var r2 = Proxy.revocable({}, {});
@@ -34,8 +34,8 @@ new Proxy({}, p);
 
 r.revoke();
 
-assertThrowsInstanceOf(() => new Proxy(p, {}), TypeError);
-assertThrowsInstanceOf(() => new Proxy({}, p), TypeError);
+new Proxy(p, {});
+new Proxy({}, p);
 
 
 var g = newGlobal();
@@ -46,8 +46,8 @@ new Proxy({}, p);
 
 g.eval(`r.revoke();`);
 
-assertThrowsInstanceOf(() => new Proxy(p, {}), TypeError);
-assertThrowsInstanceOf(() => new Proxy({}, p), TypeError);
+new Proxy(p, {});
+new Proxy({}, p);
 
 if (typeof reportCompare === "function")
   reportCompare(true, true);