Bug 1769771 - Improve DevTools support for constructable stylesheets. r=nchevobbe, a=RyanVM
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 19 May 2022 22:32:28 +0000
changeset 686410 3b52c1e7b46f8171efb84395fba7a0cdb15782a5
parent 686409 330c54f6b54781f1d2f62ce7a535a9bff4fb237e
child 686411 d670faee05c336ecd3dd8cc4af5bfe04ee6544dd
push id16684
push userryanvm@gmail.com
push dateFri, 20 May 2022 20:18:23 +0000
treeherdermozilla-beta@94c0d44227f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe, RyanVM
bugs1769771
milestone101.0
Bug 1769771 - Improve DevTools support for constructable stylesheets. r=nchevobbe, a=RyanVM These should be trivial fixes for the most part, just fixing assumptions for constructable stylesheets which have no owner node. Since Gecko internally also has the concept of "associated document", let's just use that. We don't have the authored text around for these. We could keep it around but I'd kinda prefer we didn't have to just for devtools, since it seems to me we already need to deal with rules not being there (imagine e.g. an empty style element with all the rules inserted via script with insertRule()). So returning the empty string should be reasonable for now... Writing some tests now... Differential Revision: https://phabricator.services.mozilla.com/D146682
devtools/server/actors/inspector/css-logic.js
devtools/server/actors/page-style.js
devtools/server/actors/style-rule.js
devtools/server/actors/utils/stylesheets-manager.js
dom/webidl/StyleSheet.webidl
layout/style/StyleSheet.cpp
--- a/devtools/server/actors/inspector/css-logic.js
+++ b/devtools/server/actors/inspector/css-logic.js
@@ -315,18 +315,18 @@ CssLogic.prototype = {
    *
    * @return {CssSheet} the CssSheet object for the given CSSStyleSheet object.
    */
   getSheet: function(domSheet, index) {
     let cacheId = "";
 
     if (domSheet.href) {
       cacheId = domSheet.href;
-    } else if (domSheet.ownerNode?.ownerDocument) {
-      cacheId = domSheet.ownerNode.ownerDocument.location;
+    } else if (domSheet.associatedDocument) {
+      cacheId = domSheet.associatedDocument.location;
     }
 
     let sheet = null;
     let sheetFound = false;
 
     if (cacheId in this._sheets) {
       for (sheet of this._sheets[cacheId]) {
         if (sheet.domSheet === domSheet) {
@@ -718,22 +718,17 @@ CssLogic.getComputedStyle = function(nod
  * Get a source for a stylesheet, taking into account embedded stylesheets
  * for which we need to use document.defaultView.location.href rather than
  * sheet.href
  *
  * @param {CSSStyleSheet} sheet the DOM object for the style sheet.
  * @return {string} the address of the stylesheet.
  */
 CssLogic.href = function(sheet) {
-  let href = sheet.href;
-  if (!href) {
-    href = sheet.ownerNode.ownerDocument.location;
-  }
-
-  return href;
+  return sheet.href || sheet.associatedDocument.location;
 };
 
 /**
  * Returns true if the given node has visited state.
  */
 CssLogic.hasVisitedState = hasVisitedState;
 
 /**
--- a/devtools/server/actors/page-style.js
+++ b/devtools/server/actors/page-style.js
@@ -576,18 +576,22 @@ var PageStyleActor = protocol.ActorClass
     this.cssLogic.highlight(node.rawNode);
     let entries = [];
     entries = entries.concat(
       this._getAllElementRules(node, undefined, options)
     );
 
     const result = this.getAppliedProps(node, entries, options);
     for (const rule of result.rules) {
-      // See the comment in |form| to understand this.
-      await rule.getAuthoredCssText();
+      try {
+        // See the comment in |StyleRuleActor.form| to understand this.
+        // This can throw if the authored rule text is not found (so e.g., with
+        // CSSOM or constructable stylesheets).
+        await rule.getAuthoredCssText();
+      } catch (ex) {}
     }
 
     // Reference to instances of StyleRuleActor for CSS rules matching the node.
     // Assume these are used by a consumer which wants to be notified when their
     // state or declarations change either directly or indirectly.
     this._observedRules = result.rules;
 
     return result;
--- a/devtools/server/actors/style-rule.js
+++ b/devtools/server/actors/style-rule.js
@@ -282,26 +282,22 @@ const StyleRuleActor = protocol.ActorCla
         };
       }
     }
 
     return data;
   },
 
   getDocument: function(sheet) {
-    if (sheet.ownerNode) {
-      return sheet.ownerNode.nodeType == sheet.ownerNode.DOCUMENT_NODE
-        ? sheet.ownerNode
-        : sheet.ownerNode.ownerDocument;
-    } else if (sheet.parentStyleSheet) {
-      return this.getDocument(sheet.parentStyleSheet);
+    if (!sheet.associatedDocument) {
+      throw new Error(
+        "Failed trying to get the document of an invalid stylesheet"
+      );
     }
-    throw new Error(
-      "Failed trying to get the document of an invalid stylesheet"
-    );
+    return sheet.associatedDocument;
   },
 
   toString: function() {
     return "[StyleRuleActor for " + this.rawRule + "]";
   },
 
   // eslint-disable-next-line complexity
   form: function() {
--- a/devtools/server/actors/utils/stylesheets-manager.js
+++ b/devtools/server/actors/utils/stylesheets-manager.js
@@ -272,18 +272,23 @@ class StyleSheetsManager extends EventEm
 
     // modifiedText is the content of the stylesheet updated by update function.
     // In case not updating, this is undefined.
     if (modifiedText !== undefined) {
       return modifiedText;
     }
 
     if (!styleSheet.href) {
-      // this is an inline <style> sheet
-      return styleSheet.ownerNode.textContent;
+      if (styleSheet.ownerNode) {
+        // this is an inline <style> sheet
+        return styleSheet.ownerNode.textContent;
+      }
+      // Constructed stylesheet.
+      // TODO(bug 176993): Maybe preserve authored text?
+      return "";
     }
 
     return this._fetchStyleSheet(styleSheet);
   }
 
   /**
    * Toggle the disabled property of the stylesheet
    *
@@ -370,18 +375,18 @@ class StyleSheetsManager extends EventEm
    * @param {Number} kind
    *        Either UPDATE_PRESERVING_RULES or UPDATE_GENERAL
    * @param {String} cause
    *         Indicates the cause of this update (e.g. "styleeditor") if this was called
    *         from the stylesheet to be edited by the user from the StyleEditor.
    */
   _startTransition(resourceId, kind, cause) {
     const styleSheet = this._styleSheetMap.get(resourceId);
-    const document = styleSheet.ownerNode.ownerDocument;
-    const window = styleSheet.ownerNode.ownerGlobal;
+    const document = styleSheet.associatedDocument;
+    const window = document.ownerGlobal;
 
     if (!this._transitionSheetLoaded) {
       this._transitionSheetLoaded = true;
       // We don't remove this sheet. It uses an internal selector that
       // we only apply via locks, so there's no need to load and unload
       // it all the time.
       loadSheet(window, TRANSITION_SHEET);
     }
@@ -403,17 +408,17 @@ class StyleSheetsManager extends EventEm
    * @param {Number} kind
    *        Either UPDATE_PRESERVING_RULES or UPDATE_GENERAL
    * @param {String} cause
    *         Indicates the cause of this update (e.g. "styleeditor") if this was called
    *         from the stylesheet to be edited by the user from the StyleEditor.
    */
   _onTransitionEnd(resourceId, kind, cause) {
     const styleSheet = this._styleSheetMap.get(resourceId);
-    const document = styleSheet.ownerNode.ownerDocument;
+    const document = styleSheet.associatedDocument;
 
     this._transitionTimeout = null;
     removePseudoClassLock(document.documentElement, TRANSITION_PSEUDO_CLASS);
 
     this.emit("stylesheet-updated", {
       resourceId,
       updateKind: "style-applied",
       updates: {
@@ -769,33 +774,34 @@ class StyleSheetsManager extends EventEm
   /**
    * Event handler that is called when the state of applicable of style sheet is changed.
    *
    * For now, StyleSheetApplicableStateChanged event will be called at following timings.
    * - Append <link> of stylesheet to document
    * - Append <style> to document
    * - Change disable attribute of stylesheet object
    * - Change disable attribute of <link> to false
-   * When appending <link>, <style> or changing `disable` attribute to false, `applicable`
-   * is passed as true. The other hand, when changing `disable` to true, this will be
-   * false.
+   * - Stylesheet is constructed.
+   * When appending <link>, <style> or changing `disabled` attribute to false,
+   * `applicable` is passed as true. The other hand, when changing `disabled`
+   * to true, this will be false.
    * NOTE: For now, StyleSheetApplicableStateChanged will not be called when removing the
    *       link and style element.
    *
    * @param {StyleSheetApplicableStateChanged}
    *        The triggering event.
    */
   _onApplicableStateChanged({ applicable, stylesheet: styleSheet }) {
     if (
       // Have interest in applicable stylesheet only.
       applicable &&
-      // No ownerNode means that this stylesheet is *not* associated to a DOM Element.
-      styleSheet.ownerNode &&
+      styleSheet.associatedDocument &&
       (!this._targetActor.ignoreSubFrames ||
-        styleSheet.ownerNode.ownerGlobal === this._targetActor.window) &&
+        styleSheet.associatedDocument.ownerGlobal ===
+          this._targetActor.window) &&
       this._shouldListSheet(styleSheet) &&
       !this._haveAncestorWithSameURL(styleSheet)
     ) {
       this._registerStyleSheet(styleSheet);
     }
   }
 
   /**
--- a/dom/webidl/StyleSheet.webidl
+++ b/dom/webidl/StyleSheet.webidl
@@ -41,9 +41,11 @@ interface StyleSheet {
   [ChromeOnly, Pure]
   readonly attribute DOMString sourceMapURL;
   // The source URL for this style sheet.  If the style sheet has the
   // special "# sourceURL=" comment, then this is the URL specified
   // there.  If no such comment is found, then this is the empty
   // string.
   [ChromeOnly, Pure]
   readonly attribute DOMString sourceURL;
+  [ChromeOnly, Pure]
+  readonly attribute Document? associatedDocument;
 };
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -311,19 +311,25 @@ void StyleSheet::ApplicableStateChanged(
     }
   };
 
   const StyleSheet& sheet = OutermostSheet();
   if (sheet.mDocumentOrShadowRoot) {
     Notify(*sheet.mDocumentOrShadowRoot);
   }
 
+  if (sheet.mConstructorDocument) {
+    Notify(*sheet.mConstructorDocument);
+  }
+
   for (DocumentOrShadowRoot* adopter : sheet.mAdopters) {
     MOZ_ASSERT(adopter, "adopters should never be null");
-    Notify(*adopter);
+    if (adopter != sheet.mConstructorDocument) {
+      Notify(*adopter);
+    }
   }
 }
 
 void StyleSheet::SetDisabled(bool aDisabled) {
   if (IsReadOnly()) {
     return;
   }