Bug 864802 - The _store on each variables view instance is redundant, r=rcampbell
authorVictor Porof <vporof@mozilla.com>
Thu, 25 Apr 2013 09:18:19 +0300
changeset 140792 7a0df791c0f8d8ed0ca49bcd5d17a49a9f57ca65
parent 140791 83a790e5acd8a9b1b4b1a13ee39b15a7adc751e1
child 140793 c9f50b631b7da596aecec506bfc910f89db41b39
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrcampbell
bugs864802
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 864802 - The _store on each variables view instance is redundant, r=rcampbell
browser/devtools/debugger/test/browser_dbg_propertyview-data.js
browser/devtools/shared/widgets/VariablesView.jsm
--- a/browser/devtools/debugger/test/browser_dbg_propertyview-data.js
+++ b/browser/devtools/debugger/test/browser_dbg_propertyview-data.js
@@ -95,17 +95,17 @@ function testVariablesView()
 
 function testHierarchy() {
   is(gVariablesView._currHierarchy.size, 13,
     "There should be 1 scope, 1 var, 1 proto, 8 props, 1 getter and 1 setter.");
 
   gScope = gVariablesView._currHierarchy.get("");
   gVariable = gVariablesView._currHierarchy.get("[\"\"]");
 
-  is(gVariablesView._store.size, 1,
+  is(gVariablesView._store.length, 1,
     "There should be only one scope in the view");
   is(gScope._store.size, 1,
     "There should be only one variable in the scope");
   is(gVariable._store.size, 9,
     "There should be 1 __proto__ and 8 properties in the variable");
 }
 
 function testHeader() {
--- a/browser/devtools/shared/widgets/VariablesView.jsm
+++ b/browser/devtools/shared/widgets/VariablesView.jsm
@@ -46,18 +46,17 @@ const STR = Services.strings.createBundl
  *
  * @param nsIDOMNode aParentNode
  *        The parent node to hold this view.
  * @param object aFlags [optional]
  *        An object contaning initialization options for this view.
  *        e.g. { lazyEmpty: true, searchEnabled: true ... }
  */
 this.VariablesView = function VariablesView(aParentNode, aFlags = {}) {
-  this._store = new Map();
-  this._items = [];
+  this._store = [];
   this._itemsByElement = new WeakMap();
   this._prevHierarchy = new Map();
   this._currHierarchy = new Map();
 
   this._parent = aParentNode;
   this._parent.classList.add("variables-view-container");
   this._appendEmptyNotice();
 
@@ -98,51 +97,49 @@ VariablesView.prototype = {
    * @return Scope
    *         The newly created Scope instance.
    */
   addScope: function VV_addScope(aName = "") {
     this._removeEmptyNotice();
     this._toggleSearchVisibility(true);
 
     let scope = new Scope(this, aName);
-    this._store.set(scope.id, scope);
-    this._items.push(scope);
+    this._store.push(scope);
+    this._itemsByElement.set(scope._target, scope);
     this._currHierarchy.set(aName, scope);
-    this._itemsByElement.set(scope._target, scope);
     scope.header = !!aName;
     return scope;
   },
 
   /**
    * Removes all items from this container.
    *
    * @param number aTimeout [optional]
    *        The number of milliseconds to delay the operation if
    *        lazy emptying of this container is enabled.
    */
   empty: function VV_empty(aTimeout = this.lazyEmptyDelay) {
     // If there are no items in this container, emptying is useless.
-    if (!this._store.size) {
+    if (!this._store.length) {
       return;
     }
     // Check if this empty operation may be executed lazily.
     if (this.lazyEmpty && aTimeout > 0) {
       this._emptySoon(aTimeout);
       return;
     }
 
     let list = this._list;
     let firstChild;
 
     while (firstChild = list.firstChild) {
       list.removeChild(firstChild);
     }
 
-    this._store.clear();
-    this._items.length = 0;
+    this._store.length = 0;
     this._itemsByElement.clear();
 
     this._appendEmptyNotice();
     this._toggleSearchVisibility(false);
   },
 
   /**
    * Emptying this container and rebuilding it immediately afterwards would
@@ -158,32 +155,31 @@ VariablesView.prototype = {
    *
    * @see VariablesView.empty
    * @see VariablesView.commitHierarchy
    */
   _emptySoon: function VV__emptySoon(aTimeout) {
     let prevList = this._list;
     let currList = this._list = this.document.createElement("scrollbox");
 
-    this._store.clear();
-    this._items.length = 0;
+    this._store.length = 0;
     this._itemsByElement.clear();
 
     this._emptyTimeout = this.window.setTimeout(function() {
       this._emptyTimeout = null;
 
       prevList.removeEventListener("keypress", this._onViewKeyPress, false);
       currList.addEventListener("keypress", this._onViewKeyPress, false);
       currList.setAttribute("orient", "vertical");
 
       this._parent.removeChild(prevList);
       this._parent.appendChild(currList);
       this._boxObject = currList.boxObject.QueryInterface(Ci.nsIScrollBoxObject);
 
-      if (!this._store.size) {
+      if (!this._store.length) {
         this._appendEmptyNotice();
         this._toggleSearchVisibility(false);
       }
     }.bind(this), aTimeout);
   },
 
   /**
    * The amount of time (in milliseconds) it takes to empty this view lazily.
@@ -300,30 +296,30 @@ VariablesView.prototype = {
   /**
    * Specifies if enumerable properties and variables should be displayed.
    * These variables and properties are visible by default.
    * @param boolean aFlag
    */
   set enumVisible(aFlag) {
     this._enumVisible = aFlag;
 
-    for (let [, scope] of this._store) {
+    for (let scope of this._store) {
       scope._enumVisible = aFlag;
     }
   },
 
   /**
    * Specifies if non-enumerable properties and variables should be displayed.
    * These variables and properties are visible by default.
    * @param boolean aFlag
    */
   set nonEnumVisible(aFlag) {
     this._nonEnumVisible = aFlag;
 
-    for (let [, scope] of this._store) {
+    for (let scope of this._store) {
       scope._nonEnumVisible = aFlag;
     }
   },
 
   /**
    * Specifies if only enumerable properties and variables should be displayed.
    * Both types of these variables and properties are visible by default.
    * @param boolean aFlag
@@ -379,17 +375,17 @@ VariablesView.prototype = {
     let document = this.document;
     let ownerView = this._parent.parentNode;
 
     let container = this._searchboxContainer = document.createElement("hbox");
     container.className = "devtools-toolbar";
 
     // Hide the variables searchbox container if there are no variables or
     // properties to display.
-    container.hidden = !this._store.size;
+    container.hidden = !this._store.length;
 
     let searchbox = this._searchboxNode = document.createElement("textbox");
     searchbox.className = "variables-view-searchinput devtools-searchinput";
     searchbox.setAttribute("placeholder", this._searchboxPlaceholder);
     searchbox.setAttribute("type", "search");
     searchbox.setAttribute("flex", "1");
     searchbox.addEventListener("input", this._onSearchboxInput, false);
     searchbox.addEventListener("keypress", this._onSearchboxKeyPress, false);
@@ -499,17 +495,17 @@ VariablesView.prototype = {
    * If aQuery is null or undefined, then all the scopes are just unhidden,
    * and the available variables and properties inside those scopes are also
    * just unhidden.
    *
    * @param string aQuery
    *        The variable or property to search for.
    */
   _startSearch: function VV__startSearch(aQuery) {
-    for (let [, scope] of this._store) {
+    for (let scope of this._store) {
       switch (aQuery) {
         case "":
           scope.expand();
           // fall through
         case null:
         case undefined:
           scope._performSearch("");
           break;
@@ -519,17 +515,17 @@ VariablesView.prototype = {
       }
     }
   },
 
   /**
    * Expands the first search results in this container.
    */
   expandFirstSearchResults: function VV_expandFirstSearchResults() {
-    for (let [, scope] of this._store) {
+    for (let scope of this._store) {
       let match = scope._firstMatch;
       if (match) {
         match.expand();
       }
     }
   },
 
   /**
@@ -539,17 +535,17 @@ VariablesView.prototype = {
    *
    * @param function aPredicate
    *        A function that returns true when a match is found.
    * @return Scope | Variable | Property
    *         The first visible scope, variable or property, or null if nothing
    *         is found.
    */
   _findInVisibleItems: function VV__findInVisibleItems(aPredicate) {
-    for (let scope of this._items) {
+    for (let scope of this._store) {
       let result = scope._findInVisibleItems(aPredicate);
       if (result) {
         return result;
       }
     }
     return null;
   },
 
@@ -561,18 +557,18 @@ VariablesView.prototype = {
    *
    * @param function aPredicate
    *        A function that returns true when a match is found.
    * @return Scope | Variable | Property
    *         The last visible scope, variable or property, or null if nothing
    *         is found.
    */
   _findInVisibleItemsReverse: function VV__findInVisibleItemsReverse(aPredicate) {
-    for (let i = this._items.length - 1; i >= 0; i--) {
-      let scope = this._items[i];
+    for (let i = this._store.length - 1; i >= 0; i--) {
+      let scope = this._store[i];
       let result = scope._findInVisibleItemsReverse(aPredicate);
       if (result) {
         return result;
       }
     }
     return null;
   },
 
@@ -896,17 +892,16 @@ VariablesView.prototype = {
    * @return nsIDOMWindow
    */
   get window() this._window || (this._window = this.document.defaultView),
 
   _document: null,
   _window: null,
 
   _store: null,
-  _items: null,
   _prevHierarchy: null,
   _currHierarchy: null,
   _enumVisible: true,
   _nonEnumVisible: true,
   _emptyTimeout: null,
   _searchTimeout: null,
   _searchFunction: null,
   _parent: null,
@@ -1126,18 +1121,18 @@ Scope.prototype = {
    */
   addVar: function S_addVar(aName = "", aDescriptor = {}, aRelaxed = false) {
     if (this._store.has(aName) && !aRelaxed) {
       return null;
     }
 
     let variable = new Variable(this, aName, aDescriptor);
     this._store.set(aName, variable);
+    this._variablesView._itemsByElement.set(variable._target, variable);
     this._variablesView._currHierarchy.set(variable._absoluteName, variable);
-    this._variablesView._itemsByElement.set(variable._target, variable);
     variable.header = !!aName;
     return variable;
   },
 
   /**
    * Gets the variable in this container having the specified name.
    *
    * @param string aName
@@ -1237,17 +1232,18 @@ Scope.prototype = {
     if (this._isExpanded || this._locked) {
       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.lazyAppend && this._store.size > LAZY_APPEND_BATCH) {
+         this._variablesView.lazyAppend &&
+         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;
     }
@@ -1928,16 +1924,18 @@ Scope.prototype = {
   editableNameTooltip: "",
   editButtonTooltip: "",
   deleteButtonTooltip: "",
   descriptorTooltip: true,
   contextMenuId: "",
   separatorStr: "",
 
   _store: null,
+  _enumItems: null,
+  _nonEnumItems: null,
   _fetched: false,
   _retrieved: false,
   _committed: false,
   _batchItems: null,
   _batchTimeout: null,
   _locked: false,
   _isExpanding: false,
   _isExpanded: false,
@@ -1948,19 +1946,17 @@ Scope.prototype = {
   _isMatch: true,
   _idString: "",
   _nameString: "",
   _target: null,
   _arrow: null,
   _name: null,
   _title: null,
   _enum: null,
-  _enumItems: null,
   _nonenum: null,
-  _nonEnumItems: null,
   _throbber: null
 };
 
 /**
  * A Variable is a Scope holding Property instances.
  * Iterable via "for (let [name, property] in instance) { }".
  *
  * @param Scope aScope
@@ -2006,18 +2002,18 @@ ViewHelpers.create({ constructor: Variab
    */
   addProperty: function V_addProperty(aName = "", aDescriptor = {}, aRelaxed = false) {
     if (this._store.has(aName) && !aRelaxed) {
       return null;
     }
 
     let property = new Property(this, aName, aDescriptor);
     this._store.set(aName, property);
+    this._variablesView._itemsByElement.set(property._target, property);
     this._variablesView._currHierarchy.set(property._absoluteName, property);
-    this._variablesView._itemsByElement.set(property._target, property);
     property.header = !!aName;
     return property;
   },
 
   /**
    * Adds properties for this variable.
    *
    * @param object aProperties