Fix stack trace accessor behavior.

src/messages.js

Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index fd94cc0ede9ed26c49f96e1171c9273d2b98545a..9f79539422348c1ae2d50af407d8fe2c178fd2db 100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -1080,7 +1080,8 @@ function GetStackFrames(raw_stack) {
 var formatting_custom_stack_trace = false;
 
 
-function FormatStackTrace(obj, error_string, frames) {
+function FormatStackTrace(obj, raw_stack) {
+  var frames = GetStackFrames(raw_stack);
   if (IS_FUNCTION($Error.prepareStackTrace) && !formatting_custom_stack_trace) {
     var array = [];
     %MoveArrayContents(frames, array);
@@ -1097,7 +1098,7 @@ function FormatStackTrace(obj, error_string, frames) {
   }
 
   var lines = new InternalArray();
-  lines.push(error_string);
+  lines.push(FormatErrorString(obj));
   for (var i = 0; i < frames.length; i++) {
     var frame = frames[i];
     var line;
@@ -1132,45 +1133,44 @@ function GetTypeName(receiver, requireConstructor) {
 }
 
 
-function captureStackTrace(obj, cons_opt) {
-  var stackTraceLimit = $Error.stackTraceLimit;
-  if (!stackTraceLimit || !IS_NUMBER(stackTraceLimit)) return;
-  if (stackTraceLimit < 0 || stackTraceLimit > 10000) {
-    stackTraceLimit = 10000;
-  }
-  var stack = %CollectStackTrace(obj,
-                                 cons_opt ? cons_opt : captureStackTrace,
-                                 stackTraceLimit);
-
-  var error_string = FormatErrorString(obj);
-
-  // Set the 'stack' property on the receiver.  If the receiver is the same as
-  // holder of this setter, the accessor pair is turned into a data property.
-  var setter = function(v) {
-    // Set data property on the receiver (not necessarily holder).
-    %DefineOrRedefineDataProperty(this, 'stack', v, NONE);
-    if (this === obj) {
-      // Release context values if holder is the same as the receiver.
-      stack = error_string = UNDEFINED;
+var stack_trace_symbol;  // Set during bootstrapping.
+var formatted_stack_trace_symbol = NEW_PRIVATE("formatted stack trace");
+
+
+// Format the stack trace if not yet done, and return it.
+// Cache the formatted stack trace on the holder.
+function StackTraceGetter() {
+  var formatted_stack_trace = GET_PRIVATE(this, formatted_stack_trace_symbol);
+  if (IS_UNDEFINED(formatted_stack_trace)) {
+    var stack_trace = GET_PRIVATE(this, stack_trace_symbol);
+    if (IS_UNDEFINED(stack_trace)) return UNDEFINED;
+    var holder = this;
+    while (!HAS_PRIVATE(holder, stack_trace_symbol)) {
+      holder = %GetPrototype(holder);

[Toon Verwaest, 2014/06/30 09:56:30]

This runs through the prototype chain twice, and the first is just an expensive
"early bailout". I'd rather add after holder = %GetPrototype(holder); if
(!holder) return UNDEFINED;

     }
-  };
+    formatted_stack_trace = FormatStackTrace(holder, stack_trace);
+    SET_PRIVATE(holder, stack_trace_symbol, UNDEFINED);
+    SET_PRIVATE(holder, formatted_stack_trace_symbol, formatted_stack_trace);
+  }
+  return formatted_stack_trace;
+};
 
-  // The holder of this getter ('obj') may not be the receiver ('this').
-  // When this getter is called the first time, we use the context values to
-  // format a stack trace string and turn this accessor pair into a data
-  // property (on the holder).
-  var getter = function() {
-    // Stack is still a raw array awaiting to be formatted.
-    var result = FormatStackTrace(obj, error_string, GetStackFrames(stack));
-    // Replace this accessor to return result directly.
-    %DefineOrRedefineAccessorProperty(
-        obj, 'stack', function() { return result }, setter, DONT_ENUM);
-    // Release context values.
-    stack = error_string = UNDEFINED;
-    return result;
-  };
 
-  %DefineOrRedefineAccessorProperty(obj, 'stack', getter, setter, DONT_ENUM);
+// If the receiver equals the holder, set the formatted stack trace that the
+// getter returns.
+function StackTraceSetter(v) {
+  if (HAS_PRIVATE(this, stack_trace_symbol)) {
+    SET_PRIVATE(this, stack_trace_symbol, UNDEFINED);
+    SET_PRIVATE(this, formatted_stack_trace_symbol, v);
+  }
+};
+
+
+function captureStackTrace(obj, cons_opt) {
+  // Define accessors first, as this may fail and throw.
+  ObjectDefineProperty(obj, 'stack', { get: StackTraceGetter,
+                                       set: StackTraceSetter});
+  %CollectStackTrace(obj, cons_opt ? cons_opt : captureStackTrace);
 }
 
 
@@ -1306,40 +1306,8 @@ InstallFunctions($Error.prototype, DONT_ENUM, ['toString', ErrorToString]);
 function SetUpStackOverflowBoilerplate() {
   var boilerplate = MakeRangeError('stack_overflow', []);
 
-  var error_string = boilerplate.name + ": " + boilerplate.message;
-
-  // Set the 'stack' property on the receiver.  If the receiver is the same as
-  // holder of this setter, the accessor pair is turned into a data property.
-  var setter = function(v) {
-    %DefineOrRedefineDataProperty(this, 'stack', v, NONE);
-    // Tentatively clear the hidden property. If the receiver is the same as
-    // holder, we release the raw stack trace this way.
-    %GetAndClearOverflowedStackTrace(this);
-  };
-
-  // The raw stack trace is stored as a hidden property on the holder of this
-  // getter, which may not be the same as the receiver.  Find the holder to
-  // retrieve the raw stack trace and then turn this accessor pair into a
-  // data property.
-  var getter = function() {
-    var holder = this;
-    while (!IS_ERROR(holder)) {
-      holder = %GetPrototype(holder);
-      if (IS_NULL(holder)) return MakeSyntaxError('illegal_access', []);
-    }
-    var stack = %GetAndClearOverflowedStackTrace(holder);
-    // We may not have captured any stack trace.
-    if (IS_UNDEFINED(stack)) return stack;
-
-    var result = FormatStackTrace(holder, error_string, GetStackFrames(stack));
-    // Replace this accessor to return result directly.
-    %DefineOrRedefineAccessorProperty(
-        holder, 'stack', function() { return result }, setter, DONT_ENUM);
-    return result;
-  };
-
   %DefineOrRedefineAccessorProperty(
-      boilerplate, 'stack', getter, setter, DONT_ENUM);
+      boilerplate, 'stack', StackTraceGetter, StackTraceSetter, DONT_ENUM);
 
   return boilerplate;
 }