From 251fc8c8eaeb698543f9c588ef020a35b9df882e Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Wed, 18 Dec 2013 19:01:38 +0200 Subject: [PATCH] Bug 830344 - Part 1: Remove the lazy append mechanism in the variables view, r=past --- ...r_dbg_variables-view-large-array-buffer.js | 186 --------------- browser/devtools/debugger/test/head.js | 2 - .../devtools/shared/widgets/VariablesView.jsm | 221 +++--------------- browser/devtools/webconsole/test/head.js | 1 - browser/devtools/webconsole/webconsole.js | 1 - 5 files changed, 30 insertions(+), 381 deletions(-) diff --git a/browser/devtools/debugger/test/browser_dbg_variables-view-large-array-buffer.js b/browser/devtools/debugger/test/browser_dbg_variables-view-large-array-buffer.js index c3a22a1904aa..8f4fc2f78052 100644 --- a/browser/devtools/debugger/test/browser_dbg_variables-view-large-array-buffer.js +++ b/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() { diff --git a/browser/devtools/debugger/test/head.js b/browser/devtools/debugger/test/head.js index 3350f9509eb3..0662579f5433 100644 --- a/browser/devtools/debugger/test/head.js +++ b/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. diff --git a/browser/devtools/shared/widgets/VariablesView.jsm b/browser/devtools/shared/widgets/VariablesView.jsm index 06c08522a3c1..f1931c068885 100644 --- a/browser/devtools/shared/widgets/VariablesView.jsm +++ b/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 - * True if name duplicates should be allowed. + * @param boolean aRelaxed [optional] + * 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 - * A custom class name for this scope. + * @param string aTargetClassName + * 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, - _batchTimeout: null, - _locked: false, - _isExpanding: false, + _isLocked: 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 == "" || this._nameString == "") { - 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. - * - * @param string aName - * The property's name. - * @param object aDescriptor - * The property's descriptor. + * The class name applied to this property's target element. */ - _init: function(aName = "", aDescriptor) { - 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); - } - } + targetClassName: "variables-view-property variable-or-property" }); /** @@ -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(); }, /** diff --git a/browser/devtools/webconsole/test/head.js b/browser/devtools/webconsole/test/head.js index c60e972e605a..1cd21643bab8 100644 --- a/browser/devtools/webconsole/test/head.js +++ b/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, diff --git a/browser/devtools/webconsole/webconsole.js b/browser/devtools/webconsole/webconsole.js index 5e477227fc98..3596285520fe 100644 --- a/browser/devtools/webconsole/webconsole.js +++ b/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 => {