Improve internal stringifcation for custom Error objects.

If an developer attempts to "subclass" Error by running
`MyError.prototype = new Error();`, then the internal v8::Message object
that's produced and handed off to `window.onerror` handlers is poorly
stringified as "[object Object]".

This patch adjusts the stringification process for these objects to
include not only native Error objects, but also objects that have Error
in their prototype chain, and haven't overwritten Error.toString with
some custom variant.

BUG=2822
R=mstarzinger@chromium.org, yangguo@chromium.org

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

Patch from Mike West <mkwst@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16075 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/messages.js

@@ -228,16 +228,18 @@ function NoSideEffectToString(obj) {
       }
     }
   }
-  if (IsNativeErrorObject(obj)) return %_CallFunction(obj, ErrorToString);
+  if (CanBeSafelyTreatedAsAnErrorObject(obj)) {
+    return %_CallFunction(obj, ErrorToString);
+  }
   return %_CallFunction(obj, ObjectToString);
 }
 
-
-// To check if something is a native error we need to check the
-// concrete native error types. It is not sufficient to use instanceof
-// since it possible to create an object that has Error.prototype on
-// its prototype chain. This is the case for DOMException for example.
-function IsNativeErrorObject(obj) {
+// To determine whether we can safely stringify an object using ErrorToString
+// without the risk of side-effects, we need to check whether the object is
+// either an instance of a native error type (via '%_ClassOf'), or has $Error
+// in its prototype chain and hasn't overwritten 'toString' with something
+// strange and unusual.
+function CanBeSafelyTreatedAsAnErrorObject(obj) {
   switch (%_ClassOf(obj)) {
     case 'Error':
     case 'EvalError':
@@ -248,7 +250,9 @@ function IsNativeErrorObject(obj) {
     case 'URIError':
       return true;
   }
-  return false;
+
+  var objToString = %GetDataProperty(obj, "toString");
+  return obj instanceof $Error && objToString === ErrorToString;
 }
 
 
@@ -257,7 +261,7 @@ function IsNativeErrorObject(obj) {
 // the error to string method. This is to avoid leaking error
 // objects between script tags in a browser setting.
 function ToStringCheckErrorObject(obj) {
-  if (IsNativeErrorObject(obj)) {
+  if (CanBeSafelyTreatedAsAnErrorObject(obj)) {
     return %_CallFunction(obj, ErrorToString);
   } else {
     return ToString(obj);

test/cctest/test-api.cc

@@ -4388,7 +4388,7 @@ TEST(APIThrowMessageOverwrittenToString) {
 }
 
 
-static void check_custom_error_message(
+static void check_custom_error_tostring(
     v8::Handle<v8::Message> message,
     v8::Handle<v8::Value> data) {
   const char* uncaught_error = "Uncaught MyError toString";
@@ -4399,7 +4399,7 @@ static void check_custom_error_message(
 TEST(CustomErrorToString) {
   LocalContext context;
   v8::HandleScope scope(context->GetIsolate());
-  v8::V8::AddMessageListener(check_custom_error_message);
+  v8::V8::AddMessageListener(check_custom_error_tostring);
   CompileRun(
     "function MyError(name, message) {                   "
     "  this.name = name;                                 "
@@ -4410,6 +4410,58 @@ TEST(CustomErrorToString) {
     "  return 'MyError toString';                        "
     "};                                                  "
     "throw new MyError('my name', 'my message');         ");
+  v8::V8::RemoveMessageListeners(check_custom_error_tostring);
+}
+
+
+static void check_custom_error_message(
+    v8::Handle<v8::Message> message,
+    v8::Handle<v8::Value> data) {
+  const char* uncaught_error = "Uncaught MyError: my message";
+  printf("%s\n", *v8::String::Utf8Value(message->Get()));
+  CHECK(message->Get()->Equals(v8_str(uncaught_error)));
+}
+
+
+TEST(CustomErrorMessage) {
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+  v8::V8::AddMessageListener(check_custom_error_message);
+
+  // Handlebars.
+  CompileRun(
+    "function MyError(msg) {                             "
+    "  this.name = 'MyError';                            "
+    "  this.message = msg;                               "
+    "}                                                   "
+    "MyError.prototype = new Error();                    "
+    "throw new MyError('my message');                    ");
+
+  // Closure.
+  CompileRun(
+    "function MyError(msg) {                             "
+    "  this.name = 'MyError';                            "
+    "  this.message = msg;                               "
+    "}                                                   "
+    "inherits = function(childCtor, parentCtor) {        "
+    "    function tempCtor() {};                         "
+    "    tempCtor.prototype = parentCtor.prototype;      "
+    "    childCtor.superClass_ = parentCtor.prototype;   "
+    "    childCtor.prototype = new tempCtor();           "
+    "    childCtor.prototype.constructor = childCtor;    "
+    "};                                                  "
+    "inherits(MyError, Error);                           "
+    "throw new MyError('my message');                    ");
+
+  // Object.create.
+  CompileRun(
+    "function MyError(msg) {                             "
+    "  this.name = 'MyError';                            "
+    "  this.message = msg;                               "
+    "}                                                   "
+    "MyError.prototype = Object.create(Error.prototype); "
+    "throw new MyError('my message');                    ");
+
   v8::V8::RemoveMessageListeners(check_custom_error_message);
 }