Unify logic of stack trace generation for extension errors

extensions/renderer/resources/uncaught_exception_handler.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Handles uncaught exceptions thrown by extensions. By default this is to
 // log an error message, but tests may override this behaviour.
 var handler = function(message, e) {
   console.error(message);
 };
 
 /**
  * Append the error description and stack trace to |message|.
  *
  * @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 {
       // Append the toString() representation of |e| to the message. For
       // instances of Error, it looks like "Error: <message>".
       message += ': ' + e;
     } catch (e) {
       // This could be triggered by
       // throw {toString: function() { throw 'Haha' } };
       message += ': (cannot get error message)';
     }
   }
 
   var stack;
   try {
     // If the stack was set, use it.
     // |e.stack| could be void in the following common example:
     // throw "Error message";
     stack = $String.self(e && e.stack);
   } catch (e) {}
 
   // If a stack is not provided, capture a stack trace.
-  if (!stackTrace && !stack)
+  if (!priorStackTrace && !stack)
     stack = getStackTrace();
 
   stack = filterExtensionStackTrace(stack);
   if (stack)
     message += '\n' + stack;
 
   // If an asynchronouse stack trace was set, append it.
-  if (stackTrace)
-    message += '\n' + stackTrace;
+  if (priorStackTrace)
+    message += '\n' + priorStackTrace;
 
   return message;
 }
 
 function filterExtensionStackTrace(stack) {
   if (!stack)
     return '';
   // Remove stack frames in the stack trace that weren't associated with the
   // 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;
 }
 
 function getExtensionStackTrace() {
   return filterExtensionStackTrace(getStackTrace());
 }
 
 /**
  * Formats the error message and invokes the error handler.
  *
  * @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);
 };
 
 // |newHandler| A function which matches |handler|.
 exports.setHandler = function(newHandler) {
   handler = newHandler;
 };
 
 exports.getStackTrace = getStackTrace;
 exports.getExtensionStackTrace = getExtensionStackTrace;