From 8b439f0bcefa89986372b81a0056ccd0b5358f84 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 29 Jul 2021 20:28:44 +0000 Subject: [PATCH] Bug 1721109 - Move ProfilerGetSymbols code out of toolkit/components/extensions and into devtools directories. r=canaltinova Differential Revision: https://phabricator.services.mozilla.com/D120182 --- .eslintignore | 6 +- .../client/performance-new/@types/gecko.d.ts | 18 +- .../client/performance-new/@types/perf.d.ts | 15 ++ devtools/client/performance-new/browser.js | 2 - devtools/client/performance-new/moz.build | 2 + .../performance-new}/profiler_get_symbols.js | 0 .../performance-new/symbolication-worker.js | 9 +- .../performance-new/symbolication.jsm.js | 171 +++++++++++++++++- .../extensions/ProfilerGetSymbols.jsm | 157 ---------------- toolkit/components/extensions/moz.build | 7 - 10 files changed, 194 insertions(+), 193 deletions(-) rename {toolkit/components/extensions => devtools/client/performance-new}/profiler_get_symbols.js (100%) rename toolkit/components/extensions/ProfilerGetSymbols-worker.js => devtools/client/performance-new/symbolication-worker.js (94%) delete mode 100644 toolkit/components/extensions/ProfilerGetSymbols.jsm diff --git a/.eslintignore b/.eslintignore index c551245983a6..c67199ae99d4 100644 --- a/.eslintignore +++ b/.eslintignore @@ -79,6 +79,9 @@ devtools/client/webconsole/test/browser/code_bundle_invalidmap.js devtools/server/tests/xpcshell/setBreakpoint* devtools/server/tests/xpcshell/sourcemapped.js +# Ignore generated code from wasm-bindgen +devtools/client/performance-new/profiler_get_symbols.js + # Testing syntax error devtools/client/webconsole/test/browser/test-syntaxerror-worklet.js @@ -185,9 +188,6 @@ testing/web-platform/ # toolkit/ exclusions -# Ignore generated code from wasm-bindgen -toolkit/components/extensions/profiler_get_symbols.js - # Intentionally invalid JS toolkit/components/workerloader/tests/moduleF-syntax-error.js diff --git a/devtools/client/performance-new/@types/gecko.d.ts b/devtools/client/performance-new/@types/gecko.d.ts index b1508204162f..db3d8e5311d4 100644 --- a/devtools/client/performance-new/@types/gecko.d.ts +++ b/devtools/client/performance-new/@types/gecko.d.ts @@ -32,8 +32,6 @@ declare namespace MockedExports { typeof import("resource://gre/modules/osfile.jsm"); "resource://gre/modules/AppConstants.jsm": typeof import("resource://gre/modules/AppConstants.jsm"); - "resource://gre/modules/ProfilerGetSymbols.jsm": - typeof import("resource://gre/modules/ProfilerGetSymbols.jsm"); "resource:///modules/CustomizableUI.jsm": typeof import("resource:///modules/CustomizableUI.jsm") "resource:///modules/CustomizableWidgets.jsm": @@ -189,16 +187,6 @@ declare namespace MockedExports { decorate: (target: object) => void; }; - const ProfilerGetSymbolsJSM: { - ProfilerGetSymbols: { - getSymbolTable: ( - path: string, - debugPath: string, - breakpadId: string - ) => any; - }; - }; - const AppConstantsJSM: { AppConstants: { platform: string; @@ -359,10 +347,6 @@ declare module "resource://gre/modules/AppConstants.jsm" { export = MockedExports.AppConstantsJSM; } -declare module "resource://gre/modules/ProfilerGetSymbols.jsm" { - export = MockedExports.ProfilerGetSymbolsJSM; -} - declare module "resource://gre/modules/WebChannel.jsm" { export = MockedExports.WebChannelJSM; } @@ -444,6 +428,8 @@ declare interface ChromeWindow extends Window { ) => void; } +declare class ChromeWorker extends Worker {} + declare interface MenuListElement extends XULElement { value: string; disabled: boolean; diff --git a/devtools/client/performance-new/@types/perf.d.ts b/devtools/client/performance-new/@types/perf.d.ts index 63a6463d6787..dbadd780aa82 100644 --- a/devtools/client/performance-new/@types/perf.d.ts +++ b/devtools/client/performance-new/@types/perf.d.ts @@ -499,3 +499,18 @@ export interface FeatureDescription { // This will give a reason if the feature is disabled. disabledReason?: string; } + +export type SymbolicationWorkerError = { + name: string; + message: string; + fileName?: string; + lineNumber?: number; +}; + +export type SymbolicationWorkerReplyData = + | { + result: SymbolTableAsTuple; + } + | { + error: SymbolicationWorkerError; + }; diff --git a/devtools/client/performance-new/browser.js b/devtools/client/performance-new/browser.js index 95716070626e..055eda66e752 100644 --- a/devtools/client/performance-new/browser.js +++ b/devtools/client/performance-new/browser.js @@ -30,8 +30,6 @@ const lazy = createLazyLoaders({ Chrome: () => require("chrome"), Services: () => require("Services"), OS: () => ChromeUtils.import("resource://gre/modules/osfile.jsm"), - ProfilerGetSymbols: () => - ChromeUtils.import("resource://gre/modules/ProfilerGetSymbols.jsm"), PerfSymbolication: () => ChromeUtils.import( "resource://devtools/client/performance-new/symbolication.jsm.js" diff --git a/devtools/client/performance-new/moz.build b/devtools/client/performance-new/moz.build index 61c0a6aee977..b3e9fe201efc 100644 --- a/devtools/client/performance-new/moz.build +++ b/devtools/client/performance-new/moz.build @@ -14,6 +14,8 @@ DevToolsModules( "browser.js", "initializer.js", "panel.js", + "profiler_get_symbols.js", + "symbolication-worker.js", "symbolication.jsm.js", "typescript-lazy-load.jsm.js", "utils.js", diff --git a/toolkit/components/extensions/profiler_get_symbols.js b/devtools/client/performance-new/profiler_get_symbols.js similarity index 100% rename from toolkit/components/extensions/profiler_get_symbols.js rename to devtools/client/performance-new/profiler_get_symbols.js diff --git a/toolkit/components/extensions/ProfilerGetSymbols-worker.js b/devtools/client/performance-new/symbolication-worker.js similarity index 94% rename from toolkit/components/extensions/ProfilerGetSymbols-worker.js rename to devtools/client/performance-new/symbolication-worker.js index f4a10da629a2..f1d8a21f6b65 100644 --- a/toolkit/components/extensions/ProfilerGetSymbols-worker.js +++ b/devtools/client/performance-new/symbolication-worker.js @@ -10,7 +10,7 @@ importScripts( "resource://gre/modules/osfile.jsm", - "resource://gre/modules/profiler_get_symbols.js" + "resource://devtools/client/performance-new/profiler_get_symbols.js" ); // This worker uses the wasm module that was generated from https://github.com/mstange/profiler-get-symbols. @@ -20,6 +20,7 @@ importScripts( // the wasm code, and returns the symbol table or an error. Then it shuts down // itself. +/* eslint camelcase: 0*/ const { WasmMemBuffer, get_compact_symbol_table } = wasm_bindgen; // Read an open OS.File instance into the Uint8Array dataBuf. @@ -103,7 +104,11 @@ onmessage = async e => { } try { - let output = get_compact_symbol_table(binaryData, debugData, breakpadId); + const output = get_compact_symbol_table( + binaryData, + debugData, + breakpadId + ); const result = [ output.take_addr(), output.take_index(), diff --git a/devtools/client/performance-new/symbolication.jsm.js b/devtools/client/performance-new/symbolication.jsm.js index 41de66a336cf..9ba548b52a70 100644 --- a/devtools/client/performance-new/symbolication.jsm.js +++ b/devtools/client/performance-new/symbolication.jsm.js @@ -13,14 +13,172 @@ const { createLazyLoaders } = ChromeUtils.import( * @typedef {import("./@types/perf").PerfFront} PerfFront * @typedef {import("./@types/perf").SymbolTableAsTuple} SymbolTableAsTuple * @typedef {import("./@types/perf").SymbolicationService} SymbolicationService + * @typedef {import("./@types/perf").SymbolicationWorkerReplyData} SymbolicationWorkerReplyData */ const lazy = createLazyLoaders({ OS: () => ChromeUtils.import("resource://gre/modules/osfile.jsm"), - ProfilerGetSymbols: () => - ChromeUtils.import("resource://gre/modules/ProfilerGetSymbols.jsm"), }); +ChromeUtils.defineModuleGetter( + this, + "setTimeout", + "resource://gre/modules/Timer.jsm" +); +ChromeUtils.defineModuleGetter( + this, + "clearTimeout", + "resource://gre/modules/Timer.jsm" +); + +/** @type {any} */ +const global = this; + +// This module obtains symbol tables for binaries. +// It does so with the help of a WASM module which gets pulled in from the +// internet on demand. We're doing this purely for the purposes of saving on +// code size. The contents of the WASM module are expected to be static, they +// are checked against the hash specified below. +// The WASM code is run on a ChromeWorker thread. It takes the raw byte +// contents of the to-be-dumped binary (and of an additional optional pdb file +// on Windows) as its input, and returns a set of typed arrays which make up +// the symbol table. + +// Don't let the strange looking URLs and strings below scare you. +// The hash check ensures that the contents of the wasm module are what we +// expect them to be. +// The source code is at https://github.com/mstange/profiler-get-symbols/ . + +// Generated from https://github.com/mstange/profiler-get-symbols/commit/90ee39f1d18d2727f07dc57bd93cff6bc73ce8a0 +const WASM_MODULE_URL = + "https://zealous-rosalind-a98ce8.netlify.com/wasm/8f7ca2f70e1cd21b5a2dbe96545672752887bfbd4e7b3b9437e9fc7c3da0a3bedae4584ff734f0c9f08c642e6b66ffab.wasm"; +const WASM_MODULE_INTEGRITY = + "sha384-j3yi9w4c0htaLb6WVFZydSiHv71OezuUN+n8fD2go77a5FhP9zTwyfCMZC5rZv+r"; + +const EXPIRY_TIME_IN_MS = 5 * 60 * 1000; // 5 minutes + +/** @type {Promise | null} */ +let gCachedWASMModulePromise = null; +let gCachedWASMModuleExpiryTimer = 0; + +// Keep active workers alive (see bug 1592227). +const gActiveWorkers = new Set(); + +function clearCachedWASMModule() { + gCachedWASMModulePromise = null; + gCachedWASMModuleExpiryTimer = 0; +} + +function getWASMProfilerGetSymbolsModule() { + if (!gCachedWASMModulePromise) { + gCachedWASMModulePromise = (async function() { + const request = new Request(WASM_MODULE_URL, { + integrity: WASM_MODULE_INTEGRITY, + credentials: "omit", + }); + return WebAssembly.compileStreaming(fetch(request)); + })(); + } + + // Reset expiry timer. + clearTimeout(gCachedWASMModuleExpiryTimer); + gCachedWASMModuleExpiryTimer = setTimeout( + clearCachedWASMModule, + EXPIRY_TIME_IN_MS + ); + + return gCachedWASMModulePromise; +} + +/** + * Obtain symbols for the binary at the specified location. + * + * @param {string} binaryPath The absolute path to the binary on the local + * file system. + * @param {string} debugPath The absolute path to the binary's pdb file on the + * local file system if on Windows, otherwise the same as binaryPath. + * @param {string} breakpadId The breakpadId for the binary whose symbols + * should be obtained. This is used for two purposes: 1) to locate the + * correct single-arch binary in "FatArch" files, and 2) to make sure the + * binary at the given path is actually the one that we want. If no ID match + * is found, this function throws (rejects the promise). + * @returns {Promise} The symbol table in SymbolTableAsTuple format, see the + * documentation for nsIProfiler.getSymbolTable. + */ +async function getSymbolTableFromLocalBinary( + binaryPath, + debugPath, + breakpadId +) { + const module = await getWASMProfilerGetSymbolsModule(); + + return new Promise((resolve, reject) => { + const worker = new ChromeWorker( + "resource://devtools/client/performance-new/symbolication-worker.js" + ); + gActiveWorkers.add(worker); + + /** @param {MessageEvent} msg */ + worker.onmessage = msg => { + gActiveWorkers.delete(worker); + if ("error" in msg.data) { + const error = msg.data.error; + if (error.name) { + // Turn the JSON error object into a real Error object. + const { name, message, fileName, lineNumber } = error; + const ErrorObjConstructor = + name in global && Error.isPrototypeOf(global[name]) + ? global[name] + : Error; + const e = new ErrorObjConstructor(message, fileName, lineNumber); + e.name = name; + reject(e); + } else { + reject(error); + } + return; + } + resolve(msg.data.result); + }; + + // Handle uncaught errors from the worker script. onerror is called if + // there's a syntax error in the worker script, for example, or when an + // unhandled exception is thrown, but not for unhandled promise + // rejections. Without this handler, mistakes during development such as + // syntax errors can be hard to track down. + worker.onerror = errorEvent => { + gActiveWorkers.delete(worker); + worker.terminate(); + if (errorEvent instanceof ErrorEvent) { + const { message, filename, lineno } = errorEvent; + const error = new Error(`${message} at ${filename}:${lineno}`); + error.name = "WorkerError"; + reject(error); + } else { + reject(new Error("Error in worker")); + } + }; + + // Handle errors from messages that cannot be deserialized. I'm not sure + // how to get into such a state, but having this handler seems like a good + // idea. + worker.onmessageerror = errorEvent => { + gActiveWorkers.delete(worker); + worker.terminate(); + if (errorEvent instanceof ErrorEvent) { + const { message, filename, lineno } = errorEvent; + const error = new Error(`${message} at ${filename}:${lineno}`); + error.name = "WorkerMessageError"; + reject(error); + } else { + reject(new Error("Error in worker")); + } + }; + + worker.postMessage({ binaryPath, debugPath, breakpadId, module }); + }); +} + /** * @param {PerfFront} perfFront * @param {string} path @@ -140,20 +298,21 @@ class LocalSymbolicationService { const candidatePaths = this._getCandidatePaths(debugName, breakpadId); // Iterate over all the paths and try to get symbols from each entry. - const { ProfilerGetSymbols } = lazy.ProfilerGetSymbols(); + const errors = []; for (const { path, debugPath } of candidatePaths) { if (await doesFileExistAtPath(path)) { try { - return await ProfilerGetSymbols.getSymbolTable( + return await getSymbolTableFromLocalBinary( path, debugPath, breakpadId ); } catch (e) { - // ProfilerGetSymbols.getSymbolTable was unsuccessful. So either the + // getSymbolTable was unsuccessful. So either the // file wasn't parseable or its contents didn't match the specified // breakpadId, or some other error occurred. // Advance to the next candidate path. + errors.push(e); } } } @@ -162,7 +321,7 @@ class LocalSymbolicationService { `Could not obtain symbols for the library ${debugName} ${breakpadId} ` + `because there was no matching file at any of the candidate paths: ${JSON.stringify( candidatePaths - )}` + )}. Errors: ${errors.map(e => e.message).join(", ")}` ); } diff --git a/toolkit/components/extensions/ProfilerGetSymbols.jsm b/toolkit/components/extensions/ProfilerGetSymbols.jsm deleted file mode 100644 index 47973976ee0c..000000000000 --- a/toolkit/components/extensions/ProfilerGetSymbols.jsm +++ /dev/null @@ -1,157 +0,0 @@ -/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */ -/* vim: set sts=2 sw=2 et tw=80: */ -/* 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 EXPORTED_SYMBOLS = ["ProfilerGetSymbols"]; - -ChromeUtils.defineModuleGetter( - this, - "setTimeout", - "resource://gre/modules/Timer.jsm" -); -ChromeUtils.defineModuleGetter( - this, - "clearTimeout", - "resource://gre/modules/Timer.jsm" -); - -Cu.importGlobalProperties(["fetch"]); - -const global = this; - -// This module obtains symbol tables for binaries. -// It does so with the help of a WASM module which gets pulled in from the -// internet on demand. We're doing this purely for the purposes of saving on -// code size. The contents of the WASM module are expected to be static, they -// are checked against the hash specified below. -// The WASM code is run on a ChromeWorker thread. It takes the raw byte -// contents of the to-be-dumped binary (and of an additional optional pdb file -// on Windows) as its input, and returns a set of typed arrays which make up -// the symbol table. - -// Don't let the strange looking URLs and strings below scare you. -// The hash check ensures that the contents of the wasm module are what we -// expect them to be. -// The source code is at https://github.com/mstange/profiler-get-symbols/ . - -// Generated from https://github.com/mstange/profiler-get-symbols/commit/90ee39f1d18d2727f07dc57bd93cff6bc73ce8a0 -const WASM_MODULE_URL = - "https://zealous-rosalind-a98ce8.netlify.com/wasm/8f7ca2f70e1cd21b5a2dbe96545672752887bfbd4e7b3b9437e9fc7c3da0a3bedae4584ff734f0c9f08c642e6b66ffab.wasm"; -const WASM_MODULE_INTEGRITY = - "sha384-j3yi9w4c0htaLb6WVFZydSiHv71OezuUN+n8fD2go77a5FhP9zTwyfCMZC5rZv+r"; - -const EXPIRY_TIME_IN_MS = 5 * 60 * 1000; // 5 minutes - -let gCachedWASMModulePromise = null; -let gCachedWASMModuleExpiryTimer = 0; - -// Keep active workers alive (see bug 1592227). -let gActiveWorkers = new Set(); - -function clearCachedWASMModule() { - gCachedWASMModulePromise = null; - gCachedWASMModuleExpiryTimer = 0; -} - -function getWASMProfilerGetSymbolsModule() { - if (!gCachedWASMModulePromise) { - gCachedWASMModulePromise = (async function() { - const request = new Request(WASM_MODULE_URL, { - integrity: WASM_MODULE_INTEGRITY, - credentials: "omit", - }); - return WebAssembly.compileStreaming(fetch(request)); - })(); - } - - // Reset expiry timer. - clearTimeout(gCachedWASMModuleExpiryTimer); - gCachedWASMModuleExpiryTimer = setTimeout( - clearCachedWASMModule, - EXPIRY_TIME_IN_MS - ); - - return gCachedWASMModulePromise; -} - -this.ProfilerGetSymbols = { - /** - * Obtain symbols for the binary at the specified location. - * - * @param {string} binaryPath The absolute path to the binary on the local - * file system. - * @param {string} debugPath The absolute path to the binary's pdb file on the - * local file system if on Windows, otherwise the same as binaryPath. - * @param {string} breakpadId The breakpadId for the binary whose symbols - * should be obtained. This is used for two purposes: 1) to locate the - * correct single-arch binary in "FatArch" files, and 2) to make sure the - * binary at the given path is actually the one that we want. If no ID match - * is found, this function throws (rejects the promise). - * @returns {Promise} The symbol table in SymbolTableAsTuple format, see the - * documentation for nsIProfiler.getSymbolTable. - */ - async getSymbolTable(binaryPath, debugPath, breakpadId) { - const module = await getWASMProfilerGetSymbolsModule(); - - return new Promise((resolve, reject) => { - const worker = new ChromeWorker( - "resource://gre/modules/ProfilerGetSymbols-worker.js" - ); - gActiveWorkers.add(worker); - - worker.onmessage = msg => { - gActiveWorkers.delete(worker); - if (msg.data.error) { - const error = msg.data.error; - if (error.name) { - // Turn the JSON error object into a real Error object. - const { name, message, fileName, lineNumber } = error; - const ErrorObjConstructor = - name in global && Error.isPrototypeOf(global[name]) - ? global[name] - : Error; - const e = new ErrorObjConstructor(message, fileName, lineNumber); - e.name = name; - reject(e); - } else { - reject(error); - } - return; - } - resolve(msg.data.result); - }; - - // Handle uncaught errors from the worker script. onerror is called if - // there's a syntax error in the worker script, for example, or when an - // unhandled exception is thrown, but not for unhandled promise - // rejections. Without this handler, mistakes during development such as - // syntax errors can be hard to track down. - worker.onerror = errorEvent => { - gActiveWorkers.delete(worker); - worker.terminate(); - const { message, filename, lineno } = errorEvent; - const error = new Error(message, filename, lineno); - error.name = "WorkerError"; - reject(error); - }; - - // Handle errors from messages that cannot be deserialized. I'm not sure - // how to get into such a state, but having this handler seems like a good - // idea. - worker.onmessageerror = errorEvent => { - gActiveWorkers.delete(worker); - worker.terminate(); - const { message, filename, lineno } = errorEvent; - const error = new Error(message, filename, lineno); - error.name = "WorkerMessageError"; - reject(error); - }; - - worker.postMessage({ binaryPath, debugPath, breakpadId, module }); - }); - }, -}; diff --git a/toolkit/components/extensions/moz.build b/toolkit/components/extensions/moz.build index 721dfa2e7eba..8da7cdf3fd00 100755 --- a/toolkit/components/extensions/moz.build +++ b/toolkit/components/extensions/moz.build @@ -46,13 +46,6 @@ EXTRA_JS_MODULES += [ "WebNavigationFrames.jsm", ] -if CONFIG["MOZ_WIDGET_TOOLKIT"] != "android": - EXTRA_JS_MODULES += [ - "profiler_get_symbols.js", - "ProfilerGetSymbols-worker.js", - "ProfilerGetSymbols.jsm", - ] - EXTRA_COMPONENTS += [ "extensions-toolkit.manifest", ]