diff --git a/.eslintignore b/.eslintignore index 50ff9b573e87..5d2416efbd46 100644 --- a/.eslintignore +++ b/.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/** diff --git a/devtools/.eslintrc b/devtools/.eslintrc index adc74e4a7f81..409400983dc2 100644 --- a/devtools/.eslintrc +++ b/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 diff --git a/devtools/client/inspector/.eslintrc b/devtools/client/inspector/.eslintrc index 690da943289c..cb0a3d43c1ca 100644 --- a/devtools/client/inspector/.eslintrc +++ b/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)/.*)$"], }, } diff --git a/devtools/shared/DevToolsUtils.js b/devtools/shared/DevToolsUtils.js index 04595916b373..10e6bcc4c94c 100644 --- a/devtools/shared/DevToolsUtils.js +++ b/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; diff --git a/devtools/shared/Loader.jsm b/devtools/shared/Loader.jsm index 03b6f1b4504a..a4236c241406 100644 --- a/devtools/shared/Loader.jsm +++ b/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", diff --git a/devtools/shared/client/main.js b/devtools/shared/client/main.js index cdfa889e1539..75d058698bfa 100644 --- a/devtools/shared/client/main.js +++ b/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(); } diff --git a/devtools/shared/event-emitter.js b/devtools/shared/event-emitter.js index e676e2cdc318..5fcd5dcaa288 100644 --- a/devtools/shared/event-emitter.js +++ b/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); }, diff --git a/devtools/shared/moz.build b/devtools/shared/moz.build index 08c4483e36e0..3187cab30f7f 100644 --- a/devtools/shared/moz.build +++ b/devtools/shared/moz.build @@ -19,6 +19,7 @@ DIRS += [ 'layout', 'locales', 'performance', + 'platform', 'pretty-fast', 'qrcode', 'security', diff --git a/devtools/shared/platform/README.md b/devtools/shared/platform/README.md new file mode 100644 index 000000000000..84accd495423 --- /dev/null +++ b/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. diff --git a/devtools/shared/platform/chrome/moz.build b/devtools/shared/platform/chrome/moz.build new file mode 100644 index 000000000000..d3044ed320de --- /dev/null +++ b/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', +) diff --git a/devtools/shared/platform/chrome/stack.js b/devtools/shared/platform/chrome/stack.js new file mode 100644 index 000000000000..abbe54120184 --- /dev/null +++ b/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; +}); diff --git a/devtools/shared/platform/content/moz.build b/devtools/shared/platform/content/moz.build new file mode 100644 index 000000000000..139d8c7b845a --- /dev/null +++ b/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'] diff --git a/devtools/shared/platform/content/stack.js b/devtools/shared/platform/content/stack.js new file mode 100644 index 000000000000..87c7c411108a --- /dev/null +++ b/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; diff --git a/devtools/shared/platform/content/test/.eslintrc b/devtools/shared/platform/content/test/.eslintrc new file mode 100644 index 000000000000..211e047bc776 --- /dev/null +++ b/devtools/shared/platform/content/test/.eslintrc @@ -0,0 +1,4 @@ +{ + // Extend from the common devtools xpcshell eslintrc config. + "extends": "../../../../.eslintrc.xpcshell" +} diff --git a/devtools/shared/platform/content/test/test_stack.js b/devtools/shared/platform/content/test/test_stack.js new file mode 100644 index 000000000000..4dbb6654103f --- /dev/null +++ b/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"); +} diff --git a/devtools/shared/platform/content/test/xpcshell.ini b/devtools/shared/platform/content/test/xpcshell.ini new file mode 100644 index 000000000000..fa475165a596 --- /dev/null +++ b/devtools/shared/platform/content/test/xpcshell.ini @@ -0,0 +1,7 @@ +[DEFAULT] +tags = devtools +head = +tail = +firefox-appdir = browser + +[test_stack.js] diff --git a/devtools/shared/platform/moz.build b/devtools/shared/platform/moz.build new file mode 100644 index 000000000000..84f56dce1b7f --- /dev/null +++ b/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', +] diff --git a/devtools/shared/protocol.js b/devtools/shared/protocol.js index 214804e85ca6..89b25b8cfdba 100644 --- a/devtools/shared/protocol.js +++ b/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 diff --git a/devtools/shared/worker/loader.js b/devtools/shared/worker/loader.js index 973354de341a..1de72963df67 100644 --- a/devtools/shared/worker/loader.js +++ b/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",