Bug 830344 - Part 1: Remove the lazy append mechanism in the variables view, r=past

browser/devtools/debugger/test/browser_dbg_variables-view-large-array-buffer.js

@@ -12,10 +12,6 @@ let gTab, gDebuggee, gPanel, gDebugger;
 let gVariables;
 
 function test() {
-  // This is a very, very stressful test.
-  // Thankfully, after bug 830344 none of this will be necessary anymore.
-  requestLongerTimeout(10);
-
   initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => {
     gTab = aTab;
     gDebuggee = aDebuggee;
@@ -23,8 +19,6 @@ function test() {
     gDebugger = gPanel.panelWin;
     gVariables = gDebugger.DebuggerView.Variables;
 
-    gDebugger.DebuggerView.Variables.lazyAppend = true;
-
     waitForSourceAndCaretAndScopes(gPanel, ".html", 18)
       .then(() => performTest())
       .then(() => resumeDebuggerThenCloseAndFinish(gPanel))
@@ -39,187 +33,7 @@ function test() {
 }
 
 function performTest() {
-  let deferred = promise.defer();
 
-  let localScope = gVariables.getScopeAtIndex(0);
-  is(localScope.expanded, true,
-    "The local scope should be expanded by default.");
-
-  let localEnums = localScope.target.querySelector(".variables-view-element-details.enum").childNodes;
-  let localNonEnums = localScope.target.querySelector(".variables-view-element-details.nonenum").childNodes;
-
-  is(localEnums.length, 5,
-    "The local scope should contain all the created enumerable elements.");
-  is(localNonEnums.length, 0,
-    "The local scope should contain all the created non-enumerable elements.");
-
-  let bufferVar = localScope.get("buffer");
-  let zVar = localScope.get("z");
-
-  is(bufferVar.target.querySelector(".name").getAttribute("value"), "buffer",
-    "Should have the right property name for 'buffer'.");
-  is(bufferVar.target.querySelector(".value").getAttribute("value"), "ArrayBuffer",
-    "Should have the right property value for 'buffer'.");
-  ok(bufferVar.target.querySelector(".value").className.contains("token-other"),
-    "Should have the right token class for 'buffer'.");
-
-  is(zVar.target.querySelector(".name").getAttribute("value"), "z",
-    "Should have the right property name for 'z'.");
-  is(zVar.target.querySelector(".value").getAttribute("value"), "Int8Array",
-    "Should have the right property value for 'z'.");
-  ok(zVar.target.querySelector(".value").className.contains("token-other"),
-    "Should have the right token class for 'z'.");
-
-  EventUtils.sendMouseEvent({ type: "mousedown" },
-    bufferVar.target.querySelector(".arrow"),
-    gDebugger);
-
-  EventUtils.sendMouseEvent({ type: "mousedown" },
-    zVar.target.querySelector(".arrow"),
-    gDebugger);
-
-  // Need to wait for 0 enumerable and 2 non-enumerable properties in bufferVar,
-  // and 10000 enumerable and 5 non-enumerable properties in zVar.
-  let total = 0 + 2 + 10000 + 5;
-  let loaded = 0;
-  let paints = 0;
-
-  // Make sure the variables view doesn't scroll while adding the properties.
-  let [oldX, oldY] = getScroll();
-  info("Initial scroll position: " + oldX + ", " + oldY);
-
-  waitForProperties(total, {
-    onLoading: function(aLoaded) {
-      ok(aLoaded >= loaded,
-        "Should have loaded more properties.");
-
-      let [newX, newY] = getScroll();
-      info("Current scroll position: " + newX + " " + newY);
-      is(oldX, newX, "The variables view hasn't scrolled horizontally.");
-      is(oldY, newY, "The variables view hasn't scrolled vertically.");
-
-      info("Displayed " + aLoaded + " properties, not finished yet.");
-
-      loaded = aLoaded;
-      paints++;
-    },
-    onFinished: function(aLoaded) {
-      ok(aLoaded == total,
-        "Displayed all the properties.");
-      isnot(paints, 0,
-        "Debugger was unresponsive, sad panda.");
-
-      let [newX, newY] = getScroll();
-      info("Current scroll position: " + newX + ", " + newY);
-      is(oldX, newX, "The variables view hasn't scrolled horizontally.");
-      is(oldY, newY, "The variables view hasn't scrolled vertically.");
-
-      is(bufferVar._enum.childNodes.length, 0,
-        "The bufferVar should contain all the created enumerable elements.");
-      is(bufferVar._nonenum.childNodes.length, 2,
-        "The bufferVar should contain all the created non-enumerable elements.");
-
-      let bufferVarByteLengthProp = bufferVar.get("byteLength");
-      let bufferVarProtoProp = bufferVar.get("__proto__");
-
-      is(bufferVarByteLengthProp.target.querySelector(".name").getAttribute("value"), "byteLength",
-        "Should have the right property name for 'byteLength'.");
-      is(bufferVarByteLengthProp.target.querySelector(".value").getAttribute("value"), "10000",
-        "Should have the right property value for 'byteLength'.");
-      ok(bufferVarByteLengthProp.target.querySelector(".value").className.contains("token-number"),
-        "Should have the right token class for 'byteLength'.");
-
-      is(bufferVarProtoProp.target.querySelector(".name").getAttribute("value"), "__proto__",
-        "Should have the right property name for '__proto__'.");
-      is(bufferVarProtoProp.target.querySelector(".value").getAttribute("value"), "ArrayBufferPrototype",
-        "Should have the right property value for '__proto__'.");
-      ok(bufferVarProtoProp.target.querySelector(".value").className.contains("token-other"),
-        "Should have the right token class for '__proto__'.");
-
-      is(zVar._enum.childNodes.length, 10000,
-        "The zVar should contain all the created enumerable elements.");
-      is(zVar._nonenum.childNodes.length, 5,
-        "The zVar should contain all the created non-enumerable elements.");
-
-      let zVarByteLengthProp = zVar.get("byteLength");
-      let zVarByteOffsetProp = zVar.get("byteOffset");
-      let zVarProtoProp = zVar.get("__proto__");
-
-      is(zVarByteLengthProp.target.querySelector(".name").getAttribute("value"), "byteLength",
-        "Should have the right property name for 'byteLength'.");
-      is(zVarByteLengthProp.target.querySelector(".value").getAttribute("value"), "10000",
-        "Should have the right property value for 'byteLength'.");
-      ok(zVarByteLengthProp.target.querySelector(".value").className.contains("token-number"),
-        "Should have the right token class for 'byteLength'.");
-
-      is(zVarByteOffsetProp.target.querySelector(".name").getAttribute("value"), "byteOffset",
-        "Should have the right property name for 'byteOffset'.");
-      is(zVarByteOffsetProp.target.querySelector(".value").getAttribute("value"), "0",
-        "Should have the right property value for 'byteOffset'.");
-      ok(zVarByteOffsetProp.target.querySelector(".value").className.contains("token-number"),
-        "Should have the right token class for 'byteOffset'.");
-
-      is(zVarProtoProp.target.querySelector(".name").getAttribute("value"), "__proto__",
-        "Should have the right property name for '__proto__'.");
-      is(zVarProtoProp.target.querySelector(".value").getAttribute("value"), "Int8ArrayPrototype",
-        "Should have the right property value for '__proto__'.");
-      ok(zVarProtoProp.target.querySelector(".value").className.contains("token-other"),
-        "Should have the right token class for '__proto__'.");
-
-      let arrayElements = zVar._enum.childNodes;
-      for (let i = 0, len = arrayElements.length; i < len; i++) {
-        let node = arrayElements[i];
-        let name = node.querySelector(".name").getAttribute("value");
-        let value = node.querySelector(".value").getAttribute("value");
-        if (name !== i + "" || value !== "0") {
-          ok(false, "The array items aren't in the correct order.");
-        }
-      }
-
-      deferred.resolve();
-    },
-    onTimeout: function() {
-      ok(false, "Timed out while polling for the properties.");
-      deferred.resolve();
-    }
-  });
-
-  function getScroll() {
-    let scrollX = {};
-    let scrollY = {};
-    gVariables.boxObject.getPosition(scrollX, scrollY);
-    return [scrollX.value, scrollY.value];
-  }
-
-  return deferred.promise;
-}
-
-function waitForProperties(aTotal, aCallbacks, aInterval = 10) {
-  let localScope = gVariables.getScopeAtIndex(0);
-  let bufferEnum = localScope.get("buffer")._enum.childNodes;
-  let bufferNonEnum = localScope.get("buffer")._nonenum.childNodes;
-  let zEnum = localScope.get("z")._enum.childNodes;
-  let zNonEnum = localScope.get("z")._nonenum.childNodes;
-
-  // Poll every few milliseconds until the properties are retrieved.
-  let count = 0;
-  let intervalId = window.setInterval(() => {
-    // Make sure we don't wait for too long.
-    if (++count > 1000) {
-      window.clearInterval(intervalId);
-      aCallbacks.onTimeout();
-      return;
-    }
-    // Check if we need to wait for a few more properties to be fetched.
-    let loaded = bufferEnum.length + bufferNonEnum.length + zEnum.length + zNonEnum.length;
-    if (loaded < aTotal) {
-      aCallbacks.onLoading(loaded);
-      return;
-    }
-    // We got all the properties, it's safe to callback.
-    window.clearInterval(intervalId);
-    aCallbacks.onFinished(loaded);
-  }, aInterval);
 }
 
 registerCleanupFunction(function() {

browser/devtools/debugger/test/head.js

@@ -488,8 +488,6 @@ function prepareDebugger(aDebugger) {
   if ("target" in aDebugger) {
     let variables = aDebugger.panelWin.DebuggerView.Variables;
     variables.lazyEmpty = false;
-    variables.lazyAppend = false;
-    variables.lazyExpand = false;
     variables.lazySearch = false;
   } else {
     // Nothing to do here yet.

browser/devtools/shared/widgets/VariablesView.jsm

@@ -11,8 +11,6 @@ const Cu = Components.utils;
 const DBG_STRINGS_URI = "chrome://browser/locale/devtools/debugger.properties";
 const LAZY_EMPTY_DELAY = 150; // ms
 const LAZY_EXPAND_DELAY = 50; // ms
-const LAZY_APPEND_DELAY = 100; // ms
-const LAZY_APPEND_BATCH = 100; // nodes
 const PAGE_SIZE_SCROLL_HEIGHT_RATIO = 100;
 const PAGE_SIZE_MAX_JUMPS = 30;
 const SEARCH_ACTION_MAX_DELAY = 300; // ms
@@ -225,18 +223,6 @@ VariablesView.prototype = {
    */
   lazyEmpty: false,
 
-  /**
-   * Specifies if nodes in this view may be added lazily.
-   * @see Scope.prototype._lazyAppend
-   */
-  lazyAppend: true,
-
-  /**
-   * Specifies if nodes in this view may be expanded lazily.
-   * @see Scope.prototype.expand
-   */
-  lazyExpand: true,
-
   /**
    * Specifies if nodes in this view may be searched lazily.
    */
@@ -1201,7 +1187,6 @@ function Scope(aView, aName, aFlags = {}) {
   this._onClick = this._onClick.bind(this);
   this._openEnum = this._openEnum.bind(this);
   this._openNonEnum = this._openNonEnum.bind(this);
-  this._batchAppend = this._batchAppend.bind(this);
 
   // Inherit properties and flags from the parent view. You can override
   // each of these directly onto any scope, variable or property instance.
@@ -1227,6 +1212,11 @@ Scope.prototype = {
    */
   shouldPrefetch: true,
 
+  /**
+   * The class name applied to this scope's target element.
+   */
+  targetClassName: "variables-view-scope",
+
   /**
    * Create a new Variable that is a child of this Scope.
    *
@@ -1259,8 +1249,9 @@ Scope.prototype = {
    *             - { value: { type: "object", class: "Object" } }
    *             - { get: { type: "object", class: "Function" },
    *                 set: { type: "undefined" } }
-   * @param boolean aRelaxed
+   * @param boolean aRelaxed [optional]
-   *        True if name duplicates should be allowed.
+   *        Pass true if name duplicates should be allowed.
+   *        You probably shouldn't do it. Use this with caution.
    * @return Variable
    *         The newly created Variable instance, null if it already exists.
    */
@@ -1432,32 +1423,15 @@ Scope.prototype = {
    * Expands the scope, showing all the added details.
    */
   expand: function() {
-    if (this._isExpanded || this._locked) {
+    if (this._isExpanded || this._isLocked) {
       return;
     }
-    // If there's a large number of enumerable or non-enumerable items
-    // contained in this scope, painting them may take several seconds,
-    // even if they were already displayed before. In this case, show a throbber
-    // to suggest that this scope is expanding.
-    if (!this._isExpanding &&
-         this._variablesView.lazyExpand &&
-         this._store.size > LAZY_APPEND_BATCH) {
-      this._isExpanding = true;
-
-      // Start spinning a throbber in this scope's title and allow a few
-      // milliseconds for it to be painted.
-      this._startThrobber();
-      this.window.setTimeout(this.expand.bind(this), LAZY_EXPAND_DELAY);
-      return;
-    }
-
     if (this._variablesView._enumVisible) {
       this._openEnum();
     }
     if (this._variablesView._nonEnumVisible) {
       Services.tm.currentThread.dispatch({ run: this._openNonEnum }, 0);
     }
-    this._isExpanding = false;
     this._isExpanded = true;
 
     if (this.onexpand) {
@@ -1469,7 +1443,7 @@ Scope.prototype = {
    * Collapses the scope, hiding all the added details.
    */
   collapse: function() {
-    if (!this._isExpanded || this._locked) {
+    if (!this._isExpanded || this._isLocked) {
       return;
     }
     this._arrow.removeAttribute("open");
@@ -1576,7 +1550,7 @@ Scope.prototype = {
    * Gets the expand lock state.
    * @return boolean
    */
-  get locked() this._locked,
+  get locked() this._isLocked,
 
   /**
    * Sets the visibility state.
@@ -1606,7 +1580,7 @@ Scope.prototype = {
    * Sets the expand lock state.
    * @param boolean aFlag
    */
-  set locked(aFlag) this._locked = aFlag,
+  set locked(aFlag) this._isLocked = aFlag,
 
   /**
    * Specifies if this target node may be focused.
@@ -1699,7 +1673,7 @@ Scope.prototype = {
    */
   _init: function(aName, aFlags) {
     this._idString = generateId(this._nameString = aName);
-    this._displayScope(aName, "variables-view-scope", "devtools-toolbar");
+    this._displayScope(aName, this.targetClassName, "devtools-toolbar");
     this._addEventListeners();
     this.parentNode.appendChild(this._target);
   },
@@ -1709,17 +1683,17 @@ Scope.prototype = {
    *
    * @param string aName
    *        The scope's name.
-   * @param string aClassName
+   * @param string aTargetClassName
-   *        A custom class name for this scope.
+   *        A custom class name for this scope's target element.
    * @param string aTitleClassName [optional]
-   *        A custom class name for this scope's title.
+   *        A custom class name for this scope's title element.
    */
-  _displayScope: function(aName, aClassName, aTitleClassName) {
+  _displayScope: function(aName, aTargetClassName, aTitleClassName = "") {
     let document = this.document;
 
     let element = this._target = document.createElement("vbox");
     element.id = this._idString;
-    element.className = aClassName;
+    element.className = aTargetClassName;
 
     let arrow = this._arrow = document.createElement("hbox");
     arrow.className = "arrow";
@@ -1729,7 +1703,7 @@ Scope.prototype = {
     name.setAttribute("value", aName);
 
     let title = this._title = document.createElement("hbox");
-    title.className = "title " + (aTitleClassName || "");
+    title.className = "title " + aTitleClassName;
     title.setAttribute("align", "center");
 
     let enumerable = this._enum = document.createElement("vbox");
@@ -1766,99 +1740,12 @@ Scope.prototype = {
     this.focus();
   },
 
-  /**
-   * Lazily appends a node to this scope's enumerable or non-enumerable
-   * container. Once a certain number of nodes have been batched, they
-   * will be appended.
-   *
-   * @param boolean aImmediateFlag
-   *        Set to false if append calls should be dispatched synchronously
-   *        on the current thread, to allow for a paint flush.
-   * @param boolean aEnumerableFlag
-   *        Specifies if the node to append is enumerable or non-enumerable.
-   * @param nsIDOMNode aChild
-   *        The child node to append.
-   */
-  _lazyAppend: function(aImmediateFlag, aEnumerableFlag, aChild) {
-    // Append immediately, don't stage items and don't allow for a paint flush.
-    if (aImmediateFlag || !this._variablesView.lazyAppend) {
-      if (aEnumerableFlag) {
-        this._enum.appendChild(aChild);
-      } else {
-        this._nonenum.appendChild(aChild);
-      }
-      return;
-    }
-
-    let window = this.window;
-    let batchItems = this._batchItems;
-
-    window.clearTimeout(this._batchTimeout);
-    batchItems.push({ enumerableFlag: aEnumerableFlag, child: aChild });
-
-    // If a certain number of nodes have been batched, append all the
-    // staged items now.
-    if (batchItems.length > LAZY_APPEND_BATCH) {
-      // Allow for a paint flush.
-      Services.tm.currentThread.dispatch({ run: this._batchAppend }, 1);
-      return;
-    }
-    // Postpone appending the staged items for later, to allow batching
-    // more nodes.
-    this._batchTimeout = window.setTimeout(this._batchAppend, LAZY_APPEND_DELAY);
-  },
-
-  /**
-   * Appends all the batched nodes to this scope's enumerable and non-enumerable
-   * containers.
-   */
-  _batchAppend: function() {
-    let document = this.document;
-    let batchItems = this._batchItems;
-
-    // Create two document fragments, one for enumerable nodes, and one
-    // for non-enumerable nodes.
-    let frags = [document.createDocumentFragment(), document.createDocumentFragment()];
-
-    for (let item of batchItems) {
-      frags[~~item.enumerableFlag].appendChild(item.child);
-    }
-    batchItems.length = 0;
-    this._enum.appendChild(frags[1]);
-    this._nonenum.appendChild(frags[0]);
-  },
-
-  /**
-   * Starts spinning a throbber in this scope's title.
-   */
-  _startThrobber: function() {
-    if (this._throbber) {
-      this._throbber.hidden = false;
-      return;
-    }
-    let throbber = this._throbber = this.document.createElement("hbox");
-    throbber.className = "variables-view-throbber";
-    throbber.setAttribute("optional-visibility", "");
-    this._title.insertBefore(throbber, this._spacer);
-  },
-
-  /**
-   * Stops spinning the throbber in this scope's title.
-   */
-  _stopThrobber: function() {
-    if (!this._throbber) {
-      return;
-    }
-    this._throbber.hidden = true;
-  },
-
   /**
    * Opens the enumerable items container.
    */
   _openEnum: function() {
     this._arrow.setAttribute("open", "");
     this._enum.setAttribute("open", "");
-    this._stopThrobber();
   },
 
   /**
@@ -1866,7 +1753,6 @@ Scope.prototype = {
    */
   _openNonEnum: function() {
     this._nonenum.setAttribute("open", "");
-    this._stopThrobber();
   },
 
   /**
@@ -2108,10 +1994,7 @@ Scope.prototype = {
   _fetched: false,
   _retrieved: false,
   _committed: false,
-  _batchItems: null,
+  _isLocked: false,
-  _batchTimeout: null,
-  _locked: false,
-  _isExpanding: false,
   _isExpanded: false,
   _isContentVisible: true,
   _isHeaderVisible: true,
@@ -2125,7 +2008,6 @@ Scope.prototype = {
   _title: null,
   _enum: null,
   _nonenum: null,
-  _throbber: null
 };
 
 // Creating maps and arrays thousands of times for variables or properties
@@ -2134,7 +2016,6 @@ Scope.prototype = {
 DevToolsUtils.defineLazyPrototypeGetter(Scope.prototype, "_store", Map);
 DevToolsUtils.defineLazyPrototypeGetter(Scope.prototype, "_enumItems", Array);
 DevToolsUtils.defineLazyPrototypeGetter(Scope.prototype, "_nonEnumItems", Array);
-DevToolsUtils.defineLazyPrototypeGetter(Scope.prototype, "_batchItems", Array);
 
 /**
  * A Variable is a Scope holding Property instances.
@@ -2173,6 +2054,11 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
     return this.name == "window" || this.name == "this";
   },
 
+  /**
+   * The class name applied to this variable's target element.
+   */
+  targetClassName: "variables-view-variable variable-or-property",
+
   /**
    * Create a new Property that is a child of Variable.
    *
@@ -2415,33 +2301,21 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
    */
   _init: function(aName, aDescriptor) {
     this._idString = generateId(this._nameString = aName);
-    this._displayScope(aName, "variables-view-variable variable-or-property");
+    this._displayScope(aName, this.targetClassName);
-
     this._displayVariable();
     this._customizeVariable();
     this._prepareTooltips();
     this._setAttributes();
     this._addEventListeners();
 
-    this._onInit(this.ownerView._store.size < LAZY_APPEND_BATCH);
-  },
-
-  /**
-   * Called when this variable has finished initializing, and is ready to
-   * be attached to the owner view.
-   *
-   * @param boolean aImmediateFlag
-   *        @see Scope.prototype._lazyAppend
-   */
-  _onInit: function(aImmediateFlag) {
     if (this._initialDescriptor.enumerable ||
         this._nameString == "this" ||
         this._nameString == "<return>" ||
         this._nameString == "<exception>") {
-      this.ownerView._lazyAppend(aImmediateFlag, true, this._target);
+      this.ownerView._enum.appendChild(this._target);
       this.ownerView._enumItems.push(this);
     } else {
-      this.ownerView._lazyAppend(aImmediateFlag, false, this._target);
+      this.ownerView._nonenum.appendChild(this._target);
       this.ownerView._nonEnumItems.push(this);
     }
   },
@@ -2868,42 +2742,9 @@ function Property(aVar, aName, aDescriptor) {
 
 Property.prototype = Heritage.extend(Variable.prototype, {
   /**
-   * Initializes this property's id, view and binds event listeners.
+   * The class name applied to this property's target element.
-   *
-   * @param string aName
-   *        The property's name.
-   * @param object aDescriptor
-   *        The property's descriptor.
    */
-  _init: function(aName = "", aDescriptor) {
+  targetClassName: "variables-view-property variable-or-property"
-    this._idString = generateId(this._nameString = aName);
-    this._displayScope(aName, "variables-view-property variable-or-property");
-
-    this._displayVariable();
-    this._customizeVariable();
-    this._prepareTooltips();
-    this._setAttributes();
-    this._addEventListeners();
-
-    this._onInit(this.ownerView._store.size < LAZY_APPEND_BATCH);
-  },
-
-  /**
-   * Called when this property has finished initializing, and is ready to
-   * be attached to the owner view.
-   *
-   * @param boolean aImmediateFlag
-   *        @see Scope.prototype._lazyAppend
-   */
-  _onInit: function(aImmediateFlag) {
-    if (this._initialDescriptor.enumerable) {
-      this.ownerView._lazyAppend(aImmediateFlag, true, this._target);
-      this.ownerView._enumItems.push(this);
-    } else {
-      this.ownerView._lazyAppend(aImmediateFlag, false, this._target);
-      this.ownerView._nonEnumItems.push(this);
-    }
-  }
 });
 
 /**
@@ -3365,7 +3206,6 @@ Editable.prototype = {
     this._variable.collapse();
     this._variable.hideArrow();
     this._variable.locked = true;
-    this._variable._stopThrobber();
   },
 
   /**
@@ -3383,7 +3223,6 @@ Editable.prototype = {
     this._variable.locked = false;
     this._variable.twisty = this._prevExpandable;
     this._variable.expanded = this._prevExpanded;
-    this._variable._stopThrobber();
   },
 
   /**

browser/devtools/webconsole/test/head.js

@@ -796,7 +796,6 @@ function openDebugger(aOptions = {})
     let panelWin = panel.panelWin;
 
     panel._view.Variables.lazyEmpty = false;
-    panel._view.Variables.lazyAppend = false;
 
     let resolveObject = {
       target: target,

browser/devtools/webconsole/webconsole.js

@@ -3431,7 +3431,6 @@ JSTerm.prototype = {
     view.emptyText = l10n.getStr("emptyPropertiesList");
     view.searchEnabled = !aOptions.hideFilterInput;
     view.lazyEmpty = this._lazyVariablesView;
-    view.lazyAppend = this._lazyVariablesView;
 
     VariablesViewController.attach(view, {
       getEnvironmentClient: aGrip => {