[proxies] Support proxies in JSON.parse and JSON.stringify.

This CL tries to correctly support the following:
- stringifying a proxy,
- stringifying with a proxy as replacer (callable or arraylike),
- stringifying with a replacer that returns a proxy,
- parsing with a callable proxy as reviver,
- parsing with a reviver that inserts proxies into the object,
- and whatever else you can imagine.

This also fixes some bugs observable without proxies.

BUG=v8:3139,v8:1543
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#32843}

src/bootstrapper.cc

@@ -364,17 +364,41 @@ void Bootstrapper::DetachGlobal(Handle<Context> env) {
 
 namespace {
 
-Handle<JSFunction> InstallFunction(Handle<JSObject> target,
-                                   Handle<Name> property_name,
-                                   Handle<JSFunction> function,
-                                   Handle<String> function_name,
-                                   PropertyAttributes attributes = DONT_ENUM) {
+void InstallFunction(Handle<JSObject> target, Handle<Name> property_name,
+                     Handle<JSFunction> function, Handle<String> function_name,
+                     PropertyAttributes attributes = DONT_ENUM) {
   JSObject::AddProperty(target, property_name, function, attributes);
   if (target->IsJSGlobalObject()) {
     function->shared()->set_instance_class_name(*function_name);
   }
   function->shared()->set_native(true);
-  return function;
+}
+
+
+static void InstallFunction(Handle<JSObject> target,
+                            Handle<JSFunction> function, Handle<Name> name,
+                            PropertyAttributes attributes = DONT_ENUM) {
+  Handle<String> name_string = Name::ToFunctionName(name).ToHandleChecked();
+  InstallFunction(target, name, function, name_string, attributes);
+}
+
+
+static Handle<JSFunction> CreateFunction(Isolate* isolate, Handle<String> name,
+                                         InstanceType type, int instance_size,
+                                         MaybeHandle<JSObject> maybe_prototype,
+                                         Builtins::Name call,
+                                         bool strict_function_map = false) {
+  Factory* factory = isolate->factory();
+  Handle<Code> call_code(isolate->builtins()->builtin(call));
+  Handle<JSObject> prototype;
+  static const bool kReadOnlyPrototype = false;
+  static const bool kInstallConstructor = false;
+  return maybe_prototype.ToHandle(&prototype)
+             ? factory->NewFunction(name, call_code, prototype, type,
+                                    instance_size, kReadOnlyPrototype,
+                                    kInstallConstructor, strict_function_map)
+             : factory->NewFunctionWithoutPrototype(name, call_code,
+                                                    strict_function_map);
 }
 
 
@@ -384,21 +408,12 @@ Handle<JSFunction> InstallFunction(Handle<JSObject> target, Handle<Name> name,
                                    Builtins::Name call,
                                    PropertyAttributes attributes,
                                    bool strict_function_map = false) {
-  Isolate* isolate = target->GetIsolate();
-  Factory* factory = isolate->factory();
   Handle<String> name_string = Name::ToFunctionName(name).ToHandleChecked();
-  Handle<Code> call_code(isolate->builtins()->builtin(call));
-  Handle<JSObject> prototype;
-  static const bool kReadOnlyPrototype = false;
-  static const bool kInstallConstructor = false;
   Handle<JSFunction> function =
-      maybe_prototype.ToHandle(&prototype)
-          ? factory->NewFunction(name_string, call_code, prototype, type,
-                                 instance_size, kReadOnlyPrototype,
-                                 kInstallConstructor, strict_function_map)
-          : factory->NewFunctionWithoutPrototype(name_string, call_code,
-                                                 strict_function_map);
-  return InstallFunction(target, name, function, name_string, attributes);
+      CreateFunction(target->GetIsolate(), name_string, type, instance_size,
+                     maybe_prototype, call, strict_function_map);
+  InstallFunction(target, name, function, name_string, attributes);
+  return function;
 }
 
 
@@ -1028,17 +1043,31 @@ void Genesis::HookUpGlobalObject(Handle<JSGlobalObject> global_object) {
 }
 
 
-static void SimpleInstallFunction(Handle<JSObject> base, Handle<Name> name,
-                                  Builtins::Name call, int len, bool adapt) {
+static Handle<JSFunction> SimpleCreateFunction(Isolate* isolate,
+                                               Handle<String> name,
+                                               Builtins::Name call, int len,
+                                               bool adapt) {
   Handle<JSFunction> fun =
-      InstallFunction(base, name, JS_OBJECT_TYPE, JSObject::kHeaderSize,
-                      MaybeHandle<JSObject>(), call, DONT_ENUM);
+      CreateFunction(isolate, name, JS_OBJECT_TYPE, JSObject::kHeaderSize,
+                     MaybeHandle<JSObject>(), call);
   if (adapt) {
     fun->shared()->set_internal_formal_parameter_count(len);
   } else {
     fun->shared()->DontAdaptArguments();
   }
   fun->shared()->set_length(len);
+  return fun;
+}
+
+
+static Handle<JSFunction> SimpleInstallFunction(Handle<JSObject> base,
+                                                Handle<String> name,
+                                                Builtins::Name call, int len,
+                                                bool adapt) {
+  Handle<JSFunction> fun =
+      SimpleCreateFunction(base->GetIsolate(), name, call, len, adapt);
+  InstallFunction(base, fun, name, DONT_ENUM);
+  return fun;
 }
 
 
@@ -1146,9 +1175,10 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     initial_strong_map->set_is_strong();
     CacheInitialJSArrayMaps(native_context(), initial_strong_map);
 
-    SimpleInstallFunction(array_function,
-                          isolate->factory()->InternalizeUtf8String("isArray"),
-                          Builtins::kArrayIsArray, 1, true);
+    Handle<JSFunction> is_arraylike = SimpleInstallFunction(
+        array_function, isolate->factory()->InternalizeUtf8String("isArray"),
+        Builtins::kArrayIsArray, 1, true);
+    native_context()->set_is_arraylike(*is_arraylike);
   }
 
   {  // --- N u m b e r ---
@@ -2157,9 +2187,23 @@ void Genesis::InitializeGlobal_harmony_regexp_subclass() {
 
 
 void Genesis::InitializeGlobal_harmony_reflect() {
+  Factory* factory = isolate()->factory();
+
+  // We currently use some of the Reflect functions internally, even when
+  // the --harmony-reflect flag is not given.
+
+  Handle<JSFunction> define_property =
+      SimpleCreateFunction(isolate(), factory->defineProperty_string(),
+                           Builtins::kReflectDefineProperty, 3, true);
+  native_context()->set_reflect_define_property(*define_property);
+
+  Handle<JSFunction> delete_property =
+      SimpleCreateFunction(isolate(), factory->deleteProperty_string(),
+                           Builtins::kReflectDeleteProperty, 2, true);
+  native_context()->set_reflect_delete_property(*delete_property);
+
   if (!FLAG_harmony_reflect) return;
 
-  Factory* factory = isolate()->factory();
   Handle<JSGlobalObject> global(JSGlobalObject::cast(
       native_context()->global_object()));
   Handle<String> reflect_string = factory->NewStringFromStaticChars("Reflect");
@@ -2167,10 +2211,9 @@ void Genesis::InitializeGlobal_harmony_reflect() {
       factory->NewJSObject(isolate()->object_function(), TENURED);
   JSObject::AddProperty(global, reflect_string, reflect, DONT_ENUM);
 
-  SimpleInstallFunction(reflect, factory->defineProperty_string(),
-                        Builtins::kReflectDefineProperty, 3, true);
-  SimpleInstallFunction(reflect, factory->deleteProperty_string(),
-                        Builtins::kReflectDeleteProperty, 2, true);
+  InstallFunction(reflect, define_property, factory->defineProperty_string());
+  InstallFunction(reflect, delete_property, factory->deleteProperty_string());
+
   SimpleInstallFunction(reflect, factory->get_string(),
                         Builtins::kReflectGet, 2, false);
   SimpleInstallFunction(reflect, factory->getOwnPropertyDescriptor_string(),

src/contexts.h

@@ -78,12 +78,15 @@ enum BindingFlags {
 // Factory::NewContext.
 
 #define NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(V)                             \
+  V(IS_ARRAYLIKE, JSFunction, is_arraylike)                               \
   V(CONCAT_ITERABLE_TO_ARRAY_INDEX, JSFunction, concat_iterable_to_array) \
   V(GET_TEMPLATE_CALL_SITE_INDEX, JSFunction, get_template_call_site)     \
   V(MAKE_RANGE_ERROR_INDEX, JSFunction, make_range_error)                 \
   V(MAKE_TYPE_ERROR_INDEX, JSFunction, make_type_error)                   \
   V(REFLECT_APPLY_INDEX, JSFunction, reflect_apply)                       \
   V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct)               \
+  V(REFLECT_DEFINE_PROPERTY_INDEX, JSFunction, reflect_define_property)   \
+  V(REFLECT_DELETE_PROPERTY_INDEX, JSFunction, reflect_delete_property)   \
   V(SPREAD_ARGUMENTS_INDEX, JSFunction, spread_arguments)                 \
   V(SPREAD_ITERABLE_INDEX, JSFunction, spread_iterable)
 

src/js/json.js

@@ -12,6 +12,7 @@
 // Imports
 
 var GlobalJSON = global.JSON;
+var GlobalSet = global.Set;
 var InternalArray = utils.InternalArray;
 var MakeTypeError;
 var MaxSimple;
@@ -30,27 +31,33 @@ utils.Import(function(from) {
 
 // -------------------------------------------------------------------
 
+function CreateDataProperty(o, p, v) {
+  var desc = {value: v, enumerable: true, writable: true, configurable: true};
+  return %reflect_define_property(o, p, desc);
+}
+
+
 function InternalizeJSONProperty(holder, name, reviver) {
   var val = holder[name];
-  if (IS_OBJECT(val) && val !== null) {
-    if (IS_ARRAY(val)) {
-      var length = val.length;
+  if (IS_SPEC_OBJECT(val)) {
+    if (%is_arraylike(val)) {
+      var length = TO_LENGTH(val.length);
       for (var i = 0; i < length; i++) {
         var newElement =
             InternalizeJSONProperty(val, %_NumberToString(i), reviver);
         if (IS_UNDEFINED(newElement)) {
-          delete val[i];
+          %reflect_delete_property(val, i);
         } else {
-          val[i] = newElement;
+          CreateDataProperty(val, i, newElement);
         }
       }
     } else {
       for (var p of ObjectKeys(val)) {
         var newElement = InternalizeJSONProperty(val, p, reviver);
         if (IS_UNDEFINED(newElement)) {
-          delete val[p];
+          %reflect_delete_property(val, p);
         } else {
-          val[p] = newElement;
+          CreateDataProperty(val, p, newElement);
         }
       }
     }
@@ -74,7 +81,7 @@ function SerializeArray(value, replacer, stack, indent, gap) {
   var stepback = indent;
   indent += gap;
   var partial = new InternalArray();
-  var len = value.length;
+  var len = TO_LENGTH(value.length);
   for (var i = 0; i < len; i++) {
     var strP = JSONSerialize(%_NumberToString(i), value, replacer, stack,
                              indent, gap);
@@ -106,27 +113,23 @@ function SerializeObject(value, replacer, stack, indent, gap) {
   if (IS_ARRAY(replacer)) {
     var length = replacer.length;
     for (var i = 0; i < length; i++) {
-      if (HAS_OWN_PROPERTY(replacer, i)) {
-        var p = replacer[i];
-        var strP = JSONSerialize(p, value, replacer, stack, indent, gap);
-        if (!IS_UNDEFINED(strP)) {
-          var member = %QuoteJSONString(p) + ":";
-          if (gap != "") member += " ";
-          member += strP;
-          partial.push(member);
-        }
+      var p = replacer[i];
+      var strP = JSONSerialize(p, value, replacer, stack, indent, gap);
+      if (!IS_UNDEFINED(strP)) {
+        var member = %QuoteJSONString(p) + ":";
+        if (gap != "") member += " ";
+        member += strP;
+        partial.push(member);
       }
     }
   } else {
-    for (var p in value) {
-      if (HAS_OWN_PROPERTY(value, p)) {
-        var strP = JSONSerialize(p, value, replacer, stack, indent, gap);
-        if (!IS_UNDEFINED(strP)) {
-          var member = %QuoteJSONString(p) + ":";
-          if (gap != "") member += " ";
-          member += strP;
-          partial.push(member);
-        }
+    for (var p of ObjectKeys(value)) {
+      var strP = JSONSerialize(p, value, replacer, stack, indent, gap);
+      if (!IS_UNDEFINED(strP)) {
+        var member = %QuoteJSONString(p) + ":";
+        if (gap != "") member += " ";
+        member += strP;
+        partial.push(member);
       }
     }
   }
@@ -166,7 +169,7 @@ function JSONSerialize(key, holder, replacer, stack, indent, gap) {
     return "null";
   } else if (IS_SPEC_OBJECT(value) && !IS_CALLABLE(value)) {
     // Non-callable object. If it's a primitive wrapper, it must be unwrapped.
-    if (IS_ARRAY(value)) {
+    if (%is_arraylike(value)) {
       return SerializeArray(value, replacer, stack, indent, gap);
     } else if (IS_NUMBER_WRAPPER(value)) {
       value = TO_NUMBER(value);
@@ -185,14 +188,13 @@ function JSONSerialize(key, holder, replacer, stack, indent, gap) {
 
 
 function JSONStringify(value, replacer, space) {
-  if (%_ArgumentsLength() == 1) {
+  if (%_ArgumentsLength() == 1 && !%_IsJSProxy(value)) {
     return %BasicJSONStringify(value);
   }
-  if (IS_ARRAY(replacer)) {
-    // Deduplicate replacer array items.
+  if (!IS_CALLABLE(replacer) && %is_arraylike(replacer)) {
     var property_list = new InternalArray();
-    var seen_properties = { __proto__: null };
-    var length = replacer.length;
+    var seen_properties = new GlobalSet();
+    var length = TO_LENGTH(replacer.length);
     for (var i = 0; i < length; i++) {
       var v = replacer[i];
       var item;
@@ -205,9 +207,9 @@ function JSONStringify(value, replacer, space) {
       } else {
         continue;
       }
-      if (!seen_properties[item]) {
+      if (!seen_properties.has(item)) {
         property_list.push(item);
-        seen_properties[item] = true;
+        seen_properties.add(item);
       }
     }
     replacer = property_list;

test/mjsunit/json.js

@@ -489,3 +489,32 @@ assertTrue(Object.prototype.isPrototypeOf(o2));
 
 var json = '{"stuff before slash\\\\stuff after slash":"whatever"}';
 TestStringify(json, JSON.parse(json));
+
+
+// https://bugs.chromium.org/p/v8/issues/detail?id=3139
+
+reviver = function(p, v) {
+  if (p == "a") {
+    this.b = { get x() {return null}, set x(_){throw 666} }
+  }
+  return v;
+}
+assertEquals({a: 0, b: {x: null}}, JSON.parse('{"a":0,"b":1}', reviver));
+
+
+// Make sure a failed [[Delete]] doesn't throw
+
+reviver = function(p, v) {
+  Object.freeze(this);
+  return p === "" ? v : undefined;
+}
+assertEquals({a: 0, b: 1}, JSON.parse('{"a":0,"b":1}', reviver));
+
+
+// Make sure a failed [[DefineProperty]] doesn't throw
+
+reviver = function(p, v) {
+  Object.freeze(this);
+  return p === "" ? v : 42;
+}
+assertEquals({a: 0, b: 1}, JSON.parse('{"a":0,"b":1}', reviver));

test/mjsunit/mjsunit.status

@@ -88,7 +88,6 @@
   # Proxy tests rely on non ES6 version of Proxies
   # TODO(neis,cbruni): figure out which Proxy tests can be reused
   'harmony/proxies-example-membrane': [SKIP],
-  'harmony/proxies-json': [SKIP],
   'harmony/proxies-symbols': [SKIP],
   'harmony/proxies-with-unscopables': [SKIP],
   'strong/load-proxy': [SKIP],
@@ -800,6 +799,8 @@
   'global-vars-with': [SKIP],
   'instanceof-2': [SKIP],
   'json': [SKIP],
+  'json-replacer-number-wrapper-tostring': [SKIP],
+  'json-replacer-order': [SKIP],
   'math-floor-of-div-minus-zero': [SKIP],
   'math-min-max': [SKIP],
   'messages': [SKIP],
@@ -854,6 +855,7 @@
   'regress/regress-2163': [SKIP],
   'regress/regress-2318': [SKIP],
   'regress/regress-2339': [SKIP],
+  'regress/regress-2374': [SKIP],
   'regress/regress-2444': [SKIP],
   'regress/regress-244': [SKIP],
   'regress/regress-2593': [SKIP],
@@ -866,6 +868,7 @@
   'regress/regress-2825': [SKIP],
   'regress/regress-286': [SKIP],
   'regress/regress-298269': [SKIP],
+  'regress/regress-3135': [SKIP],
   'regress/regress-3176': [SKIP],
   'regress/regress-318420': [SKIP],
   'regress/regress-320532': [SKIP],
@@ -926,6 +929,7 @@
   'regress/regress-69': [SKIP],
   'regress/regress-70066': [SKIP],
   'regress/regress-747': [SKIP],
+  'regress/regress-753': [SKIP],
   'regress/regress-806473': [SKIP],
   'regress/regress-842017': [SKIP],
   'regress/regress-84234': [SKIP],
@@ -953,6 +957,7 @@
   'regress/regress-crbug-245480': [SKIP],
   'regress/regress-crbug-349079': [SKIP],
   'regress/regress-crbug-350864': [SKIP],
+  'regress/regress-crbug-351262': [SKIP],
   'regress/regress-crbug-352058': [SKIP],
   'regress/regress-353551': [SKIP],
   'regress/regress-crbug-357137': [SKIP],
@@ -997,6 +1002,7 @@
   'regress/regress-handle-illegal-redeclaration': [SKIP],
   'regress/regress-inline-class-constructor': [SKIP],
   'regress/regress-inlining-function-literal-context': [SKIP],
+  'regress/regress-latin-1': [SKIP],
   'regress/regress-lazy-deopt-reloc': [SKIP],
   'regress/regress-map-invalidation-2': [SKIP],
   'regress/regress-opt-after-debug-deopt': [SKIP],

test/test262/test262.status

@@ -566,6 +566,7 @@
   'built-ins/Array/prototype/toString/S15.4.4.2_A1_T4': [SKIP],
   'built-ins/Date/15.9.1.15-1': [SKIP],
   'built-ins/Date/prototype/toISOString/15.9.5.43-0-13': [SKIP],
+  'built-ins/JSON/stringify/*': [SKIP],
   'built-ins/Object/defineProperty/15.2.3.6-4-625gs': [SKIP],
   'built-ins/Object/prototype/hasOwnProperty/S15.2.4.5_A12': [SKIP],
   'built-ins/Object/prototype/isPrototypeOf/S15.2.4.6_A12': [SKIP],