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 --HG-- extra : rebase_source : 76098b08c95c7d4e7ee6bb2bd9ebae97a0c4b7ab
2017-08-15 12:46:55 -06:00 · 2017-08-15 12:46:55 -06:00 · ad5ebc78b2
--- a/devtools/client/styleeditor/StyleEditorUI.jsm
+++ b/devtools/client/styleeditor/StyleEditorUI.jsm
@ -71,10 +71,18 @@ function StyleEditorUI(debuggee, target, panelDoc, cssProperties) {
  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.  Style sheets can arrive from the server
+  // at any time, for example if a new style sheet was added, or if
+  // the style sheet actor was just created and is walking the style
+  // sheets for the first time.  In any case, in |initialize| we're
+  // going to fetch the list of sheets anyway.
+  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);
@ -82,10 +90,13 @@ function StyleEditorUI(debuggee, target, panelDoc, cssProperties) {
  this._onError = this._onError.bind(this);
  this._updateOpenLinkItem = this._updateOpenLinkItem.bind(this);
  this._openLinkNewTab = this._openLinkNewTab.bind(this);
+  this._addStyleSheet = this._addStyleSheet.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._debuggee.on("stylesheet-added", this._addStyleSheet);
 }
 this.StyleEditorUI = StyleEditorUI;

