Better stack trace support

pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart

Index: pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
diff --git a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
index 8e8a2cd130b78152f2ee78e6406ea2a16a3590eb..f764b29e873dd7f18d45d063cb61b6296e944e74 100644
--- a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
+++ b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
@@ -617,37 +617,70 @@ assert_(condition, [message]) => JS(
   if (!$condition) $throwAssertionError(message);
 })()''');
 
-var _stack = null;
-@JSExportName('throw')
-throw_(obj) => JS(
-    '',
-    '''(() => {
-    $_stack = new Error();
-    throw $obj;
-})()''');
+// Store a JS error for an exception.  For non-primitives, we store as an expando.

[Jennifer Messerly, 2017/05/05 23:59:15]

should be triple slash doc comment?

also long line

[vsm, 2017/05/08 17:07:44]

Done.

+// For primitive, we use a side cache.  To limit memory leakage, we only keep
+// the last [_maxTraceCache] entries.
+final _error = JS('', 'Symbol("_error")');
+Map _primitiveErrorCache = null;

[Jennifer Messerly, 2017/05/05 23:59:15]

`= null` should not be required.

also since this is top-level, you can initialize it here, you'll get laziness
for free from Dart.

[vsm, 2017/05/08 17:07:44]

Removed the null.  We're generating eagerly though if I initialize here &
failing at runtime.  

+var _maxErrorCache = 10;

[Jennifer Messerly, 2017/05/05 23:59:16]

this is a const right? (or at least a final)

[vsm, 2017/05/08 17:07:44]

Done.

+
+_isJsError(exception) {

[Jennifer Messerly, 2017/05/05 23:59:15]

bool return type?

[vsm, 2017/05/08 17:07:44]

Done.

+  return JS('bool', '#.Error != null && # instanceof #.Error', global_,
+      exception, global_);
+}
 
-getError(exception) => JS(
-    '',
-    '''(() => {
-  var stack = $_stack;
-  return stack !== null ? stack : $exception;
-})()''');
+// Record/return the JS error for an exception.  If an error was already
+// recorded, prefer that to [newError].
+recordJsError(exception, newError) {
+  if (_isJsError(exception)) return exception;
+
+  var useExpando =
+      exception != null && JS('bool', 'typeof # == "object"', exception);
+  var error;
+  if (useExpando) {
+    error = JS('', '#[#]', exception, _error);
+  } else {
+    if (_primitiveErrorCache == null) _primitiveErrorCache = {};
+    error = _primitiveErrorCache[exception];
+  }
+  if (error != null) return error;
+  if (newError != null) {
+    error = newError;
+  } else {
+    // We should only hit this path when a non-Error was thrown from JS.  In
+    // case, there is no stack trace on the exception, so we create one:
+    error = JS('', 'new Error()');
+  }
+  if (useExpando) {
+    JS('', '#[#] = #', exception, _error, error);
+  } else {
+    _primitiveErrorCache[exception] = error;
+    if (_primitiveErrorCache.length > _maxErrorCache) {
+      _primitiveErrorCache.remove(_primitiveErrorCache.keys.first);
+    }
+  }
+  return error;
+}
+
+@JSExportName('throw')
+throw_(obj) {

[Jennifer Messerly, 2017/05/05 23:59:16]

yay thanks for fixing this to not use a big JS-block :)

+  // Note, we create the error here to avoid the extra frame.
+  // package:stack_trace and tests appear to assume this.  We could fix use
+  // cases instead, but we're already on the exceptional path here.
+  recordJsError(obj, JS('', 'new Error()'));
+  JS('', 'throw #', obj);
+}
 
 // This is a utility function: it is only intended to be called from dev
 // tools.
 stackPrint(exception) => JS(

[Jennifer Messerly, 2017/05/05 23:59:15]

this isn't new code, but it's another place I'd try and refactor the "JS" call

stackPrint(exception) {
  var error = recordJsError(exception);
  JS('', 'console.log(#.stack ? #.stack : 'No stack trace for: ' + #)', error,
error, error);
}

[vsm, 2017/05/08 17:07:44]

Done.

     '',
     '''(() => {
-  var error = $getError($exception);
+  var error = $recordJsError($exception);
   console.log(error.stack ? error.stack : 'No stack trace for: ' + error);
 })()''');
 
-stackTrace(exception) => JS(
-    '',
-    '''(() => {
-  var error = $getError($exception);
-  return $getTraceFromException(error);
-})()''');
+stackTrace(exception) => getTraceFromException(exception);

[Jennifer Messerly, 2017/05/05 23:59:15]

hmmm. I see why you had to do this (you need to create _StackTrace from
js_helepr) ... but it looks odd as the reader.

Maybe worth a comment?

[vsm, 2017/05/08 17:07:44]

Done.

 
 ///
 /// Implements a sequence of .? operations.