Use JsonMap even for JSON.decode with custom reviver.

R=floitsch@google.com
BUG=

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

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@37931 260f80e4-7a28-3924-810f-c04153c831b5

Author: kasperl@google.com

sdk/lib/_internal/lib/convert_patch.dart

@@ -52,8 +52,7 @@ _parseJson(String source, reviver(key, value)) {
  * in-place.
  */
 _convertJsonToDart(json, reviver(key, value)) {
-  var revive = reviver == null ? (key, value) => value : reviver;
-
+  assert(reviver != null);
   walk(e) {
     // JavaScript null, string, number, bool are in the correct representation.
     if (JS('bool', '# == null', e) || JS('bool', 'typeof # != "object"', e)) {
@@ -64,41 +63,36 @@ _convertJsonToDart(json, reviver(key, value)) {
     // TODO(sra): Replace this test with cheaper '#.constructor === Array' when
     // bug 621 below is fixed.
     if (JS('bool', 'Object.getPrototypeOf(#) === Array.prototype', e)) {
-      // Teach compiler the type is known by passing it through a JS-expression.
-      var list = JS('JSExtendableArray', '#', e);
       // In-place update of the elements since JS Array is a Dart List.
-      for (int i = 0; i < list.length; i++) {
+      for (int i = 0; i < JS('int', '#.length', e); i++) {
         // Use JS indexing to avoid range checks.  We know this is the only
         // reference to the list, but the compiler will likely never be able to
         // tell that this instance of the list cannot have its length changed by
         // the reviver even though it later will be passed to the reviver at the
         // outer level.
-        var item = JS('', '#[#]', list, i);
-        JS('', '#[#]=#', list, i, revive(i, walk(item)));
+        var item = JS('', '#[#]', e, i);
+        JS('', '#[#]=#', e, i, reviver(i, walk(item)));
       }
-      return list;
+      return e;
     }
 
-    // Otherwise it is a plain Object, so copy to a Map.
-    var keys = JS('JSExtendableArray', 'Object.keys(#)', e);
-    Map map = {};
+    // Otherwise it is a plain object, so copy to a JSON map, so we process
+    // and revive all entries recursively.
+    _JsonMap map = new _JsonMap(e);
+    var processed = map._processed;
+    List<String> keys = map._computeKeys();
     for (int i = 0; i < keys.length; i++) {
       String key = keys[i];
-      map[key] = revive(key, walk(JS('', '#[#]', e, key)));
-    }
-    // V8 has a bug with properties named "__proto__"
-    // https://code.google.com/p/v8/issues/detail?id=621
-    var proto = JS('', '#.__proto__', e);
-    // __proto__ can be undefined on IE9.
-    if (JS('bool',
-           'typeof # !== "undefined" && # !== Object.prototype',
-           proto, proto)) {
-      map['__proto__'] = revive('__proto__', walk(proto));
+      var revived = reviver(key, walk(JS('', '#[#]', e, key)));
+      JS('', '#[#]=#', processed, key, revived);
     }
+
+    // Update the JSON map structure so future access is cheaper.
+    map._original = processed;  // Don't keep two objects around.
     return map;
   }
 
-  return revive(null, walk(json));
+  return reviver(null, walk(json));
 }
 
 _convertJsonToDartLazy(object) {
@@ -179,8 +173,12 @@ class _JsonMap implements LinkedHashMap {
     if (_isUpgraded) {
       _upgradedMap[key] = value;
     } else if (containsKey(key)) {
-      _setProperty(_processed, key, value);
-      _setProperty(_original, key, null);  // Reclaim memory.
+      var processed = _processed;
+      _setProperty(processed, key, value);
+      var original = _original;
+      if (!identical(original, processed)) {
+        _setProperty(original, key, null);  // Reclaim memory.
+      }
     } else {
       _upgrade()[key] = value;
     }

tests/corelib/json_map_test.dart

@@ -8,7 +8,13 @@ import "package:expect/expect.dart";
 import 'dart:convert' show JSON;
 import 'dart:collection' show LinkedHashMap, HashMap;
 
-Map jsonify(Map map) => JSON.decode(JSON.encode(map));
+bool useReviver = false;
+Map jsonify(Map map) {
+  String encoded = JSON.encode(map);
+  return useReviver
+      ? JSON.decode(encoded, reviver: (key, value) => value)
+      : JSON.decode(encoded);
+}
 
 List listEach(Map map) {
   var result = [];
@@ -20,6 +26,12 @@ List listEach(Map map) {
 }
 
 void main() {
+  test(false);
+  test(true);
+}
+
+void test(bool revive) {
+  useReviver = revive;
   testEmpty(jsonify({}));
   testAtoB(jsonify({'a': 'b'}));
 
@@ -45,6 +57,7 @@ void main() {
   testClear();
 
   testListEntry();
+  testMutation();
 }
 
 void testEmpty(Map map) {
@@ -322,3 +335,12 @@ void testListEntry() {
   Expect.equals(8, list[1]);
   Expect.equals(9, list[2]['b']);
 }
+
+void testMutation() {
+  Map map = jsonify({'a': 0});
+  Expect.listEquals(['a', 0], listEach(map));
+  map['a'] = 1;
+  Expect.listEquals(['a', 1], listEach(map));
+  map['a']++;
+  Expect.listEquals(['a', 2], listEach(map));
+}

tests/corelib/map_test.dart

@@ -7,7 +7,10 @@ import "package:expect/expect.dart";
 import 'dart:collection';
 import 'dart:convert' show JSON;
 
-Map newJsonMap() => JSON.decode('{}');
+Map newJsonMap()
+    => JSON.decode('{}');
+Map newJsonMapCustomReviver()
+    => JSON.decode('{}', reviver: (key, value) => value);
 
 void main() {
   test(new HashMap());
@@ -19,6 +22,7 @@ void main() {
   test(new MapBaseMap());
   test(new MapMixinMap());
   test(newJsonMap());
+  test(newJsonMapCustomReviver());
   testLinkedHashMap();
   testMapLiteral();
   testNullValue();
@@ -35,6 +39,7 @@ void main() {
   testWeirdStringKeys(new MapBaseMap<String, String>());
   testWeirdStringKeys(new MapMixinMap<String, String>());
   testWeirdStringKeys(newJsonMap());
+  testWeirdStringKeys(newJsonMapCustomReviver());
 
   testNumericKeys(new Map());
   testNumericKeys(new Map<num, String>());
@@ -49,6 +54,7 @@ void main() {
   testNumericKeys(new MapBaseMap<num, String>());
   testNumericKeys(new MapMixinMap<num, String>());
   testNumericKeys(newJsonMap());
+  testNumericKeys(newJsonMapCustomReviver());
 
   testNaNKeys(new Map());
   testNaNKeys(new Map<num, String>());
@@ -59,6 +65,7 @@ void main() {
   testNaNKeys(new MapBaseMap<num, String>());
   testNaNKeys(new MapMixinMap<num, String>());
   testNaNKeys(newJsonMap());
+  testNaNKeys(newJsonMapCustomReviver());
   // Identity maps fail the NaN-keys tests because the test assumes that
   // NaN is not equal to NaN.
 
@@ -87,6 +94,7 @@ void main() {
   testIterationOrder(new LinkedHashMap());
   testIterationOrder(new LinkedHashMap.identity());
   testIterationOrder(newJsonMap());
+  testIterationOrder(newJsonMapCustomReviver());
 
   testOtherKeys(new SplayTreeMap<int, int>());
   testOtherKeys(new SplayTreeMap<int, int>((int a, int b) => a - b,
@@ -110,6 +118,7 @@ void main() {
   testOtherKeys(new MapBaseMap<int, int>());
   testOtherKeys(new MapMixinMap<int, int>());
   testOtherKeys(newJsonMap());
+  testOtherKeys(newJsonMapCustomReviver());
 
   testUnmodifiableMap(const {1 : 37});
   testUnmodifiableMap(new UnmodifiableMapView({1 : 37}));