@ -164,7 +175,7 @@ StyleEditorUI.prototype = {
    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", ()=> {
@ -232,6 +243,7 @@ StyleEditorUI.prototype = {
   *        StyleSheet object for new sheet
   */
  _onNewDocument: function () {
+    this._suppressAdd = true;
    this._debuggee.getStyleSheets().then((styleSheets) => {
      return this._resetStyleSheetList(styleSheets);
    }).catch(console.error);
@ -245,6 +257,7 @@ StyleEditorUI.prototype = {
   */
  _resetStyleSheetList: Task.async(function* (styleSheets) {
    this._clear();
+    this._suppressAdd = false;

    for (let sheet of styleSheets) {
      try {
@ -287,6 +300,10 @@ StyleEditorUI.prototype = {
    this._view.removeAll();

    this.selectedEditor = null;
+    // Here the keys are style sheet actors, and the values are
+    // promises that resolve to the sheet's editor.  See |_addStyleSheet|.
+    this._seenSheets = new Map();
+    this._suppressAdd = true;

    this._root.classList.add("loading");
  },
@ -297,46 +314,67 @@ StyleEditorUI.prototype = {
   *
   * @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);
-
-    if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
-      return;
+  _addStyleSheet: function (styleSheet, isNew) {
+    if (this._suppressAdd) {
+      return null;
    }

-    let sources = yield styleSheet.getOriginalSources();
-    if (sources && sources.length) {
-      let parentEditorName = editor.friendlyName;
-      this._removeStyleSheetEditor(editor);
+    if (!this._seenSheets.has(styleSheet)) {
+      let promise = (async () => {
+        let editor = await this._addStyleSheetEditor(styleSheet, isNew);

-      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);
-      }
+        if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
+          return editor;
+        }
+
+        let sources = await styleSheet.getOriginalSources();
+        // A single generated sheet might map to multiple original
+        // sheets, so make editors for each of them.
+        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;
+            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) {
+    if (savedFile) {
      file = savedFile;
    }

@ -387,8 +425,16 @@ StyleEditorUI.prototype = {
            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);
+          });
        });
      });
    };
@ -396,14 +442,6 @@ StyleEditorUI.prototype = {
    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.
   *
@ -1010,6 +1048,9 @@ StyleEditorUI.prototype = {

    this._clearStyleSheetEditors();

+    this._seenSheets = null;
+    this._suppressAdd = false;
+
    let sidebar = this._panelDoc.querySelector(".splitview-controller");
    let sidebarWidth = sidebar.getAttribute("width");
    Services.prefs.setIntPref(PREF_NAV_WIDTH, sidebarWidth);
@ -1022,5 +1063,7 @@ StyleEditorUI.prototype = {
    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
@ -61,6 +61,7 @@ support-files =
  !/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]
--- a/devtools/client/styleeditor/test/browser_styleeditor_add_stylesheet.js
+++ 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) {
+        ui.off("editor-added", handler);
+        resolve();
+      }
+    };
+    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
@ -18,7 +18,7 @@ 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.");
--- a/devtools/server/actors/stylesheets.js
+++ b/devtools/server/actors/stylesheets.js
@ -13,7 +13,6 @@ const {Task} = require("devtools/shared/task");
 const protocol = require("devtools/shared/protocol");
 const {LongStringActor} = require("devtools/server/actors/string");
 const {fetch} = require("devtools/shared/DevToolsUtils");
-const {listenOnce} = require("devtools/shared/async-utils");
 const {originalSourceSpec, mediaRuleSpec, styleSheetSpec,
       styleSheetsSpec} = require("devtools/shared/specs/stylesheets");
 const {SourceMapConsumer} = require("source-map");
@ -248,7 +247,7 @@ var StyleSheetActor = protocol.ActorClassWithSpec(styleSheetSpec, {
  },

  destroy: function () {
-    if (this._transitionTimeout) {
+    if (this._transitionTimeout && this.window) {
      this.window.clearTimeout(this._transitionTimeout);
      removePseudoClassLock(
                   this.document.documentElement, TRANSITION_PSEUDO_CLASS);
@ -796,6 +795,64 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
    protocol.Actor.prototype.initialize.call(this, null);

    this.parentActor = tabActor;
+
+    this._onNewStyleSheetActor = this._onNewStyleSheetActor.bind(this);
+    this._onSheetAdded = this._onSheetAdded.bind(this);
+    this._onWindowReady = this._onWindowReady.bind(this);
+
+    this.parentActor.on("stylesheet-added", this._onNewStyleSheetActor);
+    this.parentActor.on("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 is used when creating a new style sheet, so that we can
+    // pass the correct flag when emitting our stylesheet-added event.
+    // See addStyleSheet and _onNewStyleSheetActor for more details.
+    this._nextStyleSheetIsNew = false;
+  },
+
+  destroy: function () {
+    for (let win of this.parentActor.windows) {
+      // This flag only exists for devtools, so we are free to clear
+      // it when we're done.
+      win.document.styleSheetChangeEventsEnabled = false;
+    }
+
+    this.parentActor.off("stylesheet-added", this._onNewStyleSheetActor);
+    this.parentActor.off("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 {StyleSheetActor} actor
+   *        The new style sheet actor.
+   */
+  _onNewStyleSheetActor: function (actor) {
+    // Forward it to the client side.
+    this.emit("stylesheet-added", actor, this._nextStyleSheetIsNew);
+    this._nextStyleSheetIsNew = false;
  },

  /**
@ -803,23 +860,11 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
   * 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;
  }),
@ -827,15 +872,13 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
  /**
   * 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
@ -846,6 +889,22 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
    return true;
  },

+  /**
+   * Event handler that is called when a new style sheet is added to
+   * a document.  In particular,  StyleSheetApplicableStateChanged is
+   * listened for, because StyleSheetAdded is sent too early, before
+   * the rules are ready.
+   *
+   * @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.
@ -859,24 +918,16 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
  _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;
-      }
+      // We have to set this flag in order to get the
+      // StyleSheetApplicableStateChanged events.  See Document.webidl.
+      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;
        }

@ -916,7 +967,7 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
          // 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);
@ -962,6 +1013,13 @@ var StyleSheetsActor = protocol.ActorClassWithSpec(styleSheetsSpec, {
   *         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");
--- a/devtools/shared/specs/stylesheets.js
+++ b/devtools/shared/specs/stylesheets.js
@ -105,6 +105,14 @@ exports.styleSheetSpec = styleSheetSpec;
 const styleSheetsSpec = generateActorSpec({
  typeName: "stylesheets",

+  events: {
+    "stylesheet-added": {
+      type: "stylesheetAdded",
+      sheet: Arg(0, "stylesheet"),
+      isNew: Arg(1, "boolean")
+    },
+  },
+
  methods: {
    getStyleSheets: {
      request: {},