Bug 1224558 - change style editor to notice stylesheet additions; r?gl draft
authorTom Tromey <tom@tromey.com>
Tue, 15 Aug 2017 12:46:55 -0600
changeset 649141 db4e4373ae2af37b621d2629c5266a7ea5a1fd5b
parent 648479 719c23d51febb3f8a8726df09961feb283fd0eed
child 649142 e49557e8ecdd60ced4823a4779bddcba516bdce2
child 649219 43d82aec403581eddc4056817ca501eb615a5003
push id74964
push userbmo:ttromey@mozilla.com
push dateFri, 18 Aug 2017 17:56:41 +0000
reviewersgl
bugs1224558
milestone57.0a1
Bug 1224558 - change style editor to notice stylesheet additions; r?gl This changes the stylesheets actor to use the tab actor's "windows" getter to get the list of new windows. It also changes the actor to emit events, and changes the style editor to add new editors based on events. MozReview-Commit-ID: 3TkQY6XHY1I
devtools/client/styleeditor/StyleEditorUI.jsm
devtools/client/styleeditor/test/browser.ini
devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js
devtools/client/styleeditor/test/browser_styleeditor_import.js
devtools/server/actors/stylesheets.js
devtools/shared/specs/stylesheets.js
--- a/devtools/client/styleeditor/StyleEditorUI.jsm
+++ b/devtools/client/styleeditor/StyleEditorUI.jsm
@@ -66,31 +66,38 @@ function StyleEditorUI(debuggee, target,
   this._panelDoc = panelDoc;
   this._cssProperties = cssProperties;
   this._window = this._panelDoc.defaultView;
   this._root = this._panelDoc.getElementById("style-editor-chrome");
 
   this.editors = [];
   this.selectedEditor = null;
   this.savedLocations = {};
+  this._seenSheets = new Map();
+
+  // Don't add any style sheets that might arrive via events, until
+  // the call to initialize.
+  this._suppressAdd = true;
 
   this._onOptionsPopupShowing = this._onOptionsPopupShowing.bind(this);
   this._onOptionsPopupHiding = this._onOptionsPopupHiding.bind(this);
-  this._onStyleSheetCreated = this._onStyleSheetCreated.bind(this);
   this._onNewDocument = this._onNewDocument.bind(this);
   this._onMediaPrefChanged = this._onMediaPrefChanged.bind(this);
   this._updateMediaList = this._updateMediaList.bind(this);
   this._clear = this._clear.bind(this);
   this._onError = this._onError.bind(this);
   this._updateOpenLinkItem = this._updateOpenLinkItem.bind(this);
   this._openLinkNewTab = this._openLinkNewTab.bind(this);
 
   this._prefObserver = new PrefObserver("devtools.styleeditor.");
   this._prefObserver.on(PREF_ORIG_SOURCES, this._onNewDocument);
   this._prefObserver.on(PREF_MEDIA_SIDEBAR, this._onMediaPrefChanged);
+
+  this._addStyleSheet = this._addStyleSheet.bind(this);
+  this._debuggee.on("stylesheet-added", this._addStyleSheet);
 }
 this.StyleEditorUI = StyleEditorUI;
 
 StyleEditorUI.prototype = {
   /**
    * Get whether any of the editors have unsaved changes.
    *
    * @return boolean
@@ -159,17 +166,17 @@ StyleEditorUI.prototype = {
    * Build the initial UI and wire buttons with event handlers.
    */
   createUI: function () {
     let viewRoot = this._root.parentNode.querySelector(".splitview-root");
 
     this._view = new SplitView(viewRoot);
 
     wire(this._view.rootElement, ".style-editor-newButton", () =>{
-      this._debuggee.addStyleSheet(null).then(this._onStyleSheetCreated);
+      this._debuggee.addStyleSheet(null);
     });
 
     wire(this._view.rootElement, ".style-editor-importButton", ()=> {
       this._importFromFile(this._mockImportFile || null, this._window);
     });
 
     this._optionsButton = this._panelDoc.getElementById("style-editor-options");
     this._panelDoc.addEventListener("contextmenu", () => {
@@ -227,29 +234,31 @@ StyleEditorUI.prototype = {
    * Refresh editors to reflect the stylesheets in the document.
    *
    * @param {string} event
    *        Event name
    * @param {StyleSheet} styleSheet
    *        StyleSheet object for new sheet
    */
   _onNewDocument: function () {
+    this._suppressAdd = true;
     this._debuggee.getStyleSheets().then((styleSheets) => {
       return this._resetStyleSheetList(styleSheets);
     }).catch(e => console.error(e));
   },
 
   /**
    * Add editors for all the given stylesheets to the UI.
    *
    * @param  {array} styleSheets
    *         Array of StyleSheetFront
    */
   _resetStyleSheetList: Task.async(function* (styleSheets) {
     this._clear();
+    this._suppressAdd = false;
 
     for (let sheet of styleSheets) {
       try {
         yield this._addStyleSheet(sheet);
       } catch (e) {
         this.emit("error", { key: LOAD_ERROR });
       }
     }
@@ -282,63 +291,83 @@ StyleEditorUI.prototype = {
         this.savedLocations[identifier] = editor.savedFile;
       }
     }
 
     this._clearStyleSheetEditors();
     this._view.removeAll();
 
     this.selectedEditor = null;
+    this._seenSheets = new Map();
+    this._suppressAdd = true;
 
     this._root.classList.add("loading");
   },
 
   /**
    * Add an editor for this stylesheet. Add editors for its original sources
    * instead (e.g. Sass sources), if applicable.
    *
    * @param  {StyleSheetFront} styleSheet
    *         Style sheet to add to style editor
+   * @param {Boolean} isNew
+   *        True if this style sheet was created by a call to the
+   *        style sheets actor's @see addStyleSheet method.
+   * @return {Promise}
+   *         A promise that resolves to the style sheet's editor when the style sheet has
+   *         been fully loaded.  If the style sheet has a source map, and source mapping
+   *         is enabled, then the promise resolves to null.
    */
-  _addStyleSheet: Task.async(function* (styleSheet) {
-    let editor = yield this._addStyleSheetEditor(styleSheet);
+  _addStyleSheet: function (styleSheet, isNew) {
+    if (this._suppressAdd) {
+      return null;
+    }
+    if (!this._seenSheets.has(styleSheet)) {
+      let promise = (async () => {
+        let editor = await this._addStyleSheetEditor(styleSheet, isNew);
 
-    if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
-      return;
-    }
+        if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
+          return editor;
+        }
 
-    let sources = yield styleSheet.getOriginalSources();
-    if (sources && sources.length) {
-      let parentEditorName = editor.friendlyName;
-      this._removeStyleSheetEditor(editor);
+        let sources = await styleSheet.getOriginalSources();
+        if (sources && sources.length) {
+          let parentEditorName = editor.friendlyName;
+          this._removeStyleSheetEditor(editor);
+          editor = null;
 
-      for (let source of sources) {
-        // set so the first sheet will be selected, even if it's a source
-        source.styleSheetIndex = styleSheet.styleSheetIndex;
-        source.relatedStyleSheet = styleSheet;
-        source.relatedEditorName = parentEditorName;
-        yield this._addStyleSheetEditor(source);
-      }
+          for (let source of sources) {
+            // set so the first sheet will be selected, even if it's a source
+            source.styleSheetIndex = styleSheet.styleSheetIndex;
+            source.relatedStyleSheet = styleSheet;
+            source.relatedEditorName = parentEditorName;
+            await this._addStyleSheetEditor(source);
+          }
+        }
+
+        return editor;
+      })();
+      this._seenSheets.set(styleSheet, promise);
     }
-  }),
+    return this._seenSheets.get(styleSheet);
+  },
 
   /**
    * Add a new editor to the UI for a source.
    *
    * @param {StyleSheet}  styleSheet
    *        Object representing stylesheet
-   * @param {nsIfile}  file
-   *         Optional file object that sheet was imported from
    * @param {Boolean} isNew
    *         Optional if stylesheet is a new sheet created by user
    * @return {Promise} that is resolved with the created StyleSheetEditor when
    *                   the editor is fully initialized or rejected on error.
    */
-  _addStyleSheetEditor: Task.async(function* (styleSheet, file, isNew) {
+  _addStyleSheetEditor: Task.async(function* (styleSheet, isNew) {
     // recall location of saved file for this sheet after page reload
+    let file = null;
     let identifier = this.getStyleSheetIdentifier(styleSheet);
     let savedFile = this.savedLocations[identifier];
     if (savedFile && !file) {
       file = savedFile;
     }
 
     let editor = new StyleSheetEditor(styleSheet, this._window, file, isNew,
                                       this._walker, this._highlighter);
@@ -382,34 +411,34 @@ StyleEditorUI.prototype = {
         if (!Components.isSuccessCode(status)) {
           this.emit("error", { key: LOAD_ERROR });
           return;
         }
         let source =
             NetUtil.readInputStreamToString(stream, stream.available());
         stream.close();
 
+        this._suppressAdd = true;
         this._debuggee.addStyleSheet(source).then((styleSheet) => {
-          this._onStyleSheetCreated(styleSheet, selectedFile);
+          this._suppressAdd = false;
+          this._addStyleSheet(styleSheet, true).then(editor => {
+            if (editor) {
+              editor.savedFile = selectedFile;
+            }
+            // Just for testing purposes.
+            this.emit("test:editor-updated", editor);
+          });
         });
       });
     };
 
     showFilePicker(file, false, parentWindow, onFileSelected);
   },
 
   /**
-   * When a new or imported stylesheet has been added to the document.
-   * Add an editor for it.
-   */
-  _onStyleSheetCreated: function (styleSheet, file) {
-    this._addStyleSheetEditor(styleSheet, file, true);
-  },
-
-  /**
    * Forward any error from a stylesheet.
    *
    * @param  {string} event
    *         Event name
    * @param  {data} data
    *         The event data
    */
   _onError: function (event, data) {
@@ -1017,10 +1046,12 @@ StyleEditorUI.prototype = {
     this._optionsMenu.removeEventListener("popupshowing",
                                           this._onOptionsPopupShowing);
     this._optionsMenu.removeEventListener("popuphiding",
                                           this._onOptionsPopupHiding);
 
     this._prefObserver.off(PREF_ORIG_SOURCES, this._onNewDocument);
     this._prefObserver.off(PREF_MEDIA_SIDEBAR, this._onMediaPrefChanged);
     this._prefObserver.destroy();
+
+    this._debuggee.off("stylesheet-added", this._addStyleSheet);
   }
 };
--- a/devtools/client/styleeditor/test/browser.ini
+++ b/devtools/client/styleeditor/test/browser.ini
@@ -56,16 +56,17 @@ support-files =
   !/devtools/client/framework/test/shared-head.js
   !/devtools/client/inspector/shared/test/head.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/responsive.html/test/browser/devices.json
   !/devtools/client/shared/test/test-actor-registry.js
   !/devtools/client/shared/test/test-actor.js
 
+[browser_styleeditor_add_stylesheet.js]
 [browser_styleeditor_autocomplete.js]
 [browser_styleeditor_autocomplete-disabled.js]
 [browser_styleeditor_bom.js]
 [browser_styleeditor_bug_740541_iframes.js]
 [browser_styleeditor_bug_851132_middle_click.js]
 [browser_styleeditor_bug_870339.js]
 [browser_styleeditor_cmd_edit.js]
 [browser_styleeditor_enabled.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js
@@ -0,0 +1,37 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+// Test that a newly-added style sheet shows up in the style editor.
+
+const TESTCASE_URI = TEST_BASE_HTTPS + "simple.html";
+
+add_task(function* () {
+  let { ui } = yield openStyleEditorForURL(TESTCASE_URI);
+
+  is(ui.editors.length, 2, "Two sheets present after load.");
+
+  // We have to wait for the length to change, because we might still
+  // be seeing events from the initial open.
+  let added = new Promise(resolve => {
+    let handler = () => {
+      if (ui.editors.length === 3) {
+        resolve();
+        ui.off("editor-added", handler);
+      }
+    };
+    ui.on("editor-added", handler);
+  });
+
+  info("Adding a style sheet");
+  yield ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    let document = content.document;
+    const style = document.createElement("style");
+    style.appendChild(document.createTextNode("div { background: #f06; }"));
+    document.head.appendChild(style);
+  });
+  yield added;
+
+  is(ui.editors.length, 3, "Three sheets present after new style sheet");
+});
--- a/devtools/client/styleeditor/test/browser_styleeditor_import.js
+++ b/devtools/client/styleeditor/test/browser_styleeditor_import.js
@@ -13,17 +13,17 @@ Components.utils.import("resource://gre/
 var FileUtils = tempScope.FileUtils;
 
 const FILENAME = "styleeditor-import-test.css";
 const SOURCE = "body{background:red;}";
 
 add_task(function* () {
   let { panel, ui } = yield openStyleEditorForURL(TESTCASE_URI);
 
-  let added = ui.once("editor-added");
+  let added = ui.once("test:editor-updated");
   importSheet(ui, panel.panelWindow);
 
   info("Waiting for editor to be added for the imported sheet.");
   let editor = yield added;
 
   is(editor.savedFile.leafName, FILENAME,
      "imported stylesheet will be saved directly into the same file");
   is(editor.friendlyName, FILENAME,
--- a/devtools/server/actors/stylesheets.js
+++ b/devtools/server/actors/stylesheets.js
@@ -244,17 +244,17 @@ var StyleSheetActor = protocol.ActorClas
           break;
         }
       }
     }
     return this._styleSheetIndex;
   },
 
   destroy: function () {
-    if (this._transitionTimeout) {
+    if (this._transitionTimeout && this.window) {
       this.window.clearTimeout(this._transitionTimeout);
       removePseudoClassLock(
                    this.document.documentElement, TRANSITION_PSEUDO_CLASS);
     }
   },
 
   initialize: function (styleSheet, parentActor, window) {
     protocol.Actor.prototype.initialize.call(this, null);
@@ -792,97 +792,137 @@ var StyleSheetsActor = protocol.ActorCla
   form: function () {
     return { actor: this.actorID };
   },
 
   initialize: function (conn, tabActor) {
     protocol.Actor.prototype.initialize.call(this, null);
 
     this.parentActor = tabActor;
+
+    this._onSheetAdded = this._onSheetAdded.bind(this);
+    this._onWindowReady = this._onWindowReady.bind(this);
+    this._onNewStyleSheetActor = this._onNewStyleSheetActor.bind(this);
+
+    events.on(this.parentActor, "stylesheet-added", this._onNewStyleSheetActor);
+    events.on(this.parentActor, "window-ready", this._onWindowReady);
+
+    // We listen for StyleSheetApplicableStateChanged rather than
+    // StyleSheetAdded, because the latter will be sent before the
+    // rules are ready.  Using the former (with a check to ensure that
+    // the sheet is enabled) ensures that the sheet is ready before we
+    // try to make an actor for it.
+    this.parentActor.chromeEventHandler
+      .addEventListener("StyleSheetApplicableStateChanged", this._onSheetAdded, true);
+
+    this._nextStyleSheetIsNew = false;
+  },
+
+  destroy: function () {
+    for (let win of this.parentActor.windows) {
+      win.document.styleSheetChangeEventsEnabled = false;
+    }
+
+    events.off(this.parentActor, "stylesheet-added", this._onNewStyleSheetActor);
+    events.off(this.parentActor, "window-ready", this._onWindowReady);
+
+    this.parentActor.chromeEventHandler.removeEventListener("StyleSheetAdded",
+                                                            this._onSheetAdded, true);
+
+    protocol.Actor.prototype.destroy.call(this);
+  },
+
+  /**
+   * Event handler that is called when a the tab actor emits window-ready.
+   * @param {Event} evt
+   *        The triggering event.
+   */
+  _onWindowReady: function (evt) {
+    this._addStyleSheets(evt.window);
+  },
+
+  /**
+   * Event handler that is called when a the tab actor emits stylesheet-added.
+   * @param {Actor} actor
+   *        The new actor.
+   */
+  _onNewStyleSheetActor: function (actor) {
+    // Forward it to the client side.
+    events.emit(this, "stylesheet-added", actor, this._nextStyleSheetIsNew);
+    this._nextStyleSheetIsNew = false;
   },
 
   /**
    * Protocol method for getting a list of StyleSheetActors representing
    * all the style sheets in this document.
    */
   getStyleSheets: Task.async(function* () {
-    // Iframe document can change during load (bug 1171919). Track their windows
-    // instead.
-    let windows = [this.window];
     let actors = [];
 
-    for (let win of windows) {
+    for (let win of this.parentActor.windows) {
       let sheets = yield this._addStyleSheets(win);
       actors = actors.concat(sheets);
-
-      // Recursively handle style sheets of the documents in iframes.
-      for (let iframe of win.document.querySelectorAll("iframe, browser, frame")) {
-        if (iframe.contentDocument && iframe.contentWindow) {
-          // Sometimes, iframes don't have any document, like the
-          // one that are over deeply nested (bug 285395)
-          windows.push(iframe.contentWindow);
-        }
-      }
     }
     return actors;
   }),
 
   /**
    * Check if we should be showing this stylesheet.
    *
-   * @param {Document} doc
-   *        Document for which we're checking
    * @param {DOMCSSStyleSheet} sheet
    *        Stylesheet we're interested in
    *
    * @return boolean
    *         Whether the stylesheet should be listed.
    */
-  _shouldListSheet: function (doc, sheet) {
+  _shouldListSheet: function (sheet) {
     // Special case about:PreferenceStyleSheet, as it is generated on the
     // fly and the URI is not registered with the about: handler.
     // https://bugzilla.mozilla.org/show_bug.cgi?id=935803#c37
     if (sheet.href && sheet.href.toLowerCase() == "about:preferencestylesheet") {
       return false;
     }
 
     return true;
   },
 
   /**
+   * Event handler that is called when a new style sheet is added to
+   * a document.
+   * @param {Event} evt
+   *        The triggering event.
+   */
+  _onSheetAdded: function (evt) {
+    let sheet = evt.stylesheet;
+    if (this._shouldListSheet(sheet) && !this._haveAncestorWithSameURL(sheet)) {
+      this.parentActor.createStyleSheetActor(sheet);
+    }
+  },
+
+  /**
    * Add all the stylesheets for the document in this window to the map and
    * create an actor for each one if not already created.
    *
    * @param {Window} win
    *        Window for which to add stylesheets
    *
    * @return {Promise}
    *         Promise that resolves to an array of StyleSheetActors
    */
   _addStyleSheets: function (win) {
     return Task.spawn(function* () {
       let doc = win.document;
-      // readyState can be uninitialized if an iframe has just been created but
-      // it has not started to load yet.
-      if (doc.readyState === "loading" || doc.readyState === "uninitialized") {
-        // Wait for the document to load first.
-        yield listenOnce(win, "DOMContentLoaded", true);
-
-        // Make sure we have the actual document for this window. If the
-        // readyState was initially uninitialized, the initial dummy document
-        // was replaced with the actual document (bug 1171919).
-        doc = win.document;
-      }
+      doc.styleSheetChangeEventsEnabled = true;
 
       let isChrome = Services.scriptSecurityManager.isSystemPrincipal(doc.nodePrincipal);
       let styleSheets = isChrome ? DOMUtils.getAllStyleSheets(doc) : doc.styleSheets;
       let actors = [];
       for (let i = 0; i < styleSheets.length; i++) {
         let sheet = styleSheets[i];
-        if (!this._shouldListSheet(doc, sheet)) {
+        if (!this._shouldListSheet(sheet)) {
           continue;
         }
 
         let actor = this.parentActor.createStyleSheetActor(sheet);
         actors.push(actor);
 
         // Get all sheets, including imported ones
         let imports = yield this._getImported(doc, actor);
@@ -912,17 +952,17 @@ var StyleSheetsActor = protocol.ActorCla
         if (rule.type == Ci.nsIDOMCSSRule.IMPORT_RULE) {
           // With the Gecko style system, the associated styleSheet may be null
           // if it has already been seen because an import cycle for the same
           // URL.  With Stylo, the styleSheet will exist (which is correct per
           // the latest CSSOM spec), so we also need to check ancestors for the
           // same URL to avoid cycles.
           let sheet = rule.styleSheet;
           if (!sheet || this._haveAncestorWithSameURL(sheet) ||
-              !this._shouldListSheet(doc, sheet)) {
+              !this._shouldListSheet(sheet)) {
             continue;
           }
           let actor = this.parentActor.createStyleSheetActor(rule.styleSheet);
           imported.push(actor);
 
           // recurse imports in this stylesheet as well
           let children = yield this._getImported(doc, actor);
           imported = imported.concat(children);
@@ -958,16 +998,23 @@ var StyleSheetsActor = protocol.ActorCla
    * Return an actor for it.
    *
    * @param  {object} request
    *         Debugging protocol request object, with 'text property'
    * @return {object}
    *         Object with 'styelSheet' property for form on new actor.
    */
   addStyleSheet: function (text) {
+    // This is a bit convoluted.  The style sheet actor may be created
+    // by a notification from platform.  In this case, we can't easily
+    // pass the "new" flag through to createStyleSheetActor, so we set
+    // a flag locally and check it before sending an event to the
+    // client.  See |_onNewStyleSheetActor|.
+    this._nextStyleSheetIsNew = true;
+
     let parent = this.document.documentElement;
     let style = this.document.createElementNS("http://www.w3.org/1999/xhtml", "style");
     style.setAttribute("type", "text/css");
 
     if (text) {
       style.appendChild(this.document.createTextNode(text));
     }
     parent.appendChild(style);
--- a/devtools/shared/specs/stylesheets.js
+++ b/devtools/shared/specs/stylesheets.js
@@ -100,16 +100,24 @@ const styleSheetSpec = generateActorSpec
   }
 });
 
 exports.styleSheetSpec = styleSheetSpec;
 
 const styleSheetsSpec = generateActorSpec({
   typeName: "stylesheets",
 
+  events: {
+    "stylesheet-added": {
+      type: "stylesheetAdded",
+      sheet: Arg(0, "stylesheet"),
+      isNew: Arg(1, "boolean")
+    },
+  },
+
   methods: {
     getStyleSheets: {
       request: {},
       response: { styleSheets: RetVal("array:stylesheet") }
     },
     addStyleSheet: {
       request: { text: Arg(0, "string") },
       response: { styleSheet: RetVal("stylesheet") }