diff --git a/devtools/client/webconsole/webconsole-connection-proxy.js b/devtools/client/webconsole/webconsole-connection-proxy.js index 0594d13d7093..c390fc8587c3 100644 --- a/devtools/client/webconsole/webconsole-connection-proxy.js +++ b/devtools/client/webconsole/webconsole-connection-proxy.js @@ -37,7 +37,6 @@ class WebConsoleConnectionProxy { this._connecter = null; - this._onPageError = this._onPageError.bind(this); this._onNetworkEvent = this._onNetworkEvent.bind(this); this._onNetworkEventUpdate = this._onNetworkEventUpdate.bind(this); this._onTabNavigated = this._onTabNavigated.bind(this); @@ -78,13 +77,8 @@ class WebConsoleConnectionProxy { ); await this.webConsoleUI.setSaveRequestAndResponseBodies(saveBodies); - // Note that we only fetch PageError from _getCachedMessages. - // ConsoleAPI is already fetched via the Resources API. - const cachedMessages = await this._getCachedMessages(); const networkMessages = this._getNetworkMessages(); - const messages = cachedMessages.concat(networkMessages); - messages.sort((a, b) => a.timeStamp - b.timeStamp); - this.dispatchMessagesAdd(messages); + this.dispatchMessagesAdd(networkMessages); this._addWebConsoleFrontEventListeners(); @@ -120,7 +114,7 @@ class WebConsoleConnectionProxy { * @returns Promise */ _attachConsole() { - const listeners = ["PageError", "NetworkActivity"]; + const listeners = ["NetworkActivity"]; // Enable the forwarding of console messages to the parent process // when we open the Browser Console or Toolbox without fission support. If Fission // is enabled, we don't use the ContentProcessMessages listener, but attach to the @@ -139,7 +133,6 @@ class WebConsoleConnectionProxy { _addWebConsoleFrontEventListeners() { this.webConsoleFront.on("networkEvent", this._onNetworkEvent); this.webConsoleFront.on("networkEventUpdate", this._onNetworkEventUpdate); - this.webConsoleFront.on("pageError", this._onPageError); this.webConsoleFront.on( "lastPrivateContextExited", this._onLastPrivateContextExited @@ -158,7 +151,6 @@ class WebConsoleConnectionProxy { _removeWebConsoleFrontEventListeners() { this.webConsoleFront.off("networkEvent", this._onNetworkEvent); this.webConsoleFront.off("networkEventUpdate", this._onNetworkEventUpdate); - this.webConsoleFront.off("pageError", this._onPageError); this.webConsoleFront.off( "lastPrivateContextExited", this._onLastPrivateContextExited @@ -169,32 +161,6 @@ class WebConsoleConnectionProxy { ); } - /** - * Get cached messages from the server. - * - * @private - * @returns A Promise that resolves with the cached messages, or reject if something - * went wrong. - */ - async _getCachedMessages() { - const response = await this.webConsoleFront.getCachedMessages([ - "PageError", - ]); - - if (response.error) { - throw new Error( - `Web Console getCachedMessages error: ${response.error} ${response.message}` - ); - } - - if (this.webConsoleFront.traits.newCacheStructure) { - return response.messages; - } - - // On older server, we're also getting cached LogMessages, so we need to remove them - return response.messages.filter(message => message._type !== "LogMessage"); - } - /** * Get network messages from the server. * @@ -205,22 +171,6 @@ class WebConsoleConnectionProxy { return Array.from(this.webConsoleFront.getNetworkEvents()); } - /** - * The "pageError" message type handler. We redirect any page errors to the UI - * for displaying. - * - * @private - * @param object packet - * The message received from the server. - */ - _onPageError(packet) { - if (!this.webConsoleUI) { - return; - } - packet.type = "pageError"; - this.dispatchMessageAdd(packet); - } - _clearLogpointMessages(logpointId) { if (this.webConsoleUI) { this.webConsoleUI.wrapper.dispatchClearLogpointMessages(logpointId); diff --git a/devtools/client/webconsole/webconsole-ui.js b/devtools/client/webconsole/webconsole-ui.js index a8c596a451c1..88af3d264ffe 100644 --- a/devtools/client/webconsole/webconsole-ui.js +++ b/devtools/client/webconsole/webconsole-ui.js @@ -335,6 +335,7 @@ class WebConsoleUI { await resourceWatcher.watch( [ resourceWatcher.TYPES.CONSOLE_MESSAGES, + resourceWatcher.TYPES.ERROR_MESSAGES, resourceWatcher.TYPES.PLATFORM_MESSAGES, ], this._onResourceAvailable @@ -351,6 +352,14 @@ class WebConsoleUI { return; } + if (resourceType == resourceWatcher.TYPES.ERROR_MESSAGES) { + // resource is the packet sent from `ConsoleActor.getCachedMessages().messages` + // or via ConsoleActor's `pageError` event. + resource.type = "pageError"; + this.wrapper.dispatchMessageAdd(resource); + return; + } + if (resourceType == resourceWatcher.TYPES.PLATFORM_MESSAGES) { // resource is the packet sent from `ConsoleActor.getCachedMessages().messages` // or via ConsoleActor's `logMessage` event. diff --git a/devtools/shared/resources/legacy-listeners/console-messages.js b/devtools/shared/resources/legacy-listeners/console-messages.js index 0c17ecb74694..a1ad75d299a7 100644 --- a/devtools/shared/resources/legacy-listeners/console-messages.js +++ b/devtools/shared/resources/legacy-listeners/console-messages.js @@ -4,13 +4,12 @@ "use strict"; -const Services = require("Services"); - module.exports = async function({ targetList, targetType, targetFront, isTopLevel, + isFissionEnabledOnContentToolbox, onAvailable, }) { // Allow the top level target unconditionnally. @@ -19,9 +18,7 @@ module.exports = async function({ // messages via the process targets // Also ignore workers as they are not supported yet. (see bug 1592584) const isContentToolbox = targetList.targetFront.isLocalTab; - const listenForFrames = - isContentToolbox && - Services.prefs.getBoolPref("devtools.contenttoolbox.fission"); + const listenForFrames = isContentToolbox && isFissionEnabledOnContentToolbox; const isAllowed = isTopLevel || targetType === targetList.TYPES.PROCESS || diff --git a/devtools/shared/resources/legacy-listeners/error-messages.js b/devtools/shared/resources/legacy-listeners/error-messages.js new file mode 100644 index 000000000000..5461983b2219 --- /dev/null +++ b/devtools/shared/resources/legacy-listeners/error-messages.js @@ -0,0 +1,62 @@ +/* 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"; + +module.exports = async function({ + targetList, + targetType, + targetFront, + isTopLevel, + isFissionEnabledOnContentToolbox, + onAvailable, +}) { + // Allow the top level target unconditionnally. + // Also allow frame, but only in content toolbox, when the fission/content toolbox pref is + // set. i.e. still ignore them in the content of the browser toolbox as we inspect + // messages via the process targets + // Also ignore workers as they are not supported yet. (see bug 1592584) + const isContentToolbox = targetList.targetFront.isLocalTab; + const listenForFrames = isContentToolbox && isFissionEnabledOnContentToolbox; + const isAllowed = + isTopLevel || + targetType === targetList.TYPES.PROCESS || + (targetType === targetList.TYPES.FRAME && listenForFrames); + + if (!isAllowed) { + return; + } + + const webConsoleFront = await targetFront.getFront("console"); + + // Request notifying about new messages. Here the "PageError" type start listening for + // both actual PageErrors (emitted as "pageError" events) as well as LogMessages ( + // emitted as "logMessage" events). This function only set up the listener on the + // webConsoleFront for "pageError". + await webConsoleFront.startListeners(["PageError"]); + + // Fetch already existing messages + // /!\ The actor implementation requires to call startListeners("PageError") first /!\ + const { messages } = await webConsoleFront.getCachedMessages(["PageError"]); + + for (let message of messages) { + // On older server (< v77), we're also getting LogMessage cached messages, so we need + // to ignore those. + if ( + !webConsoleFront.traits.newCacheStructure && + message._type !== "PageError" + ) { + continue; + } + + // Cached messages don't have the same shape as live messages, + // so we need to transform them. + if (message._type) { + message = { pageError: message }; + } + onAvailable(message); + } + + webConsoleFront.on("pageError", onAvailable); +}; diff --git a/devtools/shared/resources/legacy-listeners/moz.build b/devtools/shared/resources/legacy-listeners/moz.build index 0bb0566a3c7e..3a083a183183 100644 --- a/devtools/shared/resources/legacy-listeners/moz.build +++ b/devtools/shared/resources/legacy-listeners/moz.build @@ -4,5 +4,6 @@ DevToolsModules( 'console-messages.js', + 'error-messages.js', 'platform-messages.js', ) diff --git a/devtools/shared/resources/resource-watcher.js b/devtools/shared/resources/resource-watcher.js index ba611a630f61..9e26999ccc11 100644 --- a/devtools/shared/resources/resource-watcher.js +++ b/devtools/shared/resources/resource-watcher.js @@ -5,6 +5,7 @@ "use strict"; const EventEmitter = require("devtools/shared/event-emitter"); +const Services = require("Services"); class ResourceWatcher { /** @@ -34,6 +35,16 @@ class ResourceWatcher { this._listenerCount = new Map(); } + get contentToolboxFissionPrefValue() { + if (!this._contentToolboxFissionPrefValue) { + this._contentToolboxFissionPrefValue = Services.prefs.getBoolPref( + "devtools.contenttoolbox.fission", + false + ); + } + return this._contentToolboxFissionPrefValue; + } + /** * Request to start retrieving all already existing instances of given * type of resources and also start watching for the one to be created after. @@ -251,6 +262,7 @@ class ResourceWatcher { targetType, targetFront, isTopLevel, + isFissionEnabledOnContentToolbox: this.contentToolboxFissionPrefValue, onAvailable, }); } @@ -300,6 +312,7 @@ class ResourceWatcher { ResourceWatcher.TYPES = ResourceWatcher.prototype.TYPES = { CONSOLE_MESSAGES: "console-messages", + ERROR_MESSAGES: "error-messages", PLATFORM_MESSAGES: "platform-messages", }; module.exports = { ResourceWatcher }; @@ -310,6 +323,8 @@ module.exports = { ResourceWatcher }; const LegacyListeners = { [ResourceWatcher.TYPES .CONSOLE_MESSAGES]: require("devtools/shared/resources/legacy-listeners/console-messages"), + [ResourceWatcher.TYPES + .ERROR_MESSAGES]: require("devtools/shared/resources/legacy-listeners/error-messages"), [ResourceWatcher.TYPES .PLATFORM_MESSAGES]: require("devtools/shared/resources/legacy-listeners/platform-messages"), }; diff --git a/devtools/shared/resources/tests/browser.ini b/devtools/shared/resources/tests/browser.ini index b3495a02a370..ca0e5390d1c0 100644 --- a/devtools/shared/resources/tests/browser.ini +++ b/devtools/shared/resources/tests/browser.ini @@ -12,6 +12,7 @@ support-files = test_worker.js [browser_resources_console_messages.js] +[browser_resources_error_messages.js] [browser_resources_platform_messages.js] [browser_target_list_frames.js] [browser_target_list_preffedoff.js] diff --git a/devtools/shared/resources/tests/browser_resources_console_messages.js b/devtools/shared/resources/tests/browser_resources_console_messages.js index 63c3bcbb0ca6..a3a0b0bc7db3 100644 --- a/devtools/shared/resources/tests/browser_resources_console_messages.js +++ b/devtools/shared/resources/tests/browser_resources_console_messages.js @@ -303,39 +303,3 @@ function checkConsoleAPICall(call, expected) { checkObject(call, expected); } - -function checkObject(object, expected) { - if (object && object.getGrip) { - object = object.getGrip(); - } - - for (const name of Object.keys(expected)) { - const expectedValue = expected[name]; - const value = object[name]; - checkValue(name, value, expectedValue); - } -} - -function checkValue(name, value, expected) { - if (expected === null) { - ok(!value, "'" + name + "' is null"); - } else if (value === undefined) { - ok(false, "'" + name + "' is undefined"); - } else if (value === null) { - ok(false, "'" + name + "' is null"); - } else if ( - typeof expected == "string" || - typeof expected == "number" || - typeof expected == "boolean" - ) { - is(value, expected, "property '" + name + "'"); - } else if (expected instanceof RegExp) { - ok(expected.test(value), name + ": " + expected + " matched " + value); - } else if (Array.isArray(expected)) { - info("checking array for property '" + name + "'"); - checkObject(value, expected); - } else if (typeof expected == "object") { - info("checking object for property '" + name + "'"); - checkObject(value, expected); - } -} diff --git a/devtools/shared/resources/tests/browser_resources_error_messages.js b/devtools/shared/resources/tests/browser_resources_error_messages.js new file mode 100644 index 000000000000..e2b5b786c54a --- /dev/null +++ b/devtools/shared/resources/tests/browser_resources_error_messages.js @@ -0,0 +1,267 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test the ResourceWatcher API around ERROR_MESSAGES +// Reproduces assertions from devtools/shared/webconsole/test/chrome/test_page_errors.html + +const { + ResourceWatcher, +} = require("devtools/shared/resources/resource-watcher"); + +// Create a simple server so we have a nice sourceName in the resources packets. +const httpServer = createTestHTTPServer(); +httpServer.registerPathHandler(`/test_page_errors.html`, (req, res) => { + res.setStatusLine(req.httpVersion, 200, "OK"); + res.write(`Test Error Messages`); +}); + +const TEST_URI = `http://localhost:${httpServer.identity.primaryPort}/test_page_errors.html`; + +add_task(async function() { + // Disable the preloaded process as it creates processes intermittently + // which forces the emission of RDP requests we aren't correctly waiting for. + await pushPref("dom.ipc.processPrelaunch.enabled", false); + + // Open a test tab + gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser); + const tab = await addTab(TEST_URI); + + const { + client, + resourceWatcher, + targetList, + } = await initResourceWatcherAndTarget(tab); + + const receivedMessages = []; + // The expected messages are the errors, twice (once for cached messages, once for live messages) + const expectedMessages = Array.from(expectedPageErrors.values()).concat( + Array.from(expectedPageErrors.values()) + ); + + info( + "Log some errors *before* calling ResourceWatcher.watch in order to assert the behavior of already existing messages." + ); + await triggerErrors(tab); + + let done; + const onAllErrorReceived = new Promise(resolve => (done = resolve)); + + await resourceWatcher.watch( + [ResourceWatcher.TYPES.ERROR_MESSAGES], + ({ resourceType, targetFront, resource }) => { + const { pageError } = resource; + + ok(pageError, "The resource has a pageError attribute"); + + if (!pageError.sourceName.includes("test_page_errors")) { + info(`Ignore error from unknown source: "${pageError.sourceName}"`); + return; + } + + receivedMessages.push(pageError); + + const index = receivedMessages.length - 1; + info("checking received page error #" + index); + checkObject(pageError, expectedMessages[index]); + + if (receivedMessages.length == expectedMessages.length) { + done(); + } + } + ); + + info( + "Now log errors *after* the call to ResourceWatcher.watch and after having received all existing messages" + ); + await triggerErrors(tab); + + info("Waiting for all expected errors to be received"); + await onAllErrorReceived; + ok(true, "All the expected errors were received"); + + Services.console.reset(); + targetList.stopListening(); + await client.close(); +}); + +/** + * Triggers all the errors in the content page. + */ +function triggerErrors(tab) { + return ContentTask.spawn( + tab.linkedBrowser, + expectedPageErrors, + function frameScript(pageErrors) { + const document = content.document; + for (const expression of pageErrors.keys()) { + const container = document.createElement("script"); + document.body.appendChild(container); + container.textContent = expression; + container.remove(); + } + } + ); +} + +const expectedPageErrors = new Map([ + [ + "document.doTheImpossible();", + { + errorMessage: /doTheImpossible/, + errorMessageName: undefined, + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "(42).toString(0);", + { + errorMessage: /radix/, + errorMessageName: "JSMSG_BAD_RADIX", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "'use strict'; (Object.freeze({name: 'Elsa', score: 157})).score = 0;", + { + errorMessage: /read.only/, + errorMessageName: "JSMSG_READ_ONLY", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "([]).length = -1", + { + errorMessage: /array length/, + errorMessageName: "JSMSG_BAD_ARRAY_LENGTH", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "'abc'.repeat(-1);", + { + errorMessage: /repeat count.*non-negative/, + errorMessageName: "JSMSG_NEGATIVE_REPETITION_COUNT", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "'a'.repeat(2e28);", + { + errorMessage: /repeat count.*less than infinity/, + errorMessageName: "JSMSG_RESULTING_STRING_TOO_LARGE", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "77.1234.toExponential(-1);", + { + errorMessage: /out of range/, + errorMessageName: "JSMSG_PRECISION_RANGE", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "function a() { return; 1 + 1; }", + { + errorMessage: /unreachable code/, + errorMessageName: "JSMSG_STMT_AFTER_RETURN", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: false, + warning: true, + }, + ], + [ + "{let a, a;}", + { + errorMessage: /redeclaration of/, + errorMessageName: "JSMSG_REDECLARED_VAR", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + notes: [ + { + messageBody: /Previously declared at line/, + frame: { + source: /test_page_errors/, + }, + }, + ], + }, + ], + [ + `var error = new TypeError("abc"); + error.name = "MyError"; + error.message = "here"; + throw error`, + { + errorMessage: /MyError: here/, + errorMessageName: "", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + "DOMTokenList.prototype.contains.call([])", + { + errorMessage: /does not implement interface/, + errorMessageName: "MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], + [ + `var error2 = new TypeError("abc"); + error2.name = "MyPromiseError"; + error2.message = "here2"; + Promise.reject(error2)`, + { + errorMessage: /MyPromiseError: here2/, + errorMessageName: "", + sourceName: /test_page_errors/, + category: "content javascript", + timeStamp: /^\d+$/, + error: true, + warning: false, + }, + ], +]); diff --git a/devtools/shared/resources/tests/head.js b/devtools/shared/resources/tests/head.js index 597cf59625e5..cf6afdd9e02a 100644 --- a/devtools/shared/resources/tests/head.js +++ b/devtools/shared/resources/tests/head.js @@ -53,3 +53,40 @@ async function initResourceWatcherAndTarget(tab) { return { client, resourceWatcher, targetList }; } + +// Copied from devtools/shared/webconsole/test/chrome/common.js +function checkObject(object, expected) { + if (object && object.getGrip) { + object = object.getGrip(); + } + + for (const name of Object.keys(expected)) { + const expectedValue = expected[name]; + const value = object[name]; + checkValue(name, value, expectedValue); + } +} + +function checkValue(name, value, expected) { + if (expected === null) { + ok(!value, "'" + name + "' is null"); + } else if (value === undefined) { + ok(false, "'" + name + "' is undefined"); + } else if (value === null) { + ok(false, "'" + name + "' is null"); + } else if ( + typeof expected == "string" || + typeof expected == "number" || + typeof expected == "boolean" + ) { + is(value, expected, "property '" + name + "'"); + } else if (expected instanceof RegExp) { + ok(expected.test(value), name + ": " + expected + " matched " + value); + } else if (Array.isArray(expected)) { + info("checking array for property '" + name + "'"); + checkObject(value, expected); + } else if (typeof expected == "object") { + info("checking object for property '" + name + "'"); + checkObject(value, expected); + } +}