Bug 1448287 - Fix devtools.dump.emit when using new event emitter r?jdescottes draft
authorMike Ratcliffe <mratcliffe@mozilla.com>
Fri, 23 Mar 2018 15:34:22 +0000
changeset 779650 3d3f7c72926ccbeca661933e9041f3fb80eebdcd
parent 779502 83de58ddda2057f1cb949537f6b111e3b115ea3d
push id105833
push userbmo:mratcliffe@mozilla.com
push dateTue, 10 Apr 2018 11:42:45 +0000
reviewersjdescottes
bugs1448287
milestone61.0a1
Bug 1448287 - Fix devtools.dump.emit when using new event emitter r?jdescottes components.stack is not always available, throw new Error() to obtain a stack instead. MozReview-Commit-ID: BZq7zLKgA9H
devtools/client/shared/webpack/shims/platform-stack-stub.js
devtools/client/shared/webpack/shims/test/test_stack.js
devtools/shared/event-emitter.js
devtools/shared/old-event-emitter.js
devtools/shared/platform/stack.js
--- a/devtools/client/shared/webpack/shims/platform-stack-stub.js
+++ b/devtools/client/shared/webpack/shims/platform-stack-stub.js
@@ -13,37 +13,41 @@
 /**
  * Looks like Cu.callFunctionWithAsyncStack, but just calls the callee.
  */
 function callFunctionWithAsyncStack(callee, stack, id) {
   return callee();
 }
 
 /**
- * Return a description of the Nth caller, suitable for logging.
+ * Return the Nth path from the stack excluding substr.
  *
- * @param {Number} n the caller to describe
- * @return {String} a description of the nth caller.
+ * @param {Number}
+ *        n the Nth path from the stack to describe.
+ * @param {String} substr
+ *        A segment of the path that should be excluded.
  */
