Bug 1287910 - move devtools stack-related APIs to per-platform require; r=jryans

MozReview-Commit-ID: CgT1VGJnJqB

--HG--
extra : rebase_source : 14c52fe412675d9442d8d723cc3e8da55b169079

.eslintignore

@@ -142,6 +142,7 @@ devtools/shared/heapsnapshot/**
 devtools/shared/layout/**
 devtools/shared/locales/**
 devtools/shared/performance/**
+!devtools/shared/platform/**
 devtools/shared/qrcode/**
 devtools/shared/security/**
 devtools/shared/shims/**

devtools/.eslintrc

@@ -42,6 +42,10 @@
     "mozilla/no-single-arg-cu-import": 2,
     // See bug 1224289.
     "mozilla/reject-importGlobalProperties": 2,
+    // devtools/shared/platform is special; see the README.md in that
+    // directory for details.  We reject requires using explicit
+    // subdirectories of this directory.
+    "mozilla/reject-some-requires": [2, "^devtools/shared/platform/(chome|content)/"],
     "mozilla/var-only-at-top-level": 1,
 
     // Rules from the React plugin

devtools/client/inspector/.eslintrc

@@ -7,6 +7,6 @@
     // chrome-privileged code, so this rule disallows requiring chrome
     // code. Some files in the inspector disable this rule still. The
     // goal is to enable the rule globally on all files.
-    "mozilla/reject-some-requires": [2, "^(chrome|chrome:.*|resource:.*|devtools/server/.*|.*\\.jsm)$"],
+    "mozilla/reject-some-requires": [2, "^(chrome|chrome:.*|resource:.*|devtools/server/.*|.*\\.jsm|devtools/shared/platform/(chome|content)/.*)$"],
   },
 }

devtools/shared/DevToolsUtils.js

@@ -11,6 +11,7 @@ var Services = require("Services");
 var promise = require("promise");
 var defer = require("devtools/shared/defer");
 var flags = require("./flags");
+var {getStack, callFunctionWithAsyncStack} = require("devtools/shared/platform/stack");
 
 loader.lazyRequireGetter(this, "FileUtils",
                          "resource://gre/modules/FileUtils.jsm", true);
@@ -32,9 +33,9 @@ exports.executeSoon = function executeSoon(aFn) {
     // Only enable async stack reporting when DEBUG_JS_MODULES is set
     // (customized local builds) to avoid a performance penalty.
     if (AppConstants.DEBUG_JS_MODULES || flags.testing) {
-      let stack = components.stack;
+      let stack = getStack();
       executor = () => {
-        Cu.callFunctionWithAsyncStack(aFn, stack, "DevToolsUtils.executeSoon");
+        callFunctionWithAsyncStack(aFn, stack, "DevToolsUtils.executeSoon");
       };
     } else {
       executor = aFn;

devtools/shared/Loader.jsm

@@ -33,6 +33,13 @@ BuiltinProvider.prototype = {
       // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
       "": "resource://gre/modules/commonjs/",
       // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
+      // Modules here are intended to have one implementation for
+      // chrome, and a separate implementation for content.  Here we
+      // map the directory to the chrome subdirectory, but the content
+      // loader will map to the content subdirectory.  See the
+      // README.md in devtools/shared/platform.
+      "devtools/shared/platform": "resource://devtools/shared/platform/chrome",
+      // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
       "devtools": "resource://devtools",
       // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
       "gcli": "resource://devtools/shared/gcli/source/lib/gcli",

devtools/shared/client/main.js

@@ -6,9 +6,10 @@
 
 "use strict";
 
-const { Ci, Cu, components } = require("chrome");
+const { Ci, Cu } = require("chrome");
 const Services = require("Services");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
+const { getStack, callFunctionWithAsyncStack } = require("devtools/shared/platform/stack");
 
 const promise = Cu.import("resource://devtools/shared/deprecated-sync-thenables.js", {}).Promise;
 
@@ -703,7 +704,7 @@ DebuggerClient.prototype = {
 
     let request = new Request(aRequest);
     request.format = "json";
-    request.stack = components.stack;
+    request.stack = getStack();
     if (aOnResponse) {
       request.on("json-reply", aOnResponse);
     }
@@ -1009,8 +1010,8 @@ DebuggerClient.prototype = {
     if (activeRequest) {
       let emitReply = () => activeRequest.emit("json-reply", aPacket);
       if (activeRequest.stack) {
-        Cu.callFunctionWithAsyncStack(emitReply, activeRequest.stack,
-                                      "DevTools RDP");
+        callFunctionWithAsyncStack(emitReply, activeRequest.stack,
+                                   "DevTools RDP");
       } else {
         emitReply();
       }

devtools/shared/event-emitter.js

@@ -45,11 +45,11 @@
           return Cu.import("resource://gre/modules/Promise.jsm", {}).Promise.defer;
         case "Services":
           return Cu.import("resource://gre/modules/Services.jsm", {}).Services;
-        case "chrome":
-          return {
-            Cu,
-            components: Components
-          };
+        case "devtools/shared/platform/stack": {
+          let obj = {};
+          Cu.import("resource://devtools/shared/platform/chrome/stack.js", obj);
+          return obj;
+        }
       }
       return null;
     };
@@ -64,9 +64,9 @@
   module.exports = EventEmitter;
 
   // See comment in JSM module boilerplate when adding a new dependency.
-  const { components } = require("chrome");
   const Services = require("Services");
   const defer = require("devtools/shared/defer");
+  const { describeNthCaller } = require("devtools/shared/platform/stack");
   let loggingEnabled = true;
 
   if (!isWorker) {
@@ -204,16 +204,7 @@
         return;
       }
 
-      let caller, func, path;
-      if (!isWorker) {
-        caller = components.stack.caller.caller;
-        func = caller.name;
-        let file = caller.filename;
-        if (file.includes(" -> ")) {
-          file = caller.filename.split(/ -> /)[1];
-        }
-        path = file + ":" + caller.lineNumber;
-      }
+      let description = describeNthCaller(2);
 
       let argOut = "(";
       if (args.length === 1) {
@@ -251,7 +242,7 @@
       }
 
       argOut += ")";
-      out += "emit" + argOut + " from " + func + "() -> " + path + "\n";
+      out += "emit" + argOut + " from " + description + "\n";
 
       dump(out);
     },

devtools/shared/moz.build

@@ -19,6 +19,7 @@ DIRS += [
     'layout',
     'locales',
     'performance',
+    'platform',
     'pretty-fast',
     'qrcode',
     'security',

devtools/shared/platform/README.md

@@ -0,0 +1,13 @@
+This directory is treated specially by the loaders.
+
+In particular, when running in chrome, a resource like
+"devtools/shared/platform/mumble" will be found in the chrome
+subdirectory; and when running in content, it will be found in the
+content subdirectory.
+
+Outside of tests, it's not ok to require a specific version of a file;
+and there is an eslint test to check for that.  That is,
+require("devtools/shared/platform/client/mumble") is an error.
+
+When adding a new file, you must add two copies, one to chrome and one
+to content.  Otherwise, one case or the other will fail to work.

devtools/shared/platform/chrome/moz.build

@@ -0,0 +1,9 @@
+# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# 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/.
+
+DevToolsModules(
+    'stack.js',
+)

devtools/shared/platform/chrome/stack.js

@@ -0,0 +1,75 @@
+/* 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/. */
+
+// A few wrappers for stack-manipulation.  This version of the module
+// is used in chrome code.
+
+"use strict";
+
+(function (factory) {
+  // This file might be require()d, but might also be loaded via
+  // Cu.import.  Account for the differences here.
+  if (this.module && module.id.indexOf("stack") >= 0) {
+    // require.
+    const {components, Cu} = require("chrome");
+    factory.call(this, components, Cu, exports);
+  } else {
+    // Cu.import.
+    this.isWorker = false;
+    factory.call(this, Components, Components.utils, this);
+    this.EXPORTED_SYMBOLS = ["callFunctionWithAsyncStack", "describeNthCaller",
+                             "getStack"];
+  }
+}).call(this, function (components, Cu, exports) {
+  /**
+   * Return a description of the Nth caller, suitable for logging.
+   *
+   * @param {Number} n the caller to describe
+   * @return {String} a description of the nth caller.
+   */
+  function describeNthCaller(n) {
+    if (isWorker) {
+      return "";
+    }
+
+    let caller = components.stack;
+    // Do one extra iteration to skip this function.
+    while (n >= 0) {
+      --n;
+      caller = caller.caller;
+    }
+
+    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 a stack object that can be serialized and, when
+   * deserialized, passed to callFunctionWithAsyncStack.
+   */
+  function getStack() {
+    return components.stack.caller;
+  }
+
+  /**
+   * Like Cu.callFunctionWithAsyncStack but handles the isWorker case
+   * -- |Cu| isn't defined in workers.
+   */
+  function callFunctionWithAsyncStack(callee, stack, id) {
+    if (isWorker) {
+      return callee();
+    }
+    return Cu.callFunctionWithAsyncStack(callee, stack, id);
+  }
+
+  exports.callFunctionWithAsyncStack = callFunctionWithAsyncStack;
+  exports.describeNthCaller = describeNthCaller;
+  exports.getStack = getStack;
+});

devtools/shared/platform/content/moz.build

@@ -0,0 +1,11 @@
+# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# 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/.
+
+DevToolsModules(
+    'stack.js',
+)
+
+XPCSHELL_TESTS_MANIFESTS += ['test/xpcshell.ini']

devtools/shared/platform/content/stack.js

@@ -0,0 +1,49 @@
+/* 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/. */
+
+// A few wrappers for stack-manipulation.  This version of the module
+// is used in content code.  Note that this particular copy of the
+// file can only be loaded via require(), because Cu.import doesn't
+// exist in the content case.  So, we don't need the code to handle
+// both require and import here.
+
+"use strict";
+
+/**
+ * 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.
+ *
+ * @param {Number} n the caller to describe
+ * @return {String} a description of the nth caller.
+ */
+function describeNthCaller(n) {
+  if (isWorker) {
+    return "";
+  }
+
+  let stack = new Error().stack.split("\n");
+  // Add one here to skip this function.
+  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.getStack = getStack;

devtools/shared/platform/content/test/.eslintrc

@@ -0,0 +1,4 @@
+{
+  // Extend from the common devtools xpcshell eslintrc config.
+  "extends": "../../../../.eslintrc.xpcshell"
+}

devtools/shared/platform/content/test/test_stack.js

@@ -0,0 +1,48 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// There isn't really very much about the content stack.js that we can
+// test, but we'll do what we can.
+
+"use strict";
+
+var Cu = Components.utils;
+const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
+
+// Make sure to explicitly require the content version of this module.
+// We have to use the ".." trick due to the way the loader remaps
+// devtools/shared/platform.
+const {
+  callFunctionWithAsyncStack,
+  getStack,
+  describeNthCaller
+} = require("devtools/shared/platform/../content/stack");
+
+function f3() {
+  return describeNthCaller(2);
+}
+
+function f2() {
+  return f3();
+}
+
+function f1() {
+  return f2();
+}
+
+function run_test() {
+  let value = 7;
+
+  const changeValue = () => {
+    value = 9;
+  };
+
+  callFunctionWithAsyncStack(changeValue, getStack(), "test_stack");
+  equal(value, 9, "callFunctionWithAsyncStack worked");
+
+  let stack = getStack();
+  equal(JSON.parse(JSON.stringify(stack)), stack, "stack is serializable");
+
+  let desc = f1();
+  ok(desc.includes("f1"), "stack description includes f1");
+}

devtools/shared/platform/content/test/xpcshell.ini

@@ -0,0 +1,7 @@
+[DEFAULT]
+tags = devtools
+head =
+tail =
+firefox-appdir = browser
+
+[test_stack.js]

devtools/shared/platform/moz.build

@@ -0,0 +1,10 @@
+# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# 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/.
+
+DIRS += [
+    'chrome',
+    'content',
+]

devtools/shared/protocol.js

@@ -4,14 +4,13 @@
 
 "use strict";
 
-var { Cu, components } = require("chrome");
-var Services = require("Services");
 var promise = require("promise");
 var defer = require("devtools/shared/defer");
 var {Class} = require("sdk/core/heritage");
 var {EventTarget} = require("sdk/event/target");
 var events = require("sdk/event/core");
 var object = require("sdk/util/object");
+var {getStack, callFunctionWithAsyncStack} = require("devtools/shared/platform/stack");
 
 exports.emit = events.emit;
 
@@ -1206,7 +1205,7 @@ var Front = Class({
       deferred,
       to: to || this.actorID,
       type,
-      stack: components.stack,
+      stack: getStack(),
     });
     this.send(packet);
     return deferred.promise;
@@ -1252,7 +1251,7 @@ var Front = Class({
     }
 
     let { deferred, stack } = this._requests.shift();
-    Cu.callFunctionWithAsyncStack(() => {
+    callFunctionWithAsyncStack(() => {
       if (packet.error) {
         // "Protocol error" is here to avoid TBPL heuristics. See also
         // https://mxr.mozilla.org/webtools-central/source/tbpl/php/inc/GeneralErrorFilter.php

devtools/shared/worker/loader.js

@@ -498,6 +498,13 @@ this.worker = new WorkerDebuggerLoader({
     // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
     "": "resource://gre/modules/commonjs/",
     // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
+    // Modules here are intended to have one implementation for
+    // chrome, and a separate implementation for content.  Here we
+    // map the directory to the chrome subdirectory, but the content
+    // loader will map to the content subdirectory.  See the
+    // README.md in devtools/shared/platform.
+    "devtools/shared/platform": "resource://devtools/shared/platform/chrome",
+    // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
     "devtools": "resource://devtools",
     // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
     "promise": "resource://gre/modules/Promise-backend.js",