Bug 830388 - Avoid multiple bind calls in debugger-controller.js when handling object expansions, r=msucan
authorVictor Porof <vporof@mozilla.com>
Thu, 31 Jan 2013 18:07:25 +0200
changeset 130349 1a0e1c8355254cb9839fc9169ddc94f38ca76dfb
parent 130348 0d953b96904058d1bbe99b4e68efd2109f9bec64
child 130350 73ccb94a810dde1a682db52491a8a55ba706b61f
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmsucan
bugs830388
milestone21.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 830388 - Avoid multiple bind calls in debugger-controller.js when handling object expansions, r=msucan
browser/devtools/debugger/debugger-controller.js
browser/devtools/shared/VariablesView.jsm
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -424,16 +424,19 @@ ThreadState.prototype = {
  * stack frame cache.
  */
 function StackFrames() {
   this._onPaused = this._onPaused.bind(this);
   this._onResumed = this._onResumed.bind(this);
   this._onFrames = this._onFrames.bind(this);
   this._onFramesCleared = this._onFramesCleared.bind(this);
   this._afterFramesCleared = this._afterFramesCleared.bind(this);
+  this._fetchScopeVariables = this._fetchScopeVariables.bind(this);
+  this._fetchVarProperties = this._fetchVarProperties.bind(this);
+  this._addVarExpander = this._addVarExpander.bind(this);
   this.evaluate = this.evaluate.bind(this);
 }
 
 StackFrames.prototype = {
   get activeThread() DebuggerController.activeThread,
   autoScopeExpand: false,
   currentFrame: null,
   syncedWatchExpressions: null,
@@ -682,17 +685,17 @@ StackFrames.prototype = {
     do {
       // Create a scope to contain all the inspected variables.
       let label = this._getScopeLabel(environment);
       let scope = DebuggerView.Variables.addScope(label);
 
       // Handle additions to the innermost scope.
       if (environment == frame.environment) {
         this._insertScopeFrameReferences(scope, frame);
-        this._fetchScopeVariables(scope, environment);
+        this._addScopeExpander(scope, environment);
         // Always expand the innermost scope by default.
         scope.expand();
       }
       // Lazily add nodes for every other environment scope.
       else {
         this._addScopeExpander(scope, environment);
         this.autoScopeExpand && scope.expand();
       }
@@ -708,47 +711,47 @@ StackFrames.prototype = {
    * the addition of new variables.
    *
    * @param Scope aScope
    *        The scope where the variables will be placed into.
    * @param object aEnv
    *        The scope's environment.
    */
   _addScopeExpander: function SF__addScopeExpander(aScope, aEnv) {
-    let callback = this._fetchScopeVariables.bind(this, aScope, aEnv);
+    aScope._sourceEnvironment = aEnv;
 
     // It's a good idea to be prepared in case of an expansion.
-    aScope.addEventListener("mouseover", callback, false);
+    aScope.addEventListener("mouseover", this._fetchScopeVariables, false);
     // Make sure that variables are always available on expansion.
-    aScope.onexpand = callback;
+    aScope.onexpand = this._fetchScopeVariables;
   },
 
   /**
    * Adds an 'onexpand' callback for a variable, lazily handling
    * the addition of new properties.
    *
    * @param Variable aVar
    *        The variable where the properties will be placed into.
    * @param any aGrip
    *        The grip of the variable.
    */
   _addVarExpander: function SF__addVarExpander(aVar, aGrip) {
     // No need for expansion for primitive values.
     if (VariablesView.isPrimitive({ value: aGrip })) {
       return;
     }
-    let callback = this._fetchVarProperties.bind(this, aVar, aGrip);
+    aVar._sourceGrip = aGrip;
 
     // Some variables are likely to contain a very large number of properties.
     // It's a good idea to be prepared in case of an expansion.
     if (aVar.name == "window" || aVar.name == "this") {
-      aVar.addEventListener("mouseover", callback, false);
+      aVar.addEventListener("mouseover", this._fetchVarProperties, false);
     }
     // Make sure that properties are always available on expansion.
-    aVar.onexpand = callback;
+    aVar.onexpand = this._fetchVarProperties;
   },
 
   /**
    * Adds the watch expressions evaluation results to a scope in the view.
    *
    * @param Scope aScope
    *        The scope where the watch expressions will be placed into.
    * @param object aExp
@@ -786,50 +789,49 @@ StackFrames.prototype = {
   },
 
   /**
    * Adds variables to a scope in the view. Triggered when a scope is
    * expanded or is hovered. It does not expand the scope.
    *
    * @param Scope aScope
    *        The scope where the variables will be placed into.
-   * @param object aEnv
-   *        The scope's environment.
    */
-  _fetchScopeVariables: function SF__fetchScopeVariables(aScope, aEnv) {
+  _fetchScopeVariables: function SF__fetchScopeVariables(aScope) {
     // Fetch the variables only once.
     if (aScope._fetched) {
       return;
     }
     aScope._fetched = true;
+    let env = aScope._sourceEnvironment;
 
-    switch (aEnv.type) {
+    switch (env.type) {
       case "with":
       case "object":
         // Add nodes for every variable in scope.
-        this.activeThread.pauseGrip(aEnv.object).getPrototypeAndProperties(function(aResponse) {
+        this.activeThread.pauseGrip(env.object).getPrototypeAndProperties(function(aResponse) {
           this._insertScopeVariables(aResponse.ownProperties, aScope);
 
           // Signal that variables have been fetched.
           window.dispatchEvent("Debugger:FetchedVariables");
           DebuggerView.Variables.commitHierarchy();
         }.bind(this));
         break;
       case "block":
       case "function":
         // Add nodes for every argument and every other variable in scope.
-        this._insertScopeArguments(aEnv.bindings.arguments, aScope);
-        this._insertScopeVariables(aEnv.bindings.variables, aScope);
+        this._insertScopeArguments(env.bindings.arguments, aScope);
+        this._insertScopeVariables(env.bindings.variables, aScope);
 
         // No need to signal that variables have been fetched, since
         // the scope arguments and variables are already attached to the
         // environment bindings, so pausing the active thread is unnecessary.
         break;
       default:
-        Cu.reportError("Unknown Debugger.Environment type: " + aEnv.type);
+        Cu.reportError("Unknown Debugger.Environment type: " + env.type);
         break;
     }
   },
 
   /**
    * Add nodes for special frame references in the innermost scope.
    *
    * @param Scope aScope
@@ -897,37 +899,37 @@ StackFrames.prototype = {
   },
 
   /**
    * Adds properties to a variable in the view. Triggered when a variable is
    * expanded or certain variables are hovered. It does not expand the variable.
    *
    * @param Variable aVar
    *        The variable where the properties will be placed into.
-   * @param any aGrip
-   *        The grip of the variable.
    */
-  _fetchVarProperties: function SF__fetchVarProperties(aVar, aGrip) {
+  _fetchVarProperties: function SF__fetchVarProperties(aVar) {
     // Fetch the properties only once.
     if (aVar._fetched) {
       return;
     }
     aVar._fetched = true;
+    let grip = aVar._sourceGrip;
 
-    this.activeThread.pauseGrip(aGrip).getPrototypeAndProperties(function(aResponse) {
+    this.activeThread.pauseGrip(grip).getPrototypeAndProperties(function(aResponse) {
       let { ownProperties, prototype } = aResponse;
-      let sortable = VARIABLES_VIEW_NON_SORTABLE.indexOf(aGrip.class) == -1;
+      let sortable = VARIABLES_VIEW_NON_SORTABLE.indexOf(grip.class) == -1;
 
       // Add all the variable properties.
       if (ownProperties) {
-        aVar.addProperties(ownProperties, { sorted: sortable });
-        // Expansion handlers must be set after the properties are added.
-        for (let name in ownProperties) {
-          this._addVarExpander(aVar.get(name), ownProperties[name].value);
-        }
+        aVar.addProperties(ownProperties, {
+          // Not all variables need to force sorted properties.
+          sorted: sortable,
+          // Expansion handlers must be set after the properties are added.
+          callback: this._addVarExpander
+        });
       }
 
       // Add the variable's __proto__.
       if (prototype && prototype.type != "null") {
         aVar.addProperty("__proto__", { value: prototype });
         // Expansion handlers must be set after the properties are added.
         this._addVarExpander(aVar.get("__proto__"), prototype);
       }
--- a/browser/devtools/shared/VariablesView.jsm
+++ b/browser/devtools/shared/VariablesView.jsm
@@ -1875,27 +1875,33 @@ create({ constructor: Variable, proto: S
    *                 someProp3: { value: { type: "undefined" } },
    *                 someProp4: { value: { type: "null" } },
    *                 someProp5: { value: { type: "object", class: "Object" } },
    *                 someProp6: { get: { type: "object", class: "Function" },
    *                              set: { type: "undefined" } }
    * @param object aOptions [optional]
    *        Additional options for adding the properties. Supported options:
    *        - sorted: true to sort all the properties before adding them
+   *        - callback: function invoked after each property is added
    */
   addProperties: function V_addProperties(aProperties, aOptions = {}) {
     let propertyNames = Object.keys(aProperties);
 
     // Sort all of the properties before adding them, if preferred.
     if (aOptions.sorted) {
       propertyNames.sort();
     }
     // Add the properties to the current scope.
     for (let name of propertyNames) {
-      this.addProperty(name, aProperties[name]);
+      let descriptor = aProperties[name];
+      let property = this.addProperty(name, descriptor);
+
+      if (aOptions.callback) {
+        aOptions.callback(property, descriptor.value);
+      }
     }
   },
 
   /**
    * Populates this variable to contain all the properties of an object.
    *
    * @param object aObject
    *        The raw object you want to display.
@@ -1928,36 +1934,51 @@ create({ constructor: Variable, proto: S
     }
     // Add the variable's __proto__.
     if (prototype) {
       this._addRawValueProperty("__proto__", {}, prototype);
     }
   },
 
   /**
+   * Populates a specific variable or property instance to contain all the
+   * properties of an object
+   *
+   * @param Variable | Property aVar
+   *        The target variable to populate.
+   * @param object aObject [optional]
+   *        The raw object you want to display. If unspecified, the object is
+   *        assumed to be defined in a _sourceValue property on the target.
+   */
+  _populateTarget: function V__populateTarget(aVar, aObject = aVar._sourceValue) {
+    aVar.populate(aObject);
+  },
+
+  /**
    * Adds a property for this variable based on a raw value descriptor.
    *
    * @param string aName
    *        The property's name.
    * @param object aDescriptor
    *        Specifies the exact property descriptor as returned by a call to
    *        Object.getOwnPropertyDescriptor.
    * @param object aValue
    *        The raw property value you want to display.
    */
   _addRawValueProperty: function V__addRawValueProperty(aName, aDescriptor, aValue) {
     let descriptor = Object.create(aDescriptor);
     descriptor.value = VariablesView.getGrip(aValue);
 
     let propertyItem = this.addProperty(aName, descriptor);
+    propertyItem._sourceValue = aValue;
 
     // Add an 'onexpand' callback for the property, lazily handling
     // the addition of new child properties.
     if (!VariablesView.isPrimitive(descriptor)) {
-      propertyItem.onexpand = this.populate.bind(propertyItem, aValue);
+      propertyItem.onexpand = this._populateTarget;
     }
   },
 
   /**
    * Adds a property for this variable based on a getter/setter descriptor.
    *
    * @param string aName
    *        The property's name.
@@ -1965,18 +1986,17 @@ create({ constructor: Variable, proto: S
    *        Specifies the exact property descriptor as returned by a call to
    *        Object.getOwnPropertyDescriptor.
    */
   _addRawNonValueProperty: function V__addRawNonValueProperty(aName, aDescriptor) {
     let descriptor = Object.create(aDescriptor);
     descriptor.get = VariablesView.getGrip(aDescriptor.get);
     descriptor.set = VariablesView.getGrip(aDescriptor.set);
 
-    let propertyItem = this.addProperty(aName, descriptor);
-    return propertyItem;
+    this.addProperty(aName, descriptor);
   },
 
   /**
    * Gets this variable's path to the topmost scope.
    * For example, a symbolic name may look like "arguments['0']['foo']['bar']".
    * @return string
    */
   get symbolicName() this._symbolicName,