Bug 1171482 - throttle new-mutations events in inspector actor;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 09 Oct 2017 13:20:25 +0200
changeset 684896 2b0308a141c1f5d0a6b0e8889c0968a146274308
parent 684877 de0bf26aff7c8c5ea70efe0d11e9afa963218717
child 736991 7f8eb8899d18b7249f1ff91eefb105329998608a
push id85754
push userjdescottes@mozilla.com
push dateMon, 23 Oct 2017 18:06:59 +0000
reviewersbgrins
bugs1171482
milestone58.0a1
Bug 1171482 - throttle new-mutations events in inspector actor;r=bgrins MozReview-Commit-ID: KaXW7UeNQny
devtools/server/actors/inspector.js
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -69,16 +69,17 @@ loader.lazyRequireGetter(this, "findCssS
 loader.lazyRequireGetter(this, "getCssPath", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "getXPath", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "colorUtils", "devtools/shared/css/color", true);
 loader.lazyRequireGetter(this, "EyeDropper", "devtools/server/actors/highlighters/eye-dropper", true);
 loader.lazyRequireGetter(this, "WalkerSearch", "devtools/server/actors/utils/walker-search", true);
 loader.lazyRequireGetter(this, "PageStyleActor", "devtools/server/actors/styles", true);
 loader.lazyRequireGetter(this, "getFontPreviewData", "devtools/server/actors/styles", true);
 loader.lazyRequireGetter(this, "flags", "devtools/shared/flags");
+loader.lazyRequireGetter(this, "throttle", "devtools/shared/throttle", true);
 loader.lazyRequireGetter(this, "LayoutActor", "devtools/server/actors/layout", true);
 loader.lazyRequireGetter(this, "HighlighterActor", "devtools/server/actors/highlighters", true);
 loader.lazyRequireGetter(this, "CustomHighlighterActor", "devtools/server/actors/highlighters", true);
 loader.lazyRequireGetter(this, "isTypeRegistered", "devtools/server/actors/highlighters", true);
 loader.lazyRequireGetter(this, "HighlighterEnvironment", "devtools/server/actors/highlighters", true);
 loader.lazyRequireGetter(this, "EventParsers", "devtools/server/event-parsers", true);
 loader.lazyRequireGetter(this, "isAnonymous", "devtools/shared/layout/utils", true);
 loader.lazyRequireGetter(this, "isNativeAnonymous", "devtools/shared/layout/utils", true);
@@ -100,16 +101,26 @@ const FONT_FAMILY_PREVIEW_TEXT = "The qu
 const FONT_FAMILY_PREVIEW_TEXT_SIZE = 20;
 const PSEUDO_CLASSES = [":hover", ":active", ":focus"];
 const HIDDEN_CLASS = "__fx-devtools-hide-shortcut__";
 const SVG_NS = "http://www.w3.org/2000/svg";
 const XHTML_NS = "http://www.w3.org/1999/xhtml";
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const IMAGE_FETCHING_TIMEOUT = 500;
 
+// Minimum delay between two "new-mutations" events.
+const MUTATIONS_THROTTLING_DELAY = 100;
+// List of mutation types that should -not- be throttled.
+const IMMEDIATE_MUTATIONS = [
+  "documentUnload",
+  "frameLoad",
+  "newRoot",
+  "pseudoClassLock",
+];
+
 // SKIP_TO_* arguments are used with the DocumentWalker, driving the strategy to use if
 // the starting node is incompatible with the filter function of the walker.
 const SKIP_TO_PARENT = "SKIP_TO_PARENT";
 const SKIP_TO_SIBLING = "SKIP_TO_SIBLING";
 
 // The possible completions to a ':' with added score to give certain values
 // some preference.
 const PSEUDO_SELECTORS = [
@@ -875,16 +886,18 @@ var WalkerActor = protocol.ActorClassWit
     // The client can tell the walker that it is interested in a node
     // even when it is orphaned with the `retainNode` method.  This
     // list contains orphaned nodes that were so retained.
     this._retainedOrphans = new Set();
 
     this.onMutations = this.onMutations.bind(this);
     this.onFrameLoad = this.onFrameLoad.bind(this);
     this.onFrameUnload = this.onFrameUnload.bind(this);
+    this._throttledEmitNewMutations = throttle(this._emitNewMutations.bind(this),
+      MUTATIONS_THROTTLING_DELAY);
 
     tabActor.on("will-navigate", this.onFrameUnload);
     tabActor.on("window-ready", this.onFrameLoad);
 
     // Ensure that the root document node actor is ready and
     // managed.
     this.rootNode = this.document();
 
@@ -2294,16 +2307,17 @@ var WalkerActor = protocol.ActorClassWit
    * Keep in mind that if a node that the client hasn't seen is moved
    * into or out of the target node, it will not be included in the
    * removedNodes and addedNodes list, so if the client is interested
    * in the new set of children it needs to issue a `children` request.
    */
   getMutations: function (options = {}) {
     let pending = this._pendingMutations || [];
     this._pendingMutations = [];
+    this._waitingForGetMutations = false;
 
     if (options.cleanup) {
       for (let node of this._orphaned) {
         // Release the orphaned node.  Nodes or children that have been
         // retained will be moved to this._retainedOrphans.
         this.releaseNode(node);
       }
       this._orphaned = new Set();
@@ -2312,27 +2326,54 @@ var WalkerActor = protocol.ActorClassWit
     return pending;
   },
 
   queueMutation: function (mutation) {
     if (!this.actorID || this._destroyed) {
       // We've been destroyed, don't bother queueing this mutation.
       return;
     }
-    // We only send the `new-mutations` notification once, until the client
-    // fetches mutations with the `getMutations` packet.
-    let needEvent = this._pendingMutations.length === 0;
-
+
+    // Add the mutation to the list of mutations to be retrieved next.
     this._pendingMutations.push(mutation);
 
-    if (needEvent) {
-      this.emit("new-mutations");
+    // Bail out if we already emitted a new-mutations event and are waiting for a client
+    // to retrieve them.
+    if (this._waitingForGetMutations) {
+      return;
+    }
+
+    if (IMMEDIATE_MUTATIONS.includes(mutation.type)) {
+      this._emitNewMutations();
+    } else {
+      /**
+       * If many mutations are fired at the same time, clients might sequentially request
+       * children/siblings for updated nodes, which can be costly. By throttling the calls
+       * to getMutations, duplicated mutations will be ignored.
+       */
+      this._throttledEmitNewMutations();
     }
   },
 
+  _emitNewMutations: function () {
+    if (!this.actorID || this._destroyed) {
+      // Bail out if the actor was destroyed after throttling this call.
+      return;
+    }
+
+    if (this._waitingForGetMutations || this._pendingMutations.length == 0) {
+      // Bail out if we already fired the new-mutation event or if no mutations are
+      // waiting to be retrieved.
+      return;
+    }
+
+    this._waitingForGetMutations = true;
+    this.emit("new-mutations");
+  },
+
   /**
    * Handles mutations from the DOM mutation observer API.
    *
    * @param array[MutationRecord] mutations
    *    See https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#MutationRecord
    */
   onMutations: function (mutations) {
     // Notify any observers that want *all* mutations (even on nodes that aren't