-function describeNthCaller(n) {
+function getNthPathExcluding(n, substr) {
   if (isWorker) {
     return "";
   }
 
   let stack = new Error().stack.split("\n");
-  // Add one here to skip this function.
+  stack = stack.filter(line => {
+    return line && !line.includes(substr);
+  });
   return stack[n + 1];
 }
 
 /**
  * Return a stack object that can be serialized and, when
  * deserialized, passed to callFunctionWithAsyncStack.
  */
 function getStack() {
   // There's no reason for this to do anything fancy, since it's only
   // used to pass back into callFunctionWithAsyncStack, which we can't
   // implement.
   return null;
 }
 
 exports.callFunctionWithAsyncStack = callFunctionWithAsyncStack;
-exports.describeNthCaller = describeNthCaller;
+exports.getNthPathExcluding = getNthPathExcluding;
 exports.getStack = getStack;
--- a/devtools/client/shared/webpack/shims/test/test_stack.js
+++ b/devtools/client/shared/webpack/shims/test/test_stack.js
@@ -6,21 +6,21 @@
 
 "use strict";
 
 const {require} = ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
 
 const {
   callFunctionWithAsyncStack,
   getStack,
-  describeNthCaller
+  getNthPathExcluding
 } = require("devtools/client/shared/webpack/shims/platform-stack-stub");
 
 function f3() {
-  return describeNthCaller(2);
+  return getNthPathExcluding(2);
 }
 
 function f2() {
   return f3();
 }
 
 function f1() {
   return f2();
--- a/devtools/shared/event-emitter.js
+++ b/devtools/shared/event-emitter.js
@@ -257,55 +257,120 @@ class EventEmitter {
 }
 
 module.exports = EventEmitter;
 
 const isEventHandler = (listener) =>
   listener && handler in listener && typeof listener[handler] === "function";
 
 const Services = require("Services");
-const { describeNthCaller } = require("devtools/shared/platform/stack");
+const { getNthPathExcluding } = require("devtools/shared/platform/stack");
 let loggingEnabled = false;
 
 if (!isWorker) {
   loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
   Services.prefs.addObserver("devtools.dump.emit", {
     observe: () => {
       loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
     }
   });
 }
 
 function serialize(target) {
-  let out = String(target);
+  const MAXLEN = 60;
+
+  // Undefined
+  if (typeof target === "undefined") {
+    return "undefined";
+  }
+
+  if (target === null) {
+    return "null";
+  }
 
-  if (target && target.nodeName) {
-    out += " (" + target.nodeName;
+  // Number / String
+  if (typeof target === "string" ||
+      typeof target === "number") {
+    return truncate(target, MAXLEN);
+  }
+
+  // HTML Node
+  if (target.nodeName) {
+    let out = target.nodeName;
+
     if (target.id) {
       out += "#" + target.id;
     }
     if (target.className) {
       out += "." + target.className;
     }
-    out += ")";
+
+    return out;
+  }
+
+  // Array
+  if (Array.isArray(target)) {
+    return truncate(target.toSource(), MAXLEN);
+  }
+
+  // Function
+  if (typeof target === "function") {
+    return `function ${target.name ? target.name : "anonymous"}()`;
+  }
+
+  // Window
+  if (target.constructor &&
+      target.constructor.name &&
+      target.constructor.name === "Window") {
+    return `window (${target.location.origin})`;
   }
 
-  return out;
+  // Object
+  if (typeof target === "object") {
+    let out = "{";
+
+    let entries = Object.entries(target);
+    for (let i = 0; i < Math.min(10, entries.length); i++) {
+      let [name, value] = entries[i];
+
+      if (i > 0) {
+        out += ", ";
+      }
+
+      out += `${name}: ${truncate(value, MAXLEN)}`;
+    }
+
+    return out + "}";
+  }
+
+  // Other
+  return truncate(target.toSource(), MAXLEN);
+}
+
+function truncate(value, maxLen) {
+  // We don't use value.toString() because it can throw.
+  let str = String(value);
+  return str.length > maxLen ? str.substring(0, maxLen) + "..." : str;
 }
 
 function logEvent(type, args) {
   if (!loggingEnabled) {
     return;
   }
 
   let argsOut = "";
-  let description = describeNthCaller(2);
 
   // We need this try / catch to prevent any dead object errors.
   try {
-    argsOut = args.map(serialize).join(", ");
+    argsOut = `${args.map(serialize).join(", ")}`;
   } catch (e) {
     // Object is dead so the toolbox is most likely shutting down,
     // do nothing.
   }
 
-  dump(`EMITTING: emit(${type}${argsOut}) from ${description}\n`);
+  const path = getNthPathExcluding(0, "devtools/shared/event-emitter.js");
+
+  if (args.length > 0) {
+    dump(`EMITTING: emit(${type}, ${argsOut}) from ${path}\n`);
+  } else {
+    dump(`EMITTING: emit(${type}) from ${path}\n`);
+  }
 }
--- a/devtools/shared/old-event-emitter.js
+++ b/devtools/shared/old-event-emitter.js
@@ -1,17 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const Services = require("Services");
 const defer = require("devtools/shared/defer");
-const { describeNthCaller } = require("devtools/shared/platform/stack");
+const { getNthPathExcluding } = require("devtools/shared/platform/stack");
 let loggingEnabled = false;
 
 if (!isWorker) {
   loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
   Services.prefs.addObserver("devtools.dump.emit", {
     observe: () => {
       loggingEnabled = Services.prefs.getBoolPref("devtools.dump.emit");
     }
@@ -145,17 +145,17 @@ EventEmitter.prototype = {
     }
   },
 
   logEvent(event, args) {
     if (!loggingEnabled) {
       return;
     }
 
-    let description = describeNthCaller(2);
+    let description = getNthPathExcluding(0, "devtools/shared/old-event-emitter.js");
 
     let argOut = "(";
     if (args.length === 1) {
       argOut += event;
     }
 
     let out = "EMITTING: ";
 
--- a/devtools/shared/platform/stack.js
+++ b/devtools/shared/platform/stack.js
@@ -5,41 +5,46 @@
 // A few wrappers for stack-manipulation.  This version of the module
 // is used in chrome code.
 
 "use strict";
 
 const { Cu, components } = require("chrome");
 
 /**
- * Return a description of the Nth caller, suitable for logging.
+ * Return the Nth path from the stack excluding substr.
  *
- * @param {Number} n the caller to describe
- * @return {String} a description of the nth caller.
+ * @param {Number}
+ *        n the Nth path from the stack to describe.
+ * @param {String} substr
+ *        A segment of the path that should be excluded.
  */
-function describeNthCaller(n) {
-  if (isWorker) {
-    return "";
+function getNthPathExcluding(n, substr) {
+  let stack = components.stack.formattedStack.split("\n");
+
+  // Remove this method from the stack
+  stack = stack.splice(1);
+
+  stack = stack.map(line => {
+    if (line.includes(" -> ")) {
+      return line.split(" -> ")[1];
+    }
+    return line;
+  });
+
+  if (substr) {
+    stack = stack.filter(line => {
+      return line && !line.includes(substr);
+    });
   }
 
-  let caller = components.stack;
-  // Do one extra iteration to skip this function.
-  while (n >= 0) {
-    --n;
-    caller = caller.caller;
+  if (!stack[n]) {
+    n = 0;
   }
-
-  let func = caller.name;
-  let file = caller.filename;
-  if (file.includes(" -> ")) {
-    file = caller.filename.split(/ -> /)[1];
-  }
-  let path = file + ":" + caller.lineNumber;
-
-  return func + "() -> " + path;
+  return (stack[n] || "");
 }
 
 /**
  * Return a stack object that can be serialized and, when
  * deserialized, passed to callFunctionWithAsyncStack.
  */
 function getStack() {
   return components.stack.caller;
@@ -52,10 +57,10 @@ function getStack() {
 function callFunctionWithAsyncStack(callee, stack, id) {
   if (isWorker) {
     return callee();
   }
   return Cu.callFunctionWithAsyncStack(callee, stack, id);
 }
 
 exports.callFunctionWithAsyncStack = callFunctionWithAsyncStack;
-exports.describeNthCaller = describeNthCaller;
+exports.getNthPathExcluding = getNthPathExcluding;
 exports.getStack = getStack;