Fix debugger

tool/input_sdk/private/debugger.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library dart._debugger;
 
 import 'dart:_foreign_helper' show JS;
 import 'dart:_runtime' as dart;
 import 'dart:core';
+import 'dart:collection';
+import 'dart:html' as html;

[vsm, 2016/05/20 01:30:39]

Nothing in dart:core, etc depends on debugger, right?  Right now, we can break
the dom libs out - they're a separate strongly connected component.

[Jacob, 2016/05/20 17:02:36]

Nothing depends on the _debugger library. The only reason it is in the sdk is to
access internal dart:_ libraries.
It would be reasonable to keep it in its own component only included if a user
has the Dart Devtools Extension installed.

 
 /// Config object to pass to devtools to signal that an object should not be
 /// formatted by the Dart formatter. This is used to specify that an Object
 /// should just be displayed using the regular JavaScript view instead of a
 /// custom Dart view. For example, this is used to display the JavaScript view
 /// of a Dart Function as a child of the regular Function object.
 const skipDartConfig = const Object();
 final int maxIterableChildrenToDisplay = 50;
 
 var _devtoolsFormatter = new JsonMLFormatter(new DartFormatter());
 
 String _typeof(object) => JS('String', 'typeof #', object);
-bool _instanceof(object, clazz) => JS('bool', '# instanceof #', object, clazz);
 
 List<String> getOwnPropertyNames(object) => JS('List<String>',
     'dart.list(Object.getOwnPropertyNames(#), #)', object, String);
 
 List getOwnPropertySymbols(object) =>
     JS('List', 'Object.getOwnPropertySymbols(#)', object);
 
 // TODO(jacobr): move this to dart:js and fully implement.
 class JSNative {
   // Name may be a String or a Symbol.
   static getProperty(object, name) => JS('', '#[#]', object, name);
   // Name may be a String or a Symbol.
   static setProperty(object, name, value) =>
       JS('', '#[#]=#', object, name, value);
 }
 
-bool isRegularDartObject(object) {
-  if (_typeof(object) == 'function') return false;
-  return _instanceof(object, JS('Type', '#', Object));
-}
-
 String getObjectTypeName(object) {
   var reifiedType = dart.getReifiedType(object);
   if (reifiedType == null) {
     if (_typeof(object) == 'function') {
       return '[[Raw JavaScript Function]]';
     }
     return '<Error getting type name>';
   }
   return getTypeName(reifiedType);
 }
 
 String getTypeName(Type type) {
   var name = dart.typeName(type);
   // Hack to cleanup names for List<dynamic>
   // TODO(jacobr): it would be nice if there was a way we could distinguish
   // between a List<dynamic> created from Dart and an Array passed in from
   // JavaScript.
-  if (name == 'JSArray<dynamic>' ||
-      name == 'JSObject<Array>') return 'List<dynamic>';
+  if (name == 'JSArray<dynamic>' || name == 'JSObject<Array>')
+    return 'List<dynamic>';
   return name;
 }
 
 String safePreview(object) {
   try {
     var preview = _devtoolsFormatter._simpleFormatter.preview(object);
     if (preview != null) return preview;
     return object.toString();
   } catch (e) {
-    return '<Exception thrown>';
+    return '<Exception thrown> $e';
   }
 }
 
 String symbolName(symbol) {
   var name = symbol.toString();
   assert(name.startsWith('Symbol('));
   return name.substring('Symbol('.length, name.length - 1);
 }
 
 bool hasMethod(object, String name) {
   try {
     return dart.hasMethod(object, name);
   } catch (e) {
     return false;
   }
 }
 
 /// [JsonMLFormatter] consumes [NameValuePair] objects and
 class NameValuePair {
   NameValuePair({this.name, this.value, bool skipDart})
       : skipDart = skipDart == true;
 
+  // Define equality and hashCode so that NameValuePair can be used
+  // in a Set to dedupe entries with duplicate names.
+  operator ==(other) => other is NameValuePair && other.name == name;
+  int get hashCode => name.hashCode;
+
   final String name;
   final Object value;
   final bool skipDart;
 }
 
 class MapEntry {
   MapEntry({this.key, this.value});
 
   final String key;
   final Object value;
@@ skipping 52 matching lines @@
     JSNative.setProperty(_attributes, key, value);
   }
 
   createTextChild(String text) {
     _jsonML.add(text);
   }
 
   toJsonML() => _jsonML;
 }
 
