Skip to content

Commit

Permalink
Bug 1644187 - Add server support for CSS warning resources. r=jdescot…
Browse files Browse the repository at this point in the history
…tes.

This is quite similar to the error messages watcher, except that we call
ensureCSSErrorReportingEnabled after retrieving the cached messages.

Resource no longer includes `errorMessageName` and `isPromiseRejection` as it
makes little sense for the CSS warning.

The existing resource test is updated to run with and without the server support.

Differential Revision: https://phabricator.services.mozilla.com/D83047
  • Loading branch information
nchevobbe committed Jul 17, 2020
1 parent ec43ae2 commit 8e74f48
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const rawPackets = new Map();
rawPackets.set(`Unknown property ‘such-unknown-property’. Declaration dropped.`, {
"pageError": {
"errorMessage": "Unknown property ‘such-unknown-property’. Declaration dropped.",
"errorMessageName": "",
"sourceName": "http://example.com/browser/devtools/client/webconsole/test/browser/stub-generators/test-css-message.html",
"sourceId": null,
"lineText": "",
Expand All @@ -37,7 +36,6 @@ rawPackets.set(`Unknown property ‘such-unknown-property’. Declaration dropp
"stacktrace": null,
"notes": null,
"chromeContext": false,
"isPromiseRejection": false,
"isForwardedFromContentProcess": false
},
"resourceType": "css-message",
Expand All @@ -47,7 +45,6 @@ rawPackets.set(`Unknown property ‘such-unknown-property’. Declaration dropp
rawPackets.set(`Error in parsing value for ‘padding-top’. Declaration dropped.`, {
"pageError": {
"errorMessage": "Error in parsing value for ‘padding-top’. Declaration dropped.",
"errorMessageName": "",
"sourceName": "http://example.com/browser/devtools/client/webconsole/test/browser/stub-generators/test-css-message.html",
"sourceId": null,
"lineText": "",
Expand All @@ -63,7 +60,6 @@ rawPackets.set(`Error in parsing value for ‘padding-top’. Declaration dropp
"stacktrace": null,
"notes": null,
"chromeContext": false,
"isPromiseRejection": false,
"isForwardedFromContentProcess": false
},
"resourceType": "css-message",
Expand Down
4 changes: 3 additions & 1 deletion devtools/server/actors/descriptors/watcher/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
// FF77+ supports frames in Watcher actor
frame: true,
resources: {
// FF79+ supports console and platform messages, FF80+ supports error messages,
// FF79+ supports console and platform messages, FF80+ supports error and css messages,
// but this isn't enabled yet.
// We will implement a few other resources before enabling it in bug 1642295.
// This is to prevent having to handle backward compat if we have to change Watcher
Expand All @@ -106,6 +106,8 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
// this more broadly.
[Resources.TYPES.CONSOLE_MESSAGE]:
enableServerWatcher && hasBrowserElement,
[Resources.TYPES.CSS_MESSAGE]:
enableServerWatcher && hasBrowserElement,
[Resources.TYPES.ERROR_MESSAGE]:
enableServerWatcher && hasBrowserElement,
[Resources.TYPES.PLATFORM_MESSAGE]: enableServerWatcher,
Expand Down
131 changes: 131 additions & 0 deletions devtools/server/actors/resources/css-messages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/* 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 nsIConsoleListenerWatcher = require("devtools/server/actors/resources/utils/nsi-console-listener-watcher");
const { Ci } = require("chrome");
const { DevToolsServer } = require("devtools/server/devtools-server");
const { createStringGrip } = require("devtools/server/actors/object/utils");
const {
getActorIdForInternalSourceId,
} = require("devtools/server/actors/utils/dbg-source");

const {
TYPES: { CSS_MESSAGE },
} = require("devtools/server/actors/resources/index");

class CSSMessageWatcher extends nsIConsoleListenerWatcher {
/**
* Start watching for all CSS messages related to a given Target Actor.
* This will notify about existing messages, but also the one created in future.
*
* @param TargetActor targetActor
* The target actor from which we should observe messages
* @param Object options
* Dictionary object with following attributes:
* - onAvailable: mandatory function
* This will be called for each resource.
*/
constructor(targetActor, { onAvailable }) {
super(targetActor, { onAvailable });

// Calling ensureCSSErrorReportingEnabled will make the server parse the stylesheets to
// retrieve the warnings if the docShell wasn't already watching for CSS messages.
if (targetActor.ensureCSSErrorReportingEnabled) {
targetActor.ensureCSSErrorReportingEnabled();
}
}

/**
* Returns true if the message is considered a CSS message, and as a result, should
* be sent to the client.
*
* @param {nsIConsoleMessage|nsIScriptError} message
*/
shouldHandleMessage(targetActor, message) {
// The listener we use can be called either with a nsIConsoleMessage or as nsIScriptError.
// In this file, we want to ignore anything but nsIScriptError.
if (
// We only care about CSS Parser nsIScriptError
!(message instanceof Ci.nsIScriptError) ||
message.category !== "CSS Parser"
) {
return false;
}

// Process targets listen for everything but messages from private windows.
if (this.isProcessTarget(targetActor)) {
return !message.isFromPrivateWindow;
}

if (!message.innerWindowID) {
return false;
}

const { window } = targetActor;
const win = window?.WindowGlobalChild?.getByInnerWindowId(
message.innerWindowID
);
return targetActor.browserId === win?.browsingContext?.browserId;
}

/**
* Prepare an nsIScriptError to be sent to the client.
*
* @param nsIScriptError error
* The page error we need to send to the client.
* @return object
* The object you can send to the remote client.
*/
buildResource(targetActor, error) {
const stack = this.prepareStackForRemote(targetActor, error.stack);
let lineText = error.sourceLine;
if (
lineText &&
lineText.length > DevToolsServer.LONG_STRING_INITIAL_LENGTH
) {
lineText = lineText.substr(0, DevToolsServer.LONG_STRING_INITIAL_LENGTH);
}

const notesArray = this.prepareNotesForRemote(targetActor, error.notes);

// If there is no location information in the error but we have a stack,
// fill in the location with the first frame on the stack.
let { sourceName, sourceId, lineNumber, columnNumber } = error;
if (!sourceName && !sourceId && !lineNumber && !columnNumber && stack) {
sourceName = stack[0].filename;
sourceId = stack[0].sourceId;
lineNumber = stack[0].lineNumber;
columnNumber = stack[0].columnNumber;
}

const pageError = {
errorMessage: createStringGrip(targetActor, error.errorMessage),
sourceName,
sourceId: getActorIdForInternalSourceId(targetActor, sourceId),
lineText,
lineNumber,
columnNumber,
category: error.category,
innerWindowID: error.innerWindowID,
timeStamp: error.timeStamp,
warning: !!(error.flags & error.warningFlag),
error: !(error.flags & (error.warningFlag | error.infoFlag)),
info: !!(error.flags & error.infoFlag),
private: error.isFromPrivateWindow,
stacktrace: stack,
notes: notesArray,
chromeContext: error.isFromChromeContext,
isForwardedFromContentProcess: error.isForwardedFromContentProcess,
};

return {
pageError,
resourceType: CSS_MESSAGE,
cssSelectors: error.cssSelectors,
};
}
}
module.exports = CSSMessageWatcher;
4 changes: 4 additions & 0 deletions devtools/server/actors/resources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

const TYPES = {
CONSOLE_MESSAGE: "console-message",
CSS_MESSAGE: "css-message",
ERROR_MESSAGE: "error-message",
PLATFORM_MESSAGE: "platform-message",
};
Expand All @@ -25,6 +26,9 @@ const Resources = {
[TYPES.CONSOLE_MESSAGE]: {
path: "devtools/server/actors/resources/console-messages",
},
[TYPES.CSS_MESSAGE]: {
path: "devtools/server/actors/resources/css-messages",
},
[TYPES.ERROR_MESSAGE]: {
path: "devtools/server/actors/resources/error-messages",
},
Expand Down
1 change: 1 addition & 0 deletions devtools/server/actors/resources/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ DIRS += [

DevToolsModules(
'console-messages.js',
'css-messages.js',
'error-messages.js',
'index.js',
'platform-messages.js',
Expand Down
17 changes: 10 additions & 7 deletions devtools/server/actors/webconsole.js
Original file line number Diff line number Diff line change
Expand Up @@ -1477,7 +1477,9 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
// cached messages for that window (and not the content messages for example).
if (this.parentActor.isRootActor) {
Services.console.reset();
} else {
} else if (this.consoleServiceListener) {
// If error and css messages are handled by the ResourceWatcher, the
// consoleServiceListener is never instantiated.
this.consoleServiceListener.clearCachedMessages();
}
},
Expand Down Expand Up @@ -1707,9 +1709,11 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
columnNumber = stack[0].columnNumber;
}

const isCSSMessage = pageError.category === "CSS Parser";

const result = {
errorMessage: this._createStringGrip(pageError.errorMessage),
errorMessageName: pageError.errorMessageName,
errorMessageName: isCSSMessage ? undefined : pageError.errorMessageName,
exceptionDocURL: ErrorDocs.GetURL(pageError),
sourceName,
sourceId: this.getActorIdForInternalSourceId(sourceId),
Expand All @@ -1726,14 +1730,13 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
stacktrace: stack,
notes: notesArray,
chromeContext: pageError.isFromChromeContext,
isPromiseRejection: pageError.isPromiseRejection,
isPromiseRejection: isCSSMessage
? undefined
: pageError.isPromiseRejection,
isForwardedFromContentProcess: pageError.isForwardedFromContentProcess,
cssSelectors: isCSSMessage ? pageError.cssSelectors : undefined,
};

if (pageError.category === "CSS Parser") {
result.cssSelectors = pageError.cssSelectors;
}

// If the pageError does have an exception object, we want to return the grip for it,
// but only if we do manage to get the grip, as we're checking the property on the
// client to render things differently.
Expand Down
20 changes: 16 additions & 4 deletions devtools/shared/resources/tests/browser_resources_css_messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,19 @@ httpServer.registerPathHandler(`/test_css_messages.html`, (req, res) => {

const TEST_URI = `http://localhost:${httpServer.identity.primaryPort}/test_css_messages.html`;

add_task(async function testWatchingCssMessages() {
add_task(async function() {
info("Test css messages legacy listener");
await pushPref("devtools.testing.enableServerWatcherSupport", false);
await testWatchingCssMessages();
await testWatchingCachedCssMessages();

info("Test css messages server listener");
await pushPref("devtools.testing.enableServerWatcherSupport", true);
await testWatchingCssMessages();
await testWatchingCachedCssMessages();
});

async function testWatchingCssMessages() {
// 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);
Expand Down Expand Up @@ -66,9 +78,9 @@ add_task(async function testWatchingCssMessages() {
Services.console.reset();
targetList.stopListening();
await client.close();
});
}

add_task(async function testWatchingCachedCssMessages() {
async function testWatchingCachedCssMessages() {
// 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);
Expand Down Expand Up @@ -115,7 +127,7 @@ add_task(async function testWatchingCachedCssMessages() {
Services.console.reset();
targetList.stopListening();
await client.close();
});
}

function setupOnAvailableFunction(targetList, receivedMessages) {
// The expected messages are the CSS warnings:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
expectedPageErrors = {
"document.body.style.color = 'fooColor';": {
errorMessage: /fooColor/,
errorMessageName: undefined,
sourceName: /test_page_errors/,
category: "CSS Parser",
timeStamp: /^\d+$/,
Expand Down

0 comments on commit 8e74f48

Please sign in to comment.