Unify logic of stack trace generation for extension errors

extensions/renderer/resources/uncaught_exception_handler.js

Index: extensions/renderer/resources/uncaught_exception_handler.js
diff --git a/extensions/renderer/resources/uncaught_exception_handler.js b/extensions/renderer/resources/uncaught_exception_handler.js
index 7e0725f03499464096c8a228a9f079f41308d743..687fc7fa14ae5b0fc330a4c7a5a48a082fad8f63 100644
--- a/extensions/renderer/resources/uncaught_exception_handler.js
+++ b/extensions/renderer/resources/uncaught_exception_handler.js
@@ -14,11 +14,13 @@ var handler = function(message, e) {
  * @param {string} message - The prefix of the error message.
  * @param {Error|*} e - The thrown error object. This object is potentially
  *   unsafe, because it could be generated by an extension.
- * @param {string=} stackTrace - The stack trace to be appended to the error
- *   message. This stack trace must not include stack frames of |e.stack|.
+ * @param {string=} priorStackTrace - The stack trace to be appended to the
+ *   error message. This stack trace must not include stack frames of |e.stack|,
+ *   because both stack traces are concatenated. Overlapping stack traces will
+ *   confuse extension developers.
  * @return {string} The formatted error message.
  */
-function formatErrorMessage(message, e, stackTrace) {
+function formatErrorMessage(message, e, priorStackTrace) {

[not at google - send to devlin, 2014/08/19 16:45:56]

The diff on this file is super confusing, seems like it was a diff against the
previous patch? Reitveld being crazy? Me being crazy?

[robwu, 2014/08/19 17:35:57]

This diff looks fine to me.
I renamed the printStack variable as requested and updated the comments
accordingly;
Besides that, I made use of the new $Error and $String.indexOf objects.

Perhaps you clicked on "View" instead of "Delta from patch set 2"
(or vice versa, depending on your definition of super confusing ;) )

[not at google - send to devlin, 2014/08/19 17:54:38]

I clicked on View which is supposed to show the diff against the original, not
the last patchset. But it's showing a diff against the last patchset.

   // Append ": [error message]"
   if (e) {
     try {
@@ -41,7 +43,7 @@ function formatErrorMessage(message, e, stackTrace) {
   } catch (e) {}
 
   // If a stack is not provided, capture a stack trace.
-  if (!stackTrace && !stack)
+  if (!priorStackTrace && !stack)
     stack = getStackTrace();
 
   stack = filterExtensionStackTrace(stack);
@@ -49,8 +51,8 @@ function formatErrorMessage(message, e, stackTrace) {
     message += '\n' + stack;
 
   // If an asynchronouse stack trace was set, append it.
-  if (stackTrace)
-    message += '\n' + stackTrace;
+  if (priorStackTrace)
+    message += '\n' + priorStackTrace;
 
   return message;
 }
@@ -62,17 +64,14 @@ function filterExtensionStackTrace(stack) {
   // extension, to not confuse extension developers with internal details.
   stack = $String.split(stack, '\n');
   stack = $Array.filter(stack, function(line) {
-    return line.indexOf('chrome-extension://') >= 0;
+    return $String.indexOf(line, 'chrome-extension://') >= 0;
   });
   return $Array.join(stack, '\n');
 }
 
-// Get a local reference to the captureStackTrace method to prevent extension
-// code from interfering with our method.
-var captureStackTrace = Error.captureStackTrace;
 function getStackTrace() {
   var e = {};
-  captureStackTrace(e, getStackTrace);
+  $Error.captureStackTrace(e, getStackTrace);
   return e.stack;
 }
 
@@ -85,11 +84,11 @@ function getExtensionStackTrace() {
  *
  * @param {string} message - Error message prefix.
  * @param {Error|*} e - Thrown object.
- * @param {string=} stackTrace - Error message suffix.
+ * @param {string=} priorStackTrace - Error message suffix.
  * @see formatErrorMessage
  */
-exports.handle = function(message, e, stackTrace) {
-  message = formatErrorMessage(message, e, stackTrace);
+exports.handle = function(message, e, priorStackTrace) {
+  message = formatErrorMessage(message, e, priorStackTrace);
   handler(message, e);
 };