Fix toString() behavior on proxy objects.

Proxy objects need special treatment in toString(). Usually, we use the
@@toStringTag, if it is set, otherwise we determine the result of toString()
by checking IsArray() and other internal slots. According to
ES2017 19.1.3.6, IsArray() and the internal slots  must be checked first,
then get(@@toStringTag). The result of IsArray() and internal slots is discarded if
@@toStringTag is set. For proxy
objects, we must obey this order, because get() can have side-effects, i.e.,
revoke the proxy. For all other objects, we can skip the check of the
internal slots, if @@toStringTag is set.

BUG=

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2090773006
Cr-Commit-Position: refs/heads/master@{#37289}

src/objects.cc

@@ -15665,6 +15665,15 @@ MaybeHandle<String> Object::ObjectProtoToString(Isolate* isolate,
   Handle<JSReceiver> receiver =
       Object::ToObject(isolate, object).ToHandleChecked();
 
+  // For proxies, we must check IsArray() before get(toStringTag) to comply
+  // with the specification
+  Maybe<bool> is_array = Nothing<bool>();
+  InstanceType instance_type = receiver->map()->instance_type();
+  if (instance_type == JS_PROXY_TYPE) {
+    is_array = Object::IsArray(receiver);
+    MAYBE_RETURN(is_array, MaybeHandle<String>());
+  }
+
   Handle<String> tag;
   Handle<Object> to_string_tag;
   ASSIGN_RETURN_ON_EXCEPTION(
@@ -15675,8 +15684,6 @@ MaybeHandle<String> Object::ObjectProtoToString(Isolate* isolate,
   if (to_string_tag->IsString()) {
     tag = Handle<String>::cast(to_string_tag);
   } else {
-    InstanceType instance_type = receiver->map()->instance_type();
-
     switch (instance_type) {
       case JS_API_OBJECT_TYPE:
       case JS_SPECIAL_API_OBJECT_TYPE:
@@ -15695,14 +15702,7 @@ MaybeHandle<String> Object::ObjectProtoToString(Isolate* isolate,
         return isolate->factory()->date_to_string();
       case JS_REGEXP_TYPE:
         return isolate->factory()->regexp_to_string();
-
-      // TODO(franzih): According to the specification, isArray() must be run
-      // before get(@@toStringTag). On proxies, isArray() and get() can throw
-      // if the proxy has been revoked, so we change observable behavior
-      // by not obeying the correct order.
       case JS_PROXY_TYPE: {
-        Maybe<bool> is_array = Object::IsArray(receiver);
-        MAYBE_RETURN(is_array, MaybeHandle<String>());
         if (is_array.FromJust()) {
           return isolate->factory()->array_to_string();
         }
@@ -15731,7 +15731,6 @@ MaybeHandle<String> Object::ObjectProtoToString(Isolate* isolate,
       }
       default:
         return isolate->factory()->object_to_string();
-        break;
     }
   }
 

test/mjsunit/es6/object-tostring.js

@@ -156,7 +156,20 @@ assertThrows(() => Object.prototype.toString.call(revocable.proxy), TypeError);
 
 var handler = {};
 revocable = Proxy.revocable([], handler);
+// The first get() call, i.e., toString() revokes the proxy
 handler.get = () => revocable.revoke();
+assertEquals("[object Array]", Object.prototype.toString.call(revocable.proxy));
+assertThrows(() => Object.prototype.toString.call(revocable.proxy), TypeError);
+
+revocable = Proxy.revocable([], handler);
+handler.get = () => {revocable.revoke(); return "value";};
+assertEquals("[object value]", Object.prototype.toString.call(revocable.proxy));
+assertThrows(() => Object.prototype.toString.call(revocable.proxy), TypeError);
+
+
+revocable = Proxy.revocable(function() {}, handler);
+handler.get = () => revocable.revoke();
+assertEquals("[object Function]", Object.prototype.toString.call(revocable.proxy));
 assertThrows(() => Object.prototype.toString.call(revocable.proxy), TypeError);
 
 function* gen() { yield 1; }