+/// Whether an object is a native JavaScript type where we should display the
+/// JavaScript view of the object instead of the custom Dart specific render
+/// of properties.
+bool isNativeJavaScriptObject(object) {
+  var type = _typeof(object);
+  // Treat Node objects as a native JavaScript type as the regular DOM render
+  // in devtools is superior to the dart specific view.
+  return (type != 'object' && type != 'function') ||
+      object is dart.JSObject ||
+      object is html.Node;
+}
+
 /// Class implementing the Devtools Formatter API described by:
 /// https://docs.google.com/document/d/1FTascZXT9cxfetuPRT2eXPQKXui4nWFivUnS_335T3U
 /// Specifically, a formatter implements a header, hasBody, and body method.
 /// This class renders the simple structured format objects [_simpleFormatter]
 /// provides as JsonML.
 class JsonMLFormatter {
   // TODO(jacobr): define a SimpleFormatter base class that DartFormatter
   // implements if we decide to use this class elsewhere. We specify that the
   // type is DartFormatter here purely to get type checking benefits not because
   // this class is really intended to only support instances of type
   // DartFormatter.
   DartFormatter _simpleFormatter;
 
   JsonMLFormatter(this._simpleFormatter);
 
   header(object, config) {
-    if (identical(config, skipDartConfig)) return null;
+    if (identical(config, skipDartConfig) || isNativeJavaScriptObject(object)) {
+      return null;
+    }
 
     var c = _simpleFormatter.preview(object);
     if (c == null) return null;
 
     // Indicate this is a Dart Object by using a Dart background color.
     // This is stylistically a bit ugly but it eases distinguishing Dart and
     // JS objects.
     var element = new JsonMLElement('span')
       ..setStyle('background-color: #d9edf7')
       ..createTextChild(c);
@@ skipping 56 matching lines @@
       new MapFormatter(),
       new IterableFormatter(),
       new MapEntryFormatter(),
       new ClassMetadataFormatter(),
       new HeritageClauseFormatter(),
       new ObjectFormatter()
     ];
   }
 
   String preview(object) {
-    if (object == null) return 'null';
-    if (object is num) return object.toString();
-    if (object is String) return '"$object"';
+    try {
+      if (object == null ||

[vsm, 2016/05/20 01:30:39]

null.toString()?  perhaps return "null"?

[Jacob, 2016/05/20 17:02:36]

null.toString is defined as "null" in Dart so we don't have to special case it.
https://api.dartlang.org/dev/1.17.0-dev.1.0/dart-core/Null/toString.html

+          object is num ||
+          object is String ||
+          isNativeJavaScriptObject(object)) {
+        return object.toString();
+      }
 
-    for (var formatter in _formatters) {
-      if (formatter.accept(object)) return formatter.preview(object);
+      for (var formatter in _formatters) {
+        if (formatter.accept(object)) return formatter.preview(object);
+      }
+    } catch (e, trace) {
+      // Log formatter internal errors as unfortunately the devtools cannot
+      // be used to debug formatter errors.
+      html.window.console.error("Caught exception $e\n trace:\n$trace");
     }
 
     return null;
   }
 
   bool hasChildren(object) {
     if (object == null) return false;
-
-    for (var formatter in _formatters) {
-      if (formatter.accept(object)) return formatter.hasChildren(object);
+    try {
+      for (var formatter in _formatters) {
+        if (formatter.accept(object)) return formatter.hasChildren(object);
+      }
+    } catch (e, trace) {
+      // See comment for preview.
+      html.window.console
+          .error("[hasChildren] Caught exception $e\n trace:\n$trace");
     }
-
     return false;
   }
 
   List<NameValuePair> children(object) {
-    if (object != null) {
-      for (var formatter in _formatters) {
-        if (formatter.accept(object)) return formatter.children(object);
+    try {
+      if (object != null) {
+        for (var formatter in _formatters) {
+          if (formatter.accept(object)) return formatter.children(object);
+        }
       }
+    } catch (e, trace) {
+      // See comment for preview.
+      html.window.console.error("Caught exception $e\n trace:\n$trace");
     }
     return <NameValuePair>[];
   }
 }
 
 /// Default formatter for Dart Objects.
 class ObjectFormatter extends Formatter {
-  bool accept(object) => isRegularDartObject(object);
+  bool accept(object) => !isNativeJavaScriptObject(object);
 
   String preview(object) => getObjectTypeName(object);
 
   bool hasChildren(object) => true;
 
-  /// Helper to add members walking up the prototype chain being careful
-  /// to avoid properties that are Dart methods.
-  _addMembers(current, object, List<NameValuePair> properties) {
-    // TODO(jacobr): optionally distinguish properties and fields so that
-    // it is safe to expand untrusted objects without side effects.
-    var className = dart.getReifiedType(current).name;
-    for (var name in getOwnPropertyNames(current)) {
-      if (name == 'constructor' ||
-          name == '__proto__' ||
-          name == className) continue;
-      if (hasMethod(object, name)) {
-        continue;
+  List<NameValuePair> children(object) {
+    var properties = new LinkedHashSet<NameValuePair>();
+    // Set of property names used to avoid duplicates.
+    addMetadataChildren(object, properties);
+
+    /// Helper to add members walking up the prototype chain being careful
+    /// to avoid properties that are Dart methods.
+    var protoChain = <Object>[];
+    var current = object;
+    while (current != null &&
+        !isNativeJavaScriptObject(current) &&
+        JS("bool", "# !== Object.prototype", current)) {
+      protoChain.add(current);
+      current = JSNative.getProperty(current, '__proto__');
+    }
+
+    // We walk the prototype chain for symbol properties
+    // because they take priority and are accessed instead of
+    // Dart properties according to Dart calling conventions.

[vsm, 2016/05/20 01:30:39]

We have some of this metadata encoded on the type (e.g., dart.setSignature). 
Perhaps more robust to get it from there?

[Jacob, 2016/05/20 17:02:36]

Agreed that would be much better. Adding a TODO. Perhaps that can be an intern
getting started task? :)

+    for (current in protoChain) {
+      for (var symbol in getOwnPropertySymbols(current)) {
+        var dartName = symbolName(symbol);
+        if (hasMethod(object, dartName)) {
+          continue;
+        }
+        // TODO(jacobr): find a cleaner solution than checking for dartx
+        String dartXPrefix = 'dartx.';
+        if (dartName.startsWith(dartXPrefix)) {
+          dartName = dartName.substring(dartXPrefix.length);
+        } else if (!dartName.startsWith('_')) {
+          // Dart method extension names should either be from dartx or should
+          // start with an _
+          continue;
+        }
+        var value;
+        try {
+          value = JSNative.getProperty(object, symbol);
+        } catch (e) {
+          value = '<Exception thrown> $e';
+        }
+        properties.add(new NameValuePair(name: dartName, value: value));
       }
-      var value;
-      try {
-        value = JSNative.getProperty(object, name);
-      } catch (e) {
-        value = '<Exception thrown>';
+    }
+
+    for (current in protoChain) {
+      // TODO(jacobr): optionally distinguish properties and fields so that
+      // it is safe to expand untrusted objects without side effects.
+      var className = dart.getReifiedType(current).name;
+      for (var name in getOwnPropertyNames(current)) {
+        if (customNames.contains(name) || name == className) continue;

[vsm, 2016/05/20 01:30:39]

You're getting an SDK error on this.  :-)

[Jacob, 2016/05/20 17:02:36]

Wow. Fixed.

+        if (hasMethod(object, name)) {
+          continue;
+        }
+        var value;
+        try {
+          value = JSNative.getProperty(object, name);
+        } catch (e) {
+          value = '<Exception thrown> $e';
+        }
+        properties.add(new NameValuePair(name: name, value: value));
       }
-      properties.add(new NameValuePair(name: name, value: value));
     }
-    for (var symbol in getOwnPropertySymbols(current)) {
-      var dartName = symbolName(symbol);
-      if (hasMethod(object, dartName)) {
-        continue;
-      }
-      var value;
-      try {
-        value = JSNative.getProperty(object, symbol);
-      } catch (e) {
-        value = '<Exception thrown>';
-      }
-      properties.add(new NameValuePair(name: dartName, value: value));
-    }
-    var base = JSNative.getProperty(current, '__proto__');
-    if (base == null) return;
-    if (isRegularDartObject(base)) {
-      _addMembers(base, object, properties);
-    }
+
+    return properties.toList();
   }
 
-  List<NameValuePair> children(object) {
-    var properties = <NameValuePair>[];
-    addMetadataChildren(object, properties);
-    _addMembers(object, object, properties);
-    return properties;
-  }
-
-  addMetadataChildren(object, List<NameValuePair> ret) {
+  addMetadataChildren(object, Set<NameValuePair> ret) {
     ret.add(
         new NameValuePair(name: '[[class]]', value: new ClassMetadata(object)));
   }
 }
 
 /// Formatter for Dart Function objects.
 /// Dart functions happen to be regular JavaScript Function objects but
 /// we can distinguish them based on whether they have been tagged with
 /// runtime type information.
 class FunctionFormatter extends Formatter {
@@ skipping 24 matching lines @@
   String preview(object) {
     Map map = object;
     return '${getObjectTypeName(map)} length ${map.length}';
   }
 
   List<NameValuePair> children(object) {
     // TODO(jacobr): be lazier about enumerating contents of Maps that are not
     // the build in LinkedHashMap class.
     // TODO(jacobr): handle large Maps better.
     Map map = object;
-    var keys = map.keys.toList();
-    var entries = <NameValuePair>[];
+    var entries = new LinkedHashSet<NameValuePair>();
     map.forEach((key, value) {
       var entryWrapper = new MapEntry(key: key, value: value);
       entries.add(new NameValuePair(
           name: entries.length.toString(), value: entryWrapper));
     });
     addMetadataChildren(object, entries);
-    return entries;
+    return entries.toList();
   }
 }
 
 /// Formatter for Dart Iterable objects including List and Set.
 class IterableFormatter extends ObjectFormatter {
   bool accept(object) => object is Iterable;
 
   String preview(object) {
     Iterable iterable = object;
     try {
       var length = iterable.length;
       return '${getObjectTypeName(iterable)} length $length';
     } catch (_) {
       return '${getObjectTypeName(iterable)}';
     }
   }
 
   bool hasChildren(object) => true;
 
   List<NameValuePair> children(object) {
     // TODO(jacobr): be lazier about enumerating contents of Iterables that
     // are not the built in Set or List types.
     // TODO(jacobr): handle large Iterables better.
     // TODO(jacobr): consider only using numeric indices
     Iterable iterable = object;
-    var ret = <NameValuePair>[];
+    var ret = new LinkedHashSet<NameValuePair>();
     var i = 0;
     for (var entry in iterable) {
       if (i > maxIterableChildrenToDisplay) {
         ret.add(new NameValuePair(
             name: 'Warning', value: 'Truncated Iterable display'));
         // TODO(jacobr): provide an expandable entry to show more entries.
         break;
       }
       ret.add(new NameValuePair(name: i.toString(), value: entry));
       i++;
     }
     // TODO(jacobr): provide a link to show regular class properties here.
     // required for subclasses of iterable, etc.
     addMetadataChildren(object, ret);
-    return ret;
+    return ret.toList();
   }
 }
 
 // This class does double duting displaying metadata for
 class ClassMetadataFormatter implements Formatter {
   accept(object) => object is ClassMetadata;
 
   _getType(object) {
     if (object is Type) return object;
     return dart.getReifiedType(object);
@@ skipping 12 matching lines @@
     // links to the superclass, mixins, implemented interfaces, and methods.
     var type = _getType(entry.object);
     var ret = <NameValuePair>[];
     var implements = dart.getImplements(type);
     if (implements != null) {
       ret.add(new NameValuePair(
           name: '[[Implements]]',
           value: new HeritageClause('implements', implements())));
     }
     var mixins = dart.getMixins(type);
-    if (mixins != null) {
+    if (mixins != null && mixins.isNotEmpty) {
       ret.add(new NameValuePair(
-          name: '[[Mixins]]', value: new HeritageClause('mixins', mixins())));
+          name: '[[Mixins]]', value: new HeritageClause('mixins', mixins)));
     }
     ret.add(new NameValuePair(
         name: '[[JavaScript View]]', value: entry.object, skipDart: true));
 
     // TODO(jacobr): provide a link to the base class or perhaps the entire
     // base class hierarchy as a flat list.
 
     if (entry.object is! Type) {
       ret.add(new NameValuePair(
           name: '[[JavaScript Constructor]]',
@@ skipping 44 matching lines @@
     return ret;
   }
 }
 
 /// This entry point is automatically invoked by the code generated by
 /// Dart Dev Compiler
 registerDevtoolsFormatter() {
   var formatters = [_devtoolsFormatter];
   JS('', 'dart.global.devtoolsFormatters = #', formatters);
 }