[json] support property list argument in BasicJsonStringifier.

R=cbruni@chromium.org

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

src/api.cc

@@ -2774,12 +2774,13 @@ MaybeLocal<String> JSON::Stringify(Local<Context> context,
                                    Local<String> gap) {
   PREPARE_FOR_EXECUTION(context, JSON, Stringify, String);
   i::Handle<i::Object> object = Utils::OpenHandle(*json_object);
+  i::Handle<i::Object> replacer = isolate->factory()->undefined_value();
   i::Handle<i::String> gap_string = gap.IsEmpty()
                                         ? isolate->factory()->empty_string()
                                         : Utils::OpenHandle(*gap);
   i::Handle<i::Object> maybe;
   has_pending_exception = !i::BasicJsonStringifier(isolate)
-                               .Stringify(object, gap_string)
+                               .Stringify(object, replacer, gap_string)
                                .ToHandle(&maybe);
   RETURN_ON_FAILED_EXECUTION(String);
   Local<String> result;

src/js/json.js

@@ -121,19 +121,6 @@ function SerializeObject(value, replacer, stack, indent, gap) {
   var stepback = indent;
   indent += gap;
   var partial = new InternalArray();
-  if (IS_ARRAY(replacer)) {
-    var length = replacer.length;
-    for (var i = 0; i < length; 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);
-      }
-    }
-  } else {
   var keys = %object_keys(value);
   for (var i = 0; i < keys.length; i++) {
     var p = keys[i];
@@ -145,7 +132,6 @@ function SerializeObject(value, replacer, stack, indent, gap) {
       partial.push(member);
     }
   }
-  }
   var final;
   if (gap == "") {
     final = "{" + partial.join(",") + "}";
@@ -201,29 +187,9 @@ function JSONSerialize(key, holder, replacer, stack, indent, gap) {
 
 
 function JSONStringify(value, replacer, space) {
-  if (arguments.length === 1) return %BasicJSONStringify(value, "");
-  if (!IS_CALLABLE(replacer) && %is_arraylike(replacer)) {
-    var property_list = new InternalArray();
-    var seen_properties = new GlobalSet();
-    var length = TO_LENGTH(replacer.length);
-    for (var i = 0; i < length; i++) {
-      var v = replacer[i];
-      var item;
-      if (IS_STRING(v)) {
-        item = v;
-      } else if (IS_NUMBER(v)) {
-        item = %_NumberToString(v);
-      } else if (IS_STRING_WRAPPER(v) || IS_NUMBER_WRAPPER(v)) {
-        item = TO_STRING(v);
-      } else {
-        continue;
-      }
-      if (!seen_properties.has(item)) {
-        property_list.push(item);
-        seen_properties.add(item);
-      }
-    }
-    replacer = property_list;
+  if (arguments.length === 1) return %BasicJSONStringify(value, UNDEFINED, "");
+  if (!IS_CALLABLE(replacer)) {
+    return %BasicJSONStringify(value, replacer, space);
   }
   if (IS_OBJECT(space)) {
     // Unwrap 'space' if it is wrapped
@@ -233,9 +199,6 @@ function JSONStringify(value, replacer, space) {
       space = TO_STRING(space);
     }
   }
-  if (!IS_CALLABLE(replacer) && !property_list) {
-    return %BasicJSONStringify(value, space);
-  }
   var gap;
   if (IS_NUMBER(space)) {
     space = MaxSimple(0, MinSimple(TO_INTEGER(space), 10));

src/json-stringifier.cc

@@ -88,7 +88,9 @@ BasicJsonStringifier::BasicJsonStringifier(Isolate* isolate)
 }
 
 MaybeHandle<Object> BasicJsonStringifier::Stringify(Handle<Object> object,
+                                                    Handle<Object> replacer,
                                                     Handle<Object> gap) {
+  if (!InitializeReplacer(replacer)) return MaybeHandle<Object>();
   if (!gap->IsUndefined() && !InitializeGap(gap)) return MaybeHandle<Object>();
   Result result = SerializeObject(object);
   if (result == UNCHANGED) return factory()->undefined_value();
@@ -97,15 +99,64 @@ MaybeHandle<Object> BasicJsonStringifier::Stringify(Handle<Object> object,
   return MaybeHandle<Object>();
 }
 
+bool IsInList(Handle<String> key, List<Handle<String> >* list) {
+  // TODO(yangguo): This is O(n^2) for n properties in the list. Deal with this
+  // if this becomes an issue.
+  for (const Handle<String>& existing : *list) {
+    if (String::Equals(existing, key)) return true;
+  }
+  return false;
+}
+
+bool BasicJsonStringifier::InitializeReplacer(Handle<Object> replacer) {
+  DCHECK(property_list_.is_null());
+  Maybe<bool> is_array = Object::IsArray(replacer);
+  if (is_array.IsNothing()) return false;
+  if (is_array.FromJust()) {
+    HandleScope handle_scope(isolate_);
+    List<Handle<String> > list;
+    Handle<Object> length_obj;
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+        isolate_, length_obj,
+        Object::GetLengthFromArrayLike(isolate_, replacer), false);
+    uint32_t length;
+    if (!length_obj->ToUint32(&length)) length = kMaxUInt32;
+    for (uint32_t i = 0; i < length; i++) {
+      Handle<Object> element;
+      Handle<String> key;
+      ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+          isolate_, element, Object::GetElement(isolate_, replacer, i), false);
+      if (element->IsNumber() || element->IsString()) {
+        ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+            isolate_, key, Object::ToString(isolate_, element), false);
+      } else if (element->IsJSValue()) {
+        Handle<Object> value(Handle<JSValue>::cast(element)->value(), isolate_);
+        if (value->IsNumber() || value->IsString()) {
+          ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+              isolate_, key, Object::ToString(isolate_, element), false);
+        }
+      }
+      if (key.is_null()) continue;
+      if (!IsInList(key, &list)) list.Add(key);
+    }
+    property_list_ = factory()->NewUninitializedFixedArray(list.length());
+    for (int i = 0; i < list.length(); i++) {
+      property_list_->set(i, *list[i]);
+    }
+    property_list_ = handle_scope.CloseAndEscape(property_list_);
+  }
+  return true;
+}
+
 bool BasicJsonStringifier::InitializeGap(Handle<Object> gap) {
   DCHECK_NULL(gap_);
   HandleScope scope(isolate_);
-  if (gap->IsJSReceiver()) {
-    Handle<String> class_name(Handle<JSReceiver>::cast(gap)->class_name());
-    if (class_name.is_identical_to(factory()->String_string())) {
+  if (gap->IsJSValue()) {
+    Handle<Object> value(Handle<JSValue>::cast(gap)->value(), isolate_);
+    if (value->IsString()) {
       ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate_, gap,
                                        Object::ToString(isolate_, gap), false);
-    } else if (class_name.is_identical_to(factory()->number_string())) {
+    } else if (value->IsNumber()) {
       ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate_, gap, Object::ToNumber(gap),
                                        false);
     }
@@ -146,7 +197,8 @@ MaybeHandle<Object> BasicJsonStringifier::StringifyString(
 
   if (worst_case_length > 32 * KB) {  // Slow path if too large.
     BasicJsonStringifier stringifier(isolate);
-    return stringifier.Stringify(object, isolate->factory()->undefined_value());
+    Handle<Object> undefined = isolate->factory()->undefined_value();
+    return stringifier.Stringify(object, undefined, undefined);
   }
 
   object = String::Flatten(object);
@@ -437,7 +489,8 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSObject(
   Result stack_push = StackPush(object);
   if (stack_push != SUCCESS) return stack_push;
 
-  if (object->map()->instance_type() > LAST_CUSTOM_ELEMENTS_RECEIVER &&
+  if (property_list_.is_null() &&
+      object->map()->instance_type() > LAST_CUSTOM_ELEMENTS_RECEIVER &&
       object->HasFastProperties() &&
       Handle<JSObject>::cast(object)->elements()->length() == 0) {
     DCHECK(object->IsJSObject());
@@ -487,10 +540,12 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSObject(
 
 BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSReceiverSlow(
     Handle<JSReceiver> object) {
-  Handle<FixedArray> contents;
+  Handle<FixedArray> contents = property_list_;
+  if (contents.is_null()) {
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(
         isolate_, contents,
         JSReceiver::GetKeys(object, OWN_ONLY, ENUMERABLE_STRINGS), EXCEPTION);
+  }
 
   builder_.AppendCharacter('{');
   Indent();

src/json-stringifier.h

@@ -18,6 +18,7 @@ class BasicJsonStringifier BASE_EMBEDDED {
   ~BasicJsonStringifier() { DeleteArray(gap_); }
 
   MUST_USE_RESULT MaybeHandle<Object> Stringify(Handle<Object> object,
+                                                Handle<Object> replacer,
                                                 Handle<Object> gap);
 
   MUST_USE_RESULT static MaybeHandle<Object> StringifyString(
@@ -26,6 +27,7 @@ class BasicJsonStringifier BASE_EMBEDDED {
  private:
   enum Result { UNCHANGED, SUCCESS, EXCEPTION };
 
+  bool InitializeReplacer(Handle<Object> replacer);
   bool InitializeGap(Handle<Object> gap);
 
   MUST_USE_RESULT MaybeHandle<Object> ApplyToJsonFunction(
@@ -107,6 +109,7 @@ class BasicJsonStringifier BASE_EMBEDDED {
   IncrementalStringBuilder builder_;
   Handle<String> tojson_string_;
   Handle<JSArray> stack_;
+  Handle<FixedArray> property_list_;
   uc16* gap_;
   int indent_;
 

src/runtime/runtime-json.cc

@@ -24,11 +24,12 @@ RUNTIME_FUNCTION(Runtime_QuoteJSONString) {
 
 RUNTIME_FUNCTION(Runtime_BasicJSONStringify) {
   HandleScope scope(isolate);
-  DCHECK(args.length() == 2);
+  DCHECK(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
-  CONVERT_ARG_HANDLE_CHECKED(Object, gap, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Object, replacer, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Object, gap, 2);
   RETURN_RESULT_OR_FAILURE(
-      isolate, BasicJsonStringifier(isolate).Stringify(object, gap));
+      isolate, BasicJsonStringifier(isolate).Stringify(object, replacer, gap));
 }
 
 RUNTIME_FUNCTION(Runtime_ParseJson) {

src/runtime/runtime.h

@@ -328,7 +328,7 @@ namespace internal {
 
 #define FOR_EACH_INTRINSIC_JSON(F) \
   F(QuoteJSONString, 1, 1)         \
-  F(BasicJSONStringify, 2, 1)      \
+  F(BasicJSONStringify, 3, 1)      \
   F(ParseJson, 1, 1)
 
 #define FOR_EACH_INTRINSIC_LITERALS(F) \

test/mjsunit/json-replacer-order.js

@@ -20,7 +20,6 @@ var space = Object.defineProperty(new String, 'toString', {
 });
 
 JSON.stringify('', replacer, space);
-
 assertEquals(2, log.length);
 assertEquals('get 0', log[0]);
 assertEquals('toString', log[1]);