Bug 1625910 - Use ResourceAPI for CSS Warning messages.r=ochameau.

Differential Revision: https://phabricator.services.mozilla.com/D79069

devtools/client/webconsole/enhancers/css-error-reporting.js

@@ -7,7 +7,7 @@
 const {
   INITIALIZE,
   FILTER_TOGGLE,
-  TARGET_AVAILABLE,
+  FILTERS,
 } = require("devtools/client/webconsole/constants");
 
 /**
@@ -15,30 +15,23 @@ const {
  * filter is toggled on.
  */
 function ensureCSSErrorReportingEnabled(webConsoleUI) {
+  let watchingCSSMessages = false;
   return next => (reducer, initialState, enhancer) => {
     function ensureErrorReportingEnhancer(state, action) {
-      const proxies = webConsoleUI ? webConsoleUI.getAllProxies() : null;
-      if (!proxies) {
-        return reducer(state, action);
-      }
-
       state = reducer(state, action);
-      if (!state.filters.css) {
+
+      // If we're already watching CSS messages, or if the CSS filter is disabled,
+      // we don't do anything.
+      if (!webConsoleUI || watchingCSSMessages || !state.filters.css) {
         return state;
       }
 
       const cssFilterToggled =
-        action.type == FILTER_TOGGLE && action.filter == "css";
-      if (
-        cssFilterToggled ||
-        action.type == INITIALIZE ||
-        action.type == TARGET_AVAILABLE
-      ) {
-        for (const proxy of proxies) {
-          if (proxy.target && proxy.target.ensureCSSErrorReportingEnabled) {
-            proxy.target.ensureCSSErrorReportingEnabled();
-          }
-        }
+        action.type == FILTER_TOGGLE && action.filter == FILTERS.CSS;
+
+      if (cssFilterToggled || action.type == INITIALIZE) {
+        watchingCSSMessages = true;
+        webConsoleUI.watchCssMessages();
       }
 
       return state;

devtools/client/webconsole/test/browser/browser_webconsole_cached_messages.js

@@ -59,7 +59,7 @@ add_task(async function() {
   info("Log different type of messages to fill the cache");
   await logMessages();
 
-  info("Open the console");
+  info("Select the console");
   hud = await openConsole();
 
   await testMessagesVisibility(hud);
@@ -177,8 +177,9 @@ async function testMessagesVisibility(hud, checkNetworkMessage = true) {
   }
 
   // We can't assert the CSS warning position, so we only check that it's visible.
-  await waitFor(() =>
-    findMessage(hud, "cssColorBug611032", ".message.warn.css")
+  await waitFor(
+    () => findMessage(hud, "cssColorBug611032", ".message.warn.css"),
+    "Couldn't find the CSS warning message"
   );
   ok(true, "css warning message is visible");
 }

devtools/client/webconsole/test/browser/browser_webconsole_filters_persist.js

@@ -43,6 +43,10 @@ add_task(async function() {
   filterButtons.forEach(filterButton => {
     ok(filterIsEnabled(filterButton), "filter is enabled");
   });
+
+  // Wait for the CSS warning to be displayed so we don't have a pending promise.
+  await waitFor(() => findMessage(hud, "Expected color but found ‘blouge’"));
+
   // Check that the ui settings were persisted.
   await closeTabAndToolbox();
 });

devtools/client/webconsole/test/browser/browser_webconsole_stubs_css_message.js

@@ -66,20 +66,19 @@ async function generateCssMessageStubs() {
   // The resource-watcher only supports a single call to watch/unwatch per
   // instance, so we attach a unique watch callback, which will forward the
   // resource to `handleErrorMessage`, dynamically updated for each command.
-  let handleErrorMessage = function() {};
+  let handleCSSMessage = function() {};
 
-  const onErrorMessageAvailable = ({ resource }) => {
-    handleErrorMessage(resource);
+  const onCSSMessageAvailable = ({ resource }) => {
+    handleCSSMessage(resource);
   };
 
-  /* CSS errors are considered as pageError on the server */
-  await resourceWatcher.watchResources([resourceWatcher.TYPES.ERROR_MESSAGE], {
-    onAvailable: onErrorMessageAvailable,
+  await resourceWatcher.watchResources([resourceWatcher.TYPES.CSS_MESSAGE], {
+    onAvailable: onCSSMessageAvailable,
   });
 
   for (const code of getCommands()) {
     const received = new Promise(resolve => {
-      handleErrorMessage = function(packet) {
+      handleCSSMessage = function(packet) {
         const key = packet.pageError.errorMessage;
         stubs.set(key, getCleanedPacket(key, packet));
         resolve();
@@ -98,8 +97,8 @@ async function generateCssMessageStubs() {
     await received;
   }
 
-  resourceWatcher.unwatchResources([resourceWatcher.TYPES.ERROR_MESSAGE], {
-    onAvailable: onErrorMessageAvailable,
+  resourceWatcher.unwatchResources([resourceWatcher.TYPES.CSS_MESSAGE], {
+    onAvailable: onCSSMessageAvailable,
   });
 
   await closeTabAndToolbox().catch(() => {});

devtools/client/webconsole/test/node/fixtures/stubs/cssMessage.js

@@ -37,11 +37,11 @@ rawPackets.set(`Unknown property ‘such-unknown-property’.  Declaration dropp
     "stacktrace": null,
     "notes": null,
     "chromeContext": false,
-    "cssSelectors": "p",
     "isPromiseRejection": false,
     "isForwardedFromContentProcess": false
   },
-  "resourceType": "error-message"
+  "resourceType": "css-message",
+  "cssSelectors": "p"
 });
 
 rawPackets.set(`Error in parsing value for ‘padding-top’.  Declaration dropped.`, {
@@ -63,11 +63,11 @@ rawPackets.set(`Error in parsing value for ‘padding-top’.  Declaration dropp
     "stacktrace": null,
     "notes": null,
     "chromeContext": false,
-    "cssSelectors": "p",
     "isPromiseRejection": false,
     "isForwardedFromContentProcess": false
   },
-  "resourceType": "error-message"
+  "resourceType": "css-message",
+  "cssSelectors": "p"
 });
 
 

devtools/client/webconsole/test/node/helpers.js

@@ -143,6 +143,7 @@ function getWebConsoleUiMock(hud, proxyOverrides) {
     toolbox: {
       sessionId: 1,
     },
+    watchCssMessages: () => {},
   };
 }
 

devtools/client/webconsole/utils/messages.js

@@ -111,6 +111,10 @@ function transformResource(resource) {
       return transformPageErrorResource(resource);
     }
 
+    case ResourceWatcher.TYPES.CSS_MESSAGE: {
+      return transformCSSMessageResource(resource);
+    }
+
     case "networkEvent": {
       return transformNetworkEventResource(resource);
     }
@@ -305,7 +309,7 @@ function transformPlatformMessageResource(platformMessageResource) {
   });
 }
 
-function transformPageErrorResource(pageErrorResource) {
+function transformPageErrorResource(pageErrorResource, override = {}) {
   const { pageError, targetFront } = pageErrorResource;
   let level = MESSAGE_LEVEL.ERROR;
   if (pageError.warning) {
@@ -323,30 +327,37 @@ function transformPageErrorResource(pageErrorResource) {
       }
     : null;
 
-  const matchesCSS = pageError.category == "CSS Parser";
-  const messageSource = matchesCSS
-    ? MESSAGE_SOURCE.CSS
-    : MESSAGE_SOURCE.JAVASCRIPT;
-  return new ConsoleMessage({
-    targetFront,
-    innerWindowID: pageError.innerWindowID,
-    source: messageSource,
-    type: MESSAGE_TYPE.LOG,
-    level,
-    category: pageError.category,
-    messageText: pageError.errorMessage,
-    stacktrace: pageError.stacktrace ? pageError.stacktrace : null,
-    frame,
-    errorMessageName: pageError.errorMessageName,
-    exceptionDocURL: pageError.exceptionDocURL,
-    hasException: pageError.hasException,
-    parameters: pageError.hasException ? [pageError.exception] : null,
-    timeStamp: pageError.timeStamp,
-    notes: pageError.notes,
-    private: pageError.private,
-    chromeContext: pageError.chromeContext,
-    cssSelectors: pageError.cssSelectors,
-    isPromiseRejection: pageError.isPromiseRejection,
+  return new ConsoleMessage(
+    Object.assign(
+      {
+        targetFront,
+        innerWindowID: pageError.innerWindowID,
+        source: MESSAGE_SOURCE.JAVASCRIPT,
+        type: MESSAGE_TYPE.LOG,
+        level,
+        category: pageError.category,
+        messageText: pageError.errorMessage,
+        stacktrace: pageError.stacktrace ? pageError.stacktrace : null,
+        frame,
+        errorMessageName: pageError.errorMessageName,
+        exceptionDocURL: pageError.exceptionDocURL,
+        hasException: pageError.hasException,
+        parameters: pageError.hasException ? [pageError.exception] : null,
+        timeStamp: pageError.timeStamp,
+        notes: pageError.notes,
+        private: pageError.private,
+        chromeContext: pageError.chromeContext,
+        isPromiseRejection: pageError.isPromiseRejection,
+      },
+      override
+    )
+  );
+}
+
+function transformCSSMessageResource(cssMessageResource) {
+  return transformPageErrorResource(cssMessageResource, {
+    cssSelectors: cssMessageResource.cssSelectors,
+    source: MESSAGE_SOURCE.CSS,
   });
 }
 

devtools/client/webconsole/webconsole-ui.js

@@ -346,13 +346,23 @@ class WebConsoleUI {
     );
   }
 
+  async watchCssMessages() {
+    const { resourceWatcher } = this.hud;
+    await resourceWatcher.watchResources([resourceWatcher.TYPES.CSS_MESSAGE], {
+      onAvailable: this._onResourceAvailable,
+    });
+  }
+
   _onResourceAvailable({ resourceType, targetFront, resource }) {
+    const { TYPES } = this.hud.resourceWatcher;
     // Ignore messages forwarded from content processes if we're in fission browser toolbox.
     if (
-      resourceType === this.hud.resourceWatcher.TYPES.ERROR_MESSAGE &&
-      resource.pageError.isForwardedFromContentProcess &&
-      (this.isBrowserToolboxConsole || this.isBrowserConsole) &&
-      this.fissionSupport
+      !this.wrapper ||
+      ((resourceType === TYPES.ERROR_MESSAGE ||
+        resourceType === TYPES.CSS_MESSAGE) &&
+        resource.pageError?.isForwardedFromContentProcess &&
+        (this.isBrowserToolboxConsole || this.isBrowserConsole) &&
+        this.fissionSupport)
     ) {
       return;
     }

devtools/server/actors/targets/browsing-context.js

@@ -1230,7 +1230,8 @@ const browsingContextTargetPrototype = {
   /**
    * Ensure that CSS error reporting is enabled.
    */
-  ensureCSSErrorReportingEnabled(request) {
+  async ensureCSSErrorReportingEnabled(request) {
+    const promises = [];
     for (const docShell of this.docShells) {
       if (docShell.cssErrorReportingEnabled) {
         continue;
@@ -1253,12 +1254,15 @@ const browsingContextTargetPrototype = {
           continue;
         }
         // Reparse the sheet so that we see the existing errors.
-        getSheetText(sheet, this._consoleActor).then(text => {
-          InspectorUtils.parseStyleSheet(sheet, text, /* aUpdate = */ false);
-        });
+        const onStyleSheetParsed = getSheetText(sheet, this._consoleActor)
+          .then(text => {
+            InspectorUtils.parseStyleSheet(sheet, text, /* aUpdate = */ false);
+          })
+          .catch(e => console.error("Error while parsing stylesheet"));
+        promises.push(onStyleSheetParsed);
       }
     }
-
+    await Promise.all(promises);
     return {};
   },
 

devtools/shared/resources/legacy-listeners/css-messages.js

@@ -0,0 +1,77 @@
+/* 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 {
+  ResourceWatcher,
+} = require("devtools/shared/resources/resource-watcher");
+
+module.exports = async function({
+  targetList,
+  targetFront,
+  isFissionEnabledOnContentToolbox,
+  onAvailable,
+}) {
+  // Allow the top level target if the targetFront has an `ensureCSSErrorREportingEnabled`
+  // function. Also allow frame in non-content toolbox and in content toolbox when the
+  // fission toolbox pref is set.
+  const isContentToolbox = targetList.targetFront.isLocalTab;
+  const listenForFrames = !isContentToolbox || isFissionEnabledOnContentToolbox;
+  const isAllowed =
+    typeof targetFront.ensureCSSErrorReportingEnabled == "function" &&
+    (targetFront.isTopLevel ||
+      (targetFront.targetType === targetList.TYPES.FRAME && listenForFrames));
+
+  if (!isAllowed) {
+    return;
+  }
+
+  const webConsoleFront = await targetFront.getFront("console");
+
+  // Request notifying about new CSS messages (they're emitted from the "PageError listener").
+  await webConsoleFront.startListeners(["PageError"]);
+
+  // Fetch already existing messages
+  // /!\ The actor implementation requires to call startListeners("PageError") first /!\
+  const { messages } = await webConsoleFront.getCachedMessages(["PageError"]);
+
+  const cachedMessages = [];
+  for (let message of messages) {
+    if (message.pageError?.category !== "CSS Parser") {
+      continue;
+    }
+
+    // Handling cached messages for servers older than Firefox 78.
+    // Wrap the message into a `pageError` attribute, to match `pageError` behavior
+    if (message._type) {
+      message = {
+        pageError: message,
+      };
+    }
+    message.resourceType = ResourceWatcher.TYPES.CSS_MESSAGE;
+    message.cssSelectors = message.pageError.cssSelectors;
+    delete message.pageError.cssSelectors;
+    cachedMessages.push(message);
+  }
+
+  onAvailable(cachedMessages);
+
+  // Calling ensureCSSErrorReportingEnabled will make the server parse the stylesheets to
+  // retrieve the warnings if the docShell wasn't already watching for CSS messages.
+  await targetFront.ensureCSSErrorReportingEnabled();
+
+  // CSS Messages are emited fron the PageError listener, which also send regular errors
+  // that we need to filter out.
+  webConsoleFront.on("pageError", message => {
+    if (message.pageError.category !== "CSS Parser") {
+      return;
+    }
+
+    message.resourceType = ResourceWatcher.TYPES.CSS_MESSAGE;
+    message.cssSelectors = message.pageError.cssSelectors;
+    delete message.pageError.cssSelectors;
+    onAvailable([message]);
+  });
+};

devtools/shared/resources/legacy-listeners/error-messages.js

@@ -44,11 +44,13 @@ module.exports = async function({
 
   // On older server (< v77), we're also getting LogMessage cached messages, so we need
   // to ignore those.
+  // On server < v79, we're also getting CSS Messages that we need to filter out.
   messages = messages.filter(message => {
     return (
-      webConsoleFront.traits.newCacheStructure ||
-      !message._type ||
-      message._type == "PageError"
+      (webConsoleFront.traits.newCacheStructure ||
+        !message._type ||
+        message._type == "PageError") &&
+      message.pageError.category !== "CSS Parser"
     );
   });
 
@@ -69,6 +71,11 @@ module.exports = async function({
   onAvailable(messages);
 
   webConsoleFront.on("pageError", message => {
+    // On server < v79, we're getting CSS Messages that we need to filter out.
+    if (message.pageError.category === "CSS Parser") {
+      return;
+    }
+
     message.resourceType = ResourceWatcher.TYPES.ERROR_MESSAGE;
     onAvailable([message]);
   });

devtools/shared/resources/legacy-listeners/moz.build

@@ -5,6 +5,7 @@
 DevToolsModules(
     'console-messages.js',
     'css-changes.js',
+    'css-messages.js',
     'error-messages.js',
     'platform-messages.js',
     'root-node.js',

devtools/shared/resources/resource-watcher.js

@@ -422,6 +422,7 @@ class ResourceWatcher {
 ResourceWatcher.TYPES = ResourceWatcher.prototype.TYPES = {
   CONSOLE_MESSAGE: "console-message",
   CSS_CHANGE: "css-change",
+  CSS_MESSAGE: "css-message",
   ERROR_MESSAGE: "error-message",
   PLATFORM_MESSAGE: "platform-message",
   DOCUMENT_EVENT: "document-event",
@@ -437,6 +438,8 @@ const LegacyListeners = {
     .CONSOLE_MESSAGE]: require("devtools/shared/resources/legacy-listeners/console-messages"),
   [ResourceWatcher.TYPES
     .CSS_CHANGE]: require("devtools/shared/resources/legacy-listeners/css-changes"),
+  [ResourceWatcher.TYPES
+    .CSS_MESSAGE]: require("devtools/shared/resources/legacy-listeners/css-messages"),
   [ResourceWatcher.TYPES
     .ERROR_MESSAGE]: require("devtools/shared/resources/legacy-listeners/error-messages"),
   [ResourceWatcher.TYPES

devtools/shared/resources/tests/browser.ini

@@ -16,6 +16,7 @@ support-files =
 [browser_resources_client_caching.js]
 [browser_resources_console_messages.js]
 [browser_resources_css_changes.js]
+[browser_resources_css_messages.js]
 [browser_resources_document_events.js]
 [browser_resources_error_messages.js]
 [browser_resources_platform_messages.js]

devtools/shared/resources/tests/browser_resources_css_messages.js

@@ -0,0 +1,196 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test the ResourceWatcher API around CSS_MESSAGE
+// Reproduces the CSS message 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_css_messages.html`, (req, res) => {
+  res.setStatusLine(req.httpVersion, 200, "OK");
+  res.write(`<meta charset=utf8>
+    <style>
+      html {
+        color: bloup;
+      }
+    </style>Test CSS Messages`);
+});
+
+const TEST_URI = `http://localhost:${httpServer.identity.primaryPort}/test_css_messages.html`;
+
+add_task(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);
+
+  // Open a test tab
+  const tab = await addTab(TEST_URI);
+
+  const {
+    client,
+    resourceWatcher,
+    targetList,
+  } = await initResourceWatcherAndTarget(tab);
+
+  const receivedMessages = [];
+  const { onAvailable, onAllMessagesReceived } = setupOnAvailableFunction(
+    targetList,
+    receivedMessages
+  );
+  await resourceWatcher.watchResources([ResourceWatcher.TYPES.CSS_MESSAGE], {
+    onAvailable,
+  });
+
+  info(
+    "Now log CSS warning *after* the call to ResourceWatcher.watchResources and after " +
+      "having received the existing message"
+  );
+  // We need to wait for the first CSS Warning as it is not a cached message; when we
+  // start watching, the `cssErrorReportingEnabled` is checked on the target docShell, and
+  // if it is false, we re-parse the stylesheets to get the messages.
+  await BrowserTestUtils.waitForCondition(() => receivedMessages.length === 1);
+
+  info("Trigger a CSS Warning");
+  triggerCSSWarning(tab);
+
+  info("Waiting for all expected CSS messages to be received");
+  await onAllMessagesReceived;
+  ok(true, "All the expected CSS messages were received");
+
+  Services.console.reset();
+  targetList.stopListening();
+  await client.close();
+});
+
+add_task(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);
+
+  // Open a test tab
+  const tab = await addTab(TEST_URI);
+
+  // By default, the CSS Parser does not emit warnings at all, for performance matter.
+  // Since we actually want the Parser to emit those messages _before_ we start listening
+  // for CSS messages, we need to set the cssErrorReportingEnabled flag on the docShell.
+  await SpecialPowers.spawn(gBrowser.selectedBrowser, [], function() {
+    content.docShell.cssErrorReportingEnabled = true;
+  });
+
+  // Setting the docShell flag only indicates to the Parser that from now on, it should
+  // emit warnings. But it does not automatically emit warnings for the existing CSS
+  // errors in the stylesheets. So here we reload the tab, which will make the Parser
+  // parse the stylesheets again, this time emitting warnings.
+  const loaded = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  tab.linkedBrowser.reload();
+  // wait for the tab to be fully loaded
+  await loaded;
+  // and trigger more CSS warnings
+  await triggerCSSWarning(tab);
+
+  // At this point, all messages should be in the ConsoleService cache, and we can begin
+  // to watch and check that we do retrieve those messages.
+  const {
+    client,
+    resourceWatcher,
+    targetList,
+  } = await initResourceWatcherAndTarget(tab);
+
+  const receivedMessages = [];
+  const { onAvailable } = setupOnAvailableFunction(
+    targetList,
+    receivedMessages
+  );
+  await resourceWatcher.watchResources([ResourceWatcher.TYPES.CSS_MESSAGE], {
+    onAvailable,
+  });
+  is(receivedMessages.length, 3, "Cached messages were retrieved as expected");
+
+  Services.console.reset();
+  targetList.stopListening();
+  await client.close();
+});
+
+function setupOnAvailableFunction(targetList, receivedMessages) {
+  // The expected messages are the CSS warnings:
+  // - one for the rule in the style element
+  // - two for the JS modified style we're doing in the test.
+  const expectedMessages = [
+    {
+      pageError: {
+        errorMessage: /Expected color but found ‘bloup’/,
+        sourceName: /test_css_messages/,
+        category: "CSS Parser",
+        timeStamp: /^\d+$/,
+        error: false,
+        warning: true,
+      },
+      cssSelectors: "html",
+    },
+    {
+      pageError: {
+        errorMessage: /Error in parsing value for ‘width’/,
+        sourceName: /test_css_messages/,
+        category: "CSS Parser",
+        timeStamp: /^\d+$/,
+        error: false,
+        warning: true,
+      },
+    },
+    {
+      pageError: {
+        errorMessage: /Error in parsing value for ‘height’/,
+        sourceName: /test_css_messages/,
+        category: "CSS Parser",
+        timeStamp: /^\d+$/,
+        error: false,
+        warning: true,
+      },
+    },
+  ];
+
+  let done;
+  const onAllMessagesReceived = new Promise(resolve => (done = resolve));
+  const onAvailable = ({ resourceType, targetFront, resource }) => {
+    const { pageError } = resource;
+
+    is(
+      resource.targetFront,
+      targetList.targetFront,
+      "The targetFront property is the expected one"
+    );
+
+    if (!pageError.sourceName.includes("test_css_messages")) {
+      info(`Ignore error from unknown source: "${pageError.sourceName}"`);
+      return;
+    }
+
+    const index = receivedMessages.length;
+    receivedMessages.push(pageError);
+
+    info(`checking received css message #${index}: ${pageError.errorMessage}`);
+    ok(pageError, "The resource has a pageError attribute");
+    checkObject(resource, expectedMessages[index]);
+
+    if (receivedMessages.length == expectedMessages.length) {
+      done();
+    }
+  };
+  return { onAvailable, onAllMessagesReceived };
+}
+
+/**
+ * Sets invalid values for width and height on the document's body style attribute.
+ */
+function triggerCSSWarning(tab) {
+  return ContentTask.spawn(tab.linkedBrowser, null, function frameScript() {
+    content.document.body.style.width = "red";
+    content.document.body.style.height = "blue";
+  });
+}