Bug 810966 - Display closed over variables in the variables view for functions that are not stack frames; r=vporof,msucan

browser/devtools/debugger/debugger-toolbar.js

@@ -663,62 +663,6 @@ StackFramesView.prototype = Heritage.extend(WidgetMethods, {
   _prevBlackBoxedUrl: null
 });
 
-/**
- * Utility functions for handling stackframes.
- */
-let StackFrameUtils = {
-  /**
-   * Create a textual representation for the specified stack frame
-   * to display in the stackframes container.
-   *
-   * @param object aFrame
-   *        The stack frame to label.
-   */
-  getFrameTitle: function(aFrame) {
-    if (aFrame.type == "call") {
-      let c = aFrame.callee;
-      return (c.userDisplayName || c.displayName || c.name || "(anonymous)");
-    }
-    return "(" + aFrame.type + ")";
-  },
-
-  /**
-   * Constructs a scope label based on its environment.
-   *
-   * @param object aEnv
-   *        The scope's environment.
-   * @return string
-   *         The scope's label.
-   */
-  getScopeLabel: function(aEnv) {
-    let name = "";
-
-    // Name the outermost scope Global.
-    if (!aEnv.parent) {
-      name = L10N.getStr("globalScopeLabel");
-    }
-    // Otherwise construct the scope name.
-    else {
-      name = aEnv.type.charAt(0).toUpperCase() + aEnv.type.slice(1);
-    }
-
-    let label = L10N.getFormatStr("scopeLabel", name);
-    switch (aEnv.type) {
-      case "with":
-      case "object":
-        label += " [" + aEnv.object.class + "]";
-        break;
-      case "function":
-        let f = aEnv.function;
-        label += " [" +
-          (f.userDisplayName || f.displayName || f.name || "(anonymous)") +
-        "]";
-        break;
-    }
-    return label;
-  }
-};
-
 /**
  * Functions handling the filtering UI.
  */

browser/devtools/debugger/debugger-view.js

@@ -148,6 +148,7 @@ let DebuggerView = {
 
     // Attach a controller that handles interfacing with the debugger protocol.
     VariablesViewController.attach(this.Variables, {
+      getEnvironmentClient: aObject => gThreadClient.environment(aObject),
       getObjectClient: aObject => gThreadClient.pauseGrip(aObject)
     });
 

browser/devtools/debugger/test/browser.ini

@@ -22,6 +22,7 @@ support-files =
   code_ugly.js
   doc_binary_search.html
   doc_blackboxing.html
+  doc_closures.html
   doc_cmd-break.html
   doc_cmd-dbg.html
   doc_conditional-breakpoints.html
@@ -69,6 +70,7 @@ support-files =
 [browser_dbg_chrome-debugging.js]
 [browser_dbg_clean-exit-window.js]
 [browser_dbg_clean-exit.js]
+[browser_dbg_closure-inspection.js]
 [browser_dbg_cmd-blackbox.js]
 [browser_dbg_cmd-break.js]
 [browser_dbg_cmd-dbg.js]

browser/devtools/debugger/test/browser_dbg_closure-inspection.js

@@ -0,0 +1,216 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+const TAB_URL = EXAMPLE_URL + "doc_closures.html";
+
+// Test that inspecting a closure works as expected.
+
+function test() {
+  let gPanel, gTab, gDebuggee, gDebugger;
+
+  initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => {
+    gTab = aTab;
+    gDebuggee = aDebuggee;
+    gPanel = aPanel;
+    gDebugger = gPanel.panelWin;
+
+    waitForSourceShown(gPanel, ".html")
+      .then(testClosure)
+      .then(() => resumeDebuggerThenCloseAndFinish(gPanel))
+      .then(null, aError => {
+        ok(false, "Got an error: " + aError.message + "\n" + aError.stack);
+      });
+  });
+
+  function testClosure() {
+    // Spin the event loop before causing the debuggee to pause, to allow
+    // this function to return first.
+    executeSoon(() => {
+      EventUtils.sendMouseEvent({ type: "click" },
+        gDebuggee.document.querySelector("button"),
+        gDebuggee);
+    });
+
+    gDebuggee.gRecurseLimit = 2;
+
+    return waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_SCOPES).then(() => {
+      let deferred = promise.defer();
+
+      let gVars = gDebugger.DebuggerView.Variables,
+          localScope = gVars.getScopeAtIndex(0),
+          globalScope = gVars.getScopeAtIndex(1),
+          localNodes = localScope.target.querySelector(".variables-view-element-details").childNodes,
+          globalNodes = globalScope.target.querySelector(".variables-view-element-details").childNodes;
+
+      is(localNodes[4].querySelector(".name").getAttribute("value"), "person",
+        "Should have the right property name for |person|.");
+
+      is(localNodes[4].querySelector(".value").getAttribute("value"), "Object",
+        "Should have the right property value for |person|.");
+
+      // Expand the 'person' tree node. This causes its properties to be
+      // retrieved and displayed.
+      let personNode = gVars.getItemForNode(localNodes[4]);
+      personNode.expand();
+      is(personNode.expanded, true, "person should be expanded at this point.");
+
+      // Poll every few milliseconds until the properties are retrieved.
+      // It's important to set the timer in the chrome window, because the
+      // content window timers are disabled while the debuggee is paused.
+      let count1 = 0;
+      let intervalID = window.setInterval(function(){
+        info("count1: " + count1);
+        if (++count1 > 50) {
+          ok(false, "Timed out while polling for the properties.");
+          window.clearInterval(intervalID);
+          deferred.reject("Timed out.");
+          return;
+        }
+        if (!personNode._retrieved) {
+          return;
+        }
+        window.clearInterval(intervalID);
+
+        is(personNode.get("getName").target.querySelector(".name")
+           .getAttribute("value"), "getName",
+          "Should have the right property name for 'getName' in person.");
+        is(personNode.get("getName").target.querySelector(".value")
+           .getAttribute("value"), "Function",
+          "'getName' in person should have the right value.");
+        is(personNode.get("getFoo").target.querySelector(".name")
+           .getAttribute("value"), "getFoo",
+          "Should have the right property name for 'getFoo' in person.");
+        is(personNode.get("getFoo").target.querySelector(".value")
+           .getAttribute("value"), "Function",
+          "'getFoo' in person should have the right value.");
+
+        // Expand the function nodes. This causes their properties to be
+        // retrieved and displayed.
+        let getFooNode = personNode.get("getFoo");
+        let getNameNode = personNode.get("getName");
+        getFooNode.expand();
+        getNameNode.expand();
+        is(getFooNode.expanded, true, "person.getFoo should be expanded at this point.");
+        is(getNameNode.expanded, true, "person.getName should be expanded at this point.");
+
+        // Poll every few milliseconds until the properties are retrieved.
+        // It's important to set the timer in the chrome window, because the
+        // content window timers are disabled while the debuggee is paused.
+        let count2 = 0;
+        let intervalID1 = window.setInterval(function(){
+          info("count2: " + count2);
+          if (++count2 > 50) {
+            ok(false, "Timed out while polling for the properties.");
+            window.clearInterval(intervalID1);
+            deferred.reject("Timed out.");
+            return;
+          }
+          if (!getFooNode._retrieved || !getNameNode._retrieved) {
+            return;
+          }
+          window.clearInterval(intervalID1);
+
+          is(getFooNode.get("<Closure>").target.querySelector(".name")
+             .getAttribute("value"), "<Closure>",
+            "Found the closure node for getFoo.");
+          is(getFooNode.get("<Closure>").target.querySelector(".value")
+             .getAttribute("value"), "",
+            "The closure node has no value for getFoo.");
+          is(getNameNode.get("<Closure>").target.querySelector(".name")
+             .getAttribute("value"), "<Closure>",
+            "Found the closure node for getName.");
+          is(getNameNode.get("<Closure>").target.querySelector(".value")
+             .getAttribute("value"), "",
+            "The closure node has no value for getName.");
+
+          // Expand the Closure nodes.
+          let getFooClosure = getFooNode.get("<Closure>");
+          let getNameClosure = getNameNode.get("<Closure>");
+          getFooClosure.expand();
+          getNameClosure.expand();
+          is(getFooClosure.expanded, true, "person.getFoo closure should be expanded at this point.");
+          is(getNameClosure.expanded, true, "person.getName closure should be expanded at this point.");
+
+          // Poll every few milliseconds until the properties are retrieved.
+          // It's important to set the timer in the chrome window, because the
+          // content window timers are disabled while the debuggee is paused.
+          let count3 = 0;
+          let intervalID2 = window.setInterval(function(){
+            info("count3: " + count3);
+            if (++count3 > 50) {
+              ok(false, "Timed out while polling for the properties.");
+              window.clearInterval(intervalID2);
+              deferred.reject("Timed out.");
+              return;
+            }
+            if (!getFooClosure._retrieved || !getNameClosure._retrieved) {
+              return;
+            }
+            window.clearInterval(intervalID2);
+
+            is(getFooClosure.get("Function scope [_pfactory]").target.querySelector(".name")
+               .getAttribute("value"), "Function scope [_pfactory]",
+              "Found the function scope node for the getFoo closure.");
+            is(getFooClosure.get("Function scope [_pfactory]").target.querySelector(".value")
+               .getAttribute("value"), "",
+              "The function scope node has no value for the getFoo closure.");
+            is(getNameClosure.get("Function scope [_pfactory]").target.querySelector(".name")
+               .getAttribute("value"), "Function scope [_pfactory]",
+              "Found the function scope node for the getName closure.");
+            is(getNameClosure.get("Function scope [_pfactory]").target.querySelector(".value")
+               .getAttribute("value"), "",
+              "The function scope node has no value for the getName closure.");
+
+            // Expand the scope nodes.
+            let getFooInnerScope = getFooClosure.get("Function scope [_pfactory]");
+            let getNameInnerScope = getNameClosure.get("Function scope [_pfactory]");
+            getFooInnerScope.expand();
+            getNameInnerScope.expand();
+            is(getFooInnerScope.expanded, true, "person.getFoo inner scope should be expanded at this point.");
+            is(getNameInnerScope.expanded, true, "person.getName inner scope should be expanded at this point.");
+
+            // Poll every few milliseconds until the properties are retrieved.
+            // It's important to set the timer in the chrome window, because the
+            // content window timers are disabled while the debuggee is paused.
+            let count4 = 0;
+            let intervalID3 = window.setInterval(function(){
+              info("count4: " + count4);
+              if (++count4 > 50) {
+                ok(false, "Timed out while polling for the properties.");
+                window.clearInterval(intervalID3);
+                deferred.reject("Timed out.");
+                return;
+              }
+              if (!getFooInnerScope._retrieved || !getNameInnerScope._retrieved) {
+                return;
+              }
+              window.clearInterval(intervalID3);
+
+              // Only test that each function closes over the necessary variable.
+              // We wouldn't want future SpiderMonkey closure space
+              // optimizations to break this test.
+              is(getFooInnerScope.get("foo").target.querySelector(".name")
+                 .getAttribute("value"), "foo",
+                "Found the foo node for the getFoo inner scope.");
+              is(getFooInnerScope.get("foo").target.querySelector(".value")
+                 .getAttribute("value"), "10",
+                "The foo node has the expected value.");
+              is(getNameInnerScope.get("name").target.querySelector(".name")
+                 .getAttribute("value"), "name",
+                "Found the name node for the getName inner scope.");
+              is(getNameInnerScope.get("name").target.querySelector(".value")
+                 .getAttribute("value"), '"Bob"',
+                "The name node has the expected value.");
+
+              deferred.resolve();
+            }, 100);
+          }, 100);
+        }, 100);
+      }, 100);
+
+      return deferred.promise;
+    });
+  }
+}

browser/devtools/debugger/test/browser_dbg_variables-view-filter-01.js

@@ -81,8 +81,6 @@ function testVariablesAndPropertiesFiltering() {
     isnot(globalScope.target.querySelectorAll(".variables-view-variable:not([non-match])").length, 0,
       "There should be some variables displayed in the global scope.");
 
-    is(localScope.target.querySelectorAll(".variables-view-property:not([non-match])").length, 3,
-      "There should be 3 properties displayed in the local scope.");
     is(withScope.target.querySelectorAll(".variables-view-property:not([non-match])").length, 0,
       "There should be 0 properties displayed in the with scope.");
     is(functionScope.target.querySelectorAll(".variables-view-property:not([non-match])").length, 0,

browser/devtools/debugger/test/doc_closures.html

@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <meta charset='utf-8'/>
+    <title>Debugger Test for Closure Inspection</title>
+    <!-- Any copyright is dedicated to the Public Domain.
+         http://creativecommons.org/publicdomain/zero/1.0/ -->
+    <script type="text/javascript">
+      window.addEventListener("load", function onload() {
+        window.removeEventListener("load", onload);
+        function clickHandler(event) {
+          button.removeEventListener("click", clickHandler, false);
+          var PersonFactory = function _pfactory(name) {
+              var foo = 10;
+              return {
+                getName: function() { return name; },
+                getFoo: function() { foo = Date.now(); return foo; }
+              };
+          };
+          var person = new PersonFactory("Bob");
+          debugger;
+        }
+        var button = document.querySelector("button");
+        button.addEventListener("click", clickHandler, false);
+      });
+    </script>
+
+  </head>
+  <body>
+    <button>Click me!</button>
+  </body>
+</html>

browser/devtools/scratchpad/scratchpad.js

@@ -56,6 +56,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "VariablesView",
 XPCOMUtils.defineLazyModuleGetter(this, "VariablesViewController",
   "resource:///modules/devtools/VariablesViewController.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "EnvironmentClient",
+  "resource://gre/modules/devtools/dbg-client.jsm");
+
 XPCOMUtils.defineLazyModuleGetter(this, "ObjectClient",
   "resource://gre/modules/devtools/dbg-client.jsm");
 
@@ -1744,6 +1747,9 @@ ScratchpadSidebar.prototype = {
         });
 
         VariablesViewController.attach(this.variablesView, {
+          getEnvironmentClient: aGrip => {
+            return new EnvironmentClient(this._scratchpad.debuggerClient, aGrip);
+          },
           getObjectClient: aGrip => {
             return new ObjectClient(this._scratchpad.debuggerClient, aGrip);
           },

browser/devtools/shared/widgets/VariablesView.jsm

@@ -1136,7 +1136,8 @@ Scope.prototype = {
    * @param object aDescriptor
    *        Specifies the value and/or type & class of the child,
    *        or 'get' & 'set' accessor properties. If the type is implicit,
-   *        it will be inferred from the value.
+   *        it will be inferred from the value. If this parameter is omitted,
+   *        a property without a value will be added (useful for branch nodes).
    *        e.g. - { value: 42 }
    *             - { value: true }
    *             - { value: "nasu" }
@@ -2304,6 +2305,11 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
       this.hideArrow();
     }
 
+    // If no value will be displayed, we don't need the separator.
+    if (!descriptor.get && !descriptor.set && !descriptor.value) {
+      separatorLabel.hidden = true;
+    }
+
     if (descriptor.get || descriptor.set) {
       separatorLabel.hidden = true;
       valueLabel.hidden = true;
@@ -2441,7 +2447,8 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
 
   /**
    * Sets a variable's configurable, enumerable and writable attributes,
-   * and specifies if it's a 'this', '<exception>' or '__proto__' reference.
+   * and specifies if it's a 'this', '<exception>', '<return>' or '__proto__'
+   * reference.
    */
   _setAttributes: function() {
     let ownerView = this.ownerView;
@@ -2485,7 +2492,6 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
     if (name == "this") {
       target.setAttribute("self", "");
     }
-
     else if (name == "<exception>") {
       target.setAttribute("exception", "");
     }

browser/devtools/shared/widgets/VariablesViewController.jsm

@@ -28,9 +28,13 @@ XPCOMUtils.defineLazyGetter(this, "VARIABLES_SORTING_ENABLED", () =>
   Services.prefs.getBoolPref("devtools.debugger.ui.variables-sorting-enabled")
 );
 
-const MAX_LONG_STRING_LENGTH = 200000;
+XPCOMUtils.defineLazyModuleGetter(this, "console",
+  "resource://gre/modules/devtools/Console.jsm");
 
-this.EXPORTED_SYMBOLS = ["VariablesViewController"];
+const MAX_LONG_STRING_LENGTH = 200000;
+const DBG_STRINGS_URI = "chrome://browser/locale/devtools/debugger.properties";
+
+this.EXPORTED_SYMBOLS = ["VariablesViewController", "StackFrameUtils"];
 
 
 /**
@@ -44,6 +48,7 @@ this.EXPORTED_SYMBOLS = ["VariablesViewController"];
  *        Options for configuring the controller. Supported options:
  *        - getObjectClient: callback for creating an object grip client
  *        - getLongStringClient: callback for creating a long string grip client
+ *        - getEnvironmentClient: callback for creating an environment client
  *        - releaseActor: callback for releasing an actor when it's no longer needed
  *        - overrideValueEvalMacro: callback for creating an overriding eval macro
  *        - getterOrSetterEvalMacro: callback for creating a getter/setter eval macro
@@ -54,6 +59,7 @@ function VariablesViewController(aView, aOptions) {
 
   this._getObjectClient = aOptions.getObjectClient;
   this._getLongStringClient = aOptions.getLongStringClient;
+  this._getEnvironmentClient = aOptions.getEnvironmentClient;
   this._releaseActor = aOptions.releaseActor;
 
   if (aOptions.overrideValueEvalMacro) {
@@ -131,8 +137,15 @@ VariablesViewController.prototype = {
    */
   _populateFromObject: function(aTarget, aGrip) {
     let deferred = promise.defer();
+    // Mark the specified variable as having retrieved all its properties.
+    let finish = variable => {
+      variable._retrieved = true;
+      this.view.commitHierarchy();
+      deferred.resolve();
+    };
 
-    this._getObjectClient(aGrip).getPrototypeAndProperties(aResponse => {
+    let objectClient = this._getObjectClient(aGrip);
+    objectClient.getPrototypeAndProperties(aResponse => {
       let { ownProperties, prototype } = aResponse;
       // safeGetterValues is new and isn't necessary defined on old actors
       let safeGetterValues = aResponse.safeGetterValues || {};
@@ -167,20 +180,108 @@ VariablesViewController.prototype = {
         this.addExpander(proto, prototype);
       }
 
-      // Mark the variable as having retrieved all its properties.
+      // If the object is a function we need to fetch its scope chain.
-      aTarget._retrieved = true;
+      if (aGrip.class == "Function") {
-      this.view.commitHierarchy();
+        objectClient.getScope(aResponse => {
-      deferred.resolve();
+          if (aResponse.error) {
+            console.error(aResponse.error + ": " + aResponse.message);
+            finish(aTarget);
+            return;
+          }
+          this._addVarScope(aTarget, aResponse.scope).then(() => finish(aTarget));
+        });
+      } else {
+        finish(aTarget);
+      }
     });
 
     return deferred.promise;
   },
 
+  /**
+   * Adds the scope chain elements (closures) of a function variable to the
+   * view.
+   *
+   * @param Variable aTarget
+   *        The variable where the properties will be placed into.
+   * @param Scope aScope
+   *        The lexical environment form as specified in the protocol.
+   */
+  _addVarScope: function(aTarget, aScope) {
+    let objectScopes = [];
+    let environment = aScope;
+    let funcScope = aTarget.addItem("<Closure>");
+    funcScope._target.setAttribute("scope", "");
+    funcScope._fetched = true;
+    funcScope.showArrow();
+    do {
+      // Create a scope to contain all the inspected variables.
+      let label = StackFrameUtils.getScopeLabel(environment);
+      // Block scopes have the same label, so make addItem allow duplicates.
+      let closure = funcScope.addItem(label, undefined, true);
+      closure._target.setAttribute("scope", "");
+      closure._fetched = environment.class == "Function";
+      closure.showArrow();
+      // Add nodes for every argument and every other variable in scope.
+      if (environment.bindings) {
+        this._addBindings(closure, environment.bindings);
+        funcScope._retrieved = true;
+        closure._retrieved = true;
+      } else {
+        let deferred = Promise.defer();
+        objectScopes.push(deferred.promise);
+        this._getEnvironmentClient(environment).getBindings(response => {
+          this._addBindings(closure, response.bindings);
+          funcScope._retrieved = true;
+          closure._retrieved = true;
+          deferred.resolve();
+        });
+      }
+    } while ((environment = environment.parent));
+    aTarget.expand();
+
+    return Promise.all(objectScopes).then(() => {
+      // Signal that scopes have been fetched.
+      this.view.emit("fetched", "scopes", funcScope);
+    });
+  },
+
+  /**
+   * Adds nodes for every specified binding to the closure node.
+   *
+   * @param Variable closure
+   *        The node where the bindings will be placed into.
+   * @param object bindings
+   *        The bindings form as specified in the protocol.
+   */
+  _addBindings: function(closure, bindings) {
+    for (let argument of bindings.arguments) {
+      let name = Object.getOwnPropertyNames(argument)[0];
+      let argRef = closure.addItem(name, argument[name]);
+      let argVal = argument[name].value;
+      this.addExpander(argRef, argVal);
+    }
+
+    let aVariables = bindings.variables;
+    let variableNames = Object.keys(aVariables);
+
+    // Sort all of the variables before adding them, if preferred.
+    if (VARIABLES_SORTING_ENABLED) {
+      variableNames.sort();
+    }
+    // Add the variables to the specified scope.
+    for (let name of variableNames) {
+      let varRef = closure.addItem(name, aVariables[name]);
+      let varVal = aVariables[name].value;
+      this.addExpander(varRef, varVal);
+    }
+  },
+
   /**
    * Adds an 'onexpand' callback for a variable, lazily handling
    * the addition of new properties.
    *
-   * @param Variable aVar
+   * @param Variable aTarget
    *        The variable where the properties will be placed into.
    * @param any aSource
    *        The source to use to populate the target.
@@ -254,7 +355,7 @@ VariablesViewController.prototype = {
       throw new Error("No actor grip was given for the variable.");
     }
 
-    // If the target a Variable or Property then we're fetching properties
+    // If the target is a Variable or Property then we're fetching properties.
     if (VariablesView.isVariable(aTarget)) {
       this._populateFromObject(aTarget, aSource).then(() => {
         deferred.resolve();
@@ -360,3 +461,64 @@ VariablesViewController.attach = function(aView, aOptions) {
   }
   return new VariablesViewController(aView, aOptions);
 };
+
+/**
+ * Utility functions for handling stackframes.
+ */
+let StackFrameUtils = {
+  /**
+   * Create a textual representation for the specified stack frame
+   * to display in the stackframes container.
+   *
+   * @param object aFrame
+   *        The stack frame to label.
+   */
+  getFrameTitle: function(aFrame) {
+    if (aFrame.type == "call") {
+      let c = aFrame.callee;
+      return (c.name || c.userDisplayName || c.displayName || "(anonymous)");
+    }
+    return "(" + aFrame.type + ")";
+  },
+
+  /**
+   * Constructs a scope label based on its environment.
+   *
+   * @param object aEnv
+   *        The scope's environment.
+   * @return string
+   *         The scope's label.
+   */
+  getScopeLabel: function(aEnv) {
+    let name = "";
+
+    // Name the outermost scope Global.
+    if (!aEnv.parent) {
+      name = L10N.getStr("globalScopeLabel");
+    }
+    // Otherwise construct the scope name.
+    else {
+      name = aEnv.type.charAt(0).toUpperCase() + aEnv.type.slice(1);
+    }
+
+    let label = L10N.getFormatStr("scopeLabel", name);
+    switch (aEnv.type) {
+      case "with":
+      case "object":
+        label += " [" + aEnv.object.class + "]";
+        break;
+      case "function":
+        let f = aEnv.function;
+        label += " [" +
+          (f.name || f.userDisplayName || f.displayName || "(anonymous)") +
+        "]";
+        break;
+    }
+    return label;
+  }
+};
+
+/**
+ * Localization convenience methods.
+ */
+let L10N = new ViewHelpers.L10N(DBG_STRINGS_URI);

browser/devtools/webconsole/test/browser.ini

@@ -59,6 +59,7 @@ support-files =
   test-bug-859170-longstring-hang.html
   test-bug-869003-iframe.html
   test-bug-869003-top-window.html
+  test-closures.html
   test-console-extras.html
   test-console-replaced-api.html
   test-console.html
@@ -210,6 +211,7 @@ support-files =
 [browser_webconsole_cached_autocomplete.js]
 [browser_webconsole_change_font_size.js]
 [browser_webconsole_chrome.js]
+[browser_webconsole_closure_inspection.js]
 [browser_webconsole_completion.js]
 [browser_webconsole_console_extras.js]
 [browser_webconsole_console_logging_api.js]

browser/devtools/webconsole/test/browser_webconsole_closure_inspection.js

@@ -0,0 +1,91 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// Check that inspecting a closure in the variables view sidebar works when
+// execution is paused.
+
+const TEST_URI = "http://example.com/browser/browser/devtools/webconsole/test/test-closures.html";
+
+let gWebConsole, gJSTerm, gVariablesView;
+
+function test()
+{
+  registerCleanupFunction(() => {
+    gWebConsole = gJSTerm = gVariablesView = null;
+  });
+
+  addTab(TEST_URI);
+  browser.addEventListener("load", function onLoad() {
+    browser.removeEventListener("load", onLoad, true);
+    openConsole(null, (hud) => {
+      openDebugger().then(({ toolbox, panelWin }) => {
+        let deferred = promise.defer();
+        panelWin.gThreadClient.addOneTimeListener("resumed", (aEvent, aPacket) => {
+          ok(true, "Debugger resumed");
+          deferred.resolve({ toolbox: toolbox, panelWin: panelWin });
+        });
+        return deferred.promise;
+      }).then(({ toolbox, panelWin }) => {
+        let deferred = promise.defer();
+        panelWin.once(panelWin.EVENTS.FETCHED_SCOPES, (aEvent, aPacket) => {
+          ok(true, "Scopes were fetched");
+          toolbox.selectTool("webconsole").then(() => consoleOpened(hud));
+          deferred.resolve();
+        });
+
+        let button = content.document.querySelector("button");
+        ok(button, "button element found");
+        button.click();
+
+        return deferred.promise;
+      });
+    });
+  }, true);
+}
+
+function consoleOpened(hud)
+{
+  gWebConsole = hud;
+  gJSTerm = hud.jsterm;
+  gJSTerm.execute("window.george.getName");
+
+  waitForMessages({
+    webconsole: gWebConsole,
+    messages: [{
+      text: "[object Function]",
+      category: CATEGORY_OUTPUT,
+      objects: true,
+    }],
+  }).then(onExecuteGetName);
+}
+
+function onExecuteGetName(aResults)
+{
+  let clickable = aResults[0].clickableElements[0];
+  ok(clickable, "clickable object found");
+
+  gJSTerm.once("variablesview-fetched", onGetNameFetch);
+  EventUtils.synthesizeMouse(clickable, 2, 2, {}, gWebConsole.iframeWindow)
+}
+
+function onGetNameFetch(aEvent, aVar)
+{
+  gVariablesView = aVar._variablesView;
+  ok(gVariablesView, "variables view object");
+
+  findVariableViewProperties(aVar, [
+    { name: /_pfactory/, value: "" },
+  ], { webconsole: gWebConsole }).then(onExpandClosure);
+}
+
+function onExpandClosure(aResults)
+{
+  let prop = aResults[0].matchedProp;
+  ok(prop, "matched the name property in the variables view");
+
+  gVariablesView.window.focus();
+  gJSTerm.once("sidebar-closed", finishTest);
+  EventUtils.synthesizeKey("VK_ESCAPE", {}, gVariablesView.window);
+}

browser/devtools/webconsole/test/test-closures.html

@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <meta charset='utf-8'/>
+    <title>Console Test for Closure Inspection</title>
+    <!-- Any copyright is dedicated to the Public Domain.
+         http://creativecommons.org/publicdomain/zero/1.0/ -->
+    <script type="text/javascript">
+      function injectPerson() {
+        var PersonFactory = function _pfactory(name) {
+            var foo = 10;
+            return {
+              getName: function() { return name; },
+              getFoo: function() { foo = Date.now(); return foo; }
+            };
+        };
+        window.george = new PersonFactory("George");
+        debugger;
+      }
+    </script>
+
+  </head>
+  <body>
+    <button onclick="injectPerson()">Test</button>
+  </body>
+</html>

browser/devtools/webconsole/webconsole.js

@@ -26,6 +26,7 @@ loader.lazyGetter(this, "ConsoleOutput",
                   () => require("devtools/webconsole/console-output").ConsoleOutput);
 loader.lazyGetter(this, "Messages",
                   () => require("devtools/webconsole/console-output").Messages);
+loader.lazyImporter(this, "EnvironmentClient", "resource://gre/modules/devtools/dbg-client.jsm");
 loader.lazyImporter(this, "ObjectClient", "resource://gre/modules/devtools/dbg-client.jsm");
 loader.lazyImporter(this, "VariablesView", "resource:///modules/devtools/VariablesView.jsm");
 loader.lazyImporter(this, "VariablesViewController", "resource:///modules/devtools/VariablesViewController.jsm");
@@ -3423,6 +3424,9 @@ JSTerm.prototype = {
     view.lazyAppend = this._lazyVariablesView;
 
     VariablesViewController.attach(view, {
+      getEnvironmentClient: aGrip => {
+        return new EnvironmentClient(this.hud.proxy.client, aGrip);
+      },
       getObjectClient: aGrip => {
         return new ObjectClient(this.hud.proxy.client, aGrip);
       },

browser/themes/linux/devtools/widgets.css

@@ -476,7 +476,7 @@
 /* Custom configurable/enumerable/writable or frozen/sealed/extensible
  * variables and properties */
 
-.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]):not([scope]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -530,6 +530,11 @@
   text-shadow: 0 0 8px #cfc;
 }
 
+.variable-or-property[scope]:not(:focus) > .title > .name {
+  color: #00a;
+  text-shadow: 0 0 8px #ccf;
+}
+
 /* Variables and properties tooltips */
 
 .variable-or-property > tooltip > label {

browser/themes/osx/devtools/widgets.css

@@ -476,7 +476,7 @@
 /* Custom configurable/enumerable/writable or frozen/sealed/extensible
  * variables and properties */
 
-.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]):not([scope]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -530,6 +530,11 @@
   text-shadow: 0 0 8px #cfc;
 }
 
+.variable-or-property[scope]:not(:focus) > .title > .name {
+  color: #00a;
+  text-shadow: 0 0 8px #ccf;
+}
+
 /* Variables and properties tooltips */
 
 .variable-or-property > tooltip > label {

browser/themes/windows/devtools/widgets.css

@@ -479,7 +479,7 @@
 /* Custom configurable/enumerable/writable or frozen/sealed/extensible
  * variables and properties */
 
-.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]) > .title > .name {
+.variable-or-property[non-enumerable]:not([self]):not([exception]):not([return]):not([scope]) > .title > .name {
   opacity: 0.5;
 }
 
@@ -533,6 +533,11 @@
   text-shadow: 0 0 8px #cfc;
 }
 
+.variable-or-property[scope]:not(:focus) > .title > .name {
+  color: #00a;
+  text-shadow: 0 0 8px #ccf;
+}
+
 /* Variables and properties tooltips */
 
 .variable-or-property > tooltip > label {

toolkit/components/telemetry/Histograms.json

@@ -3780,6 +3780,42 @@
     "n_buckets": "1000",
     "description": "The time (in milliseconds) that it took an 'unblackbox' request to go round trip."
   },
+  "DEVTOOLS_DEBUGGER_RDP_LOCAL_SCOPE_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took a 'scope' request to go round trip."
+  },
+  "DEVTOOLS_DEBUGGER_RDP_REMOTE_SCOPE_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took a 'scope' request to go round trip."
+  },
+  "DEVTOOLS_DEBUGGER_RDP_LOCAL_BINDINGS_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took a 'bindings' request to go round trip."
+  },
+  "DEVTOOLS_DEBUGGER_RDP_REMOTE_BINDINGS_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took a 'bindings' request to go round trip."
+  },
+  "DEVTOOLS_DEBUGGER_RDP_LOCAL_ASSIGN_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took an 'assign' request to go round trip."
+  },
+  "DEVTOOLS_DEBUGGER_RDP_REMOTE_ASSIGN_MS": {
+    "kind": "exponential",
+    "high": "10000",
+    "n_buckets": "1000",
+    "description": "The time (in milliseconds) that it took an 'assign' request to go round trip."
+  },
   "DEVTOOLS_OPTIONS_OPENED_BOOLEAN": {
     "kind": "boolean",
     "description": "How many times has the devtool's Options panel been opened?"

toolkit/devtools/client/dbg-client.jsm

@@ -15,6 +15,7 @@ this.EXPORTED_SYMBOLS = ["DebuggerTransport",
                          "RootClient",
                          "debuggerSocketConnect",
                          "LongStringClient",
+                         "EnvironmentClient",
                          "ObjectClient"];
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -1641,6 +1642,13 @@ ThreadClient.prototype = {
     this._client._eventsEnabled && this.notify(aPacket.type, aPacket);
   },
 
+  /**
+   * Return an EnvironmentClient instance for the given environment actor form.
+   */
+  environment: function(aForm) {
+    return new EnvironmentClient(this._client, aForm);
+  },
+
   /**
    * Return an instance of SourceClient for the given source actor form.
    */
@@ -1893,6 +1901,23 @@ ObjectClient.prototype = {
   }, {
     telemetry: "DISPLAYSTRING"
   }),
+
+  /**
+   * Request the scope of the object.
+   *
+   * @param aOnResponse function Called with the request's response.
+   */
+  getScope: DebuggerClient.requester({
+    type: "scope"
+  }, {
+    before: function (aPacket) {
+      if (this._grip.class !== "Function") {
+        throw new Error("scope is only valid for function grips.");
+      }
+      return aPacket;
+    },
+    telemetry: "SCOPE"
+  })
 };
 
 /**
@@ -2092,6 +2117,49 @@ BreakpointClient.prototype = {
 
 eventSource(BreakpointClient.prototype);
 
+/**
+ * Environment clients are used to manipulate the lexical environment actors.
+ *
+ * @param aClient DebuggerClient
+ *        The debugger client parent.
+ * @param aForm Object
+ *        The form sent across the remote debugging protocol.
+ */
+function EnvironmentClient(aClient, aForm) {
+  this._client = aClient;
+  this._form = aForm;
+  this.request = this._client.request;
+}
+
+EnvironmentClient.prototype = {
+
+  get actor() this._form.actor,
+  get _transport() { return this._client._transport; },
+
+  /**
+   * Fetches the bindings introduced by this lexical environment.
+   */
+  getBindings: DebuggerClient.requester({
+    type: "bindings"
+  }, {
+    telemetry: "BINDINGS"
+  }),
+
+  /**
+   * Changes the value of the identifier whose name is name (a string) to that
+   * represented by value (a grip).
+   */
+  assign: DebuggerClient.requester({
+    type: "assign",
+    name: args(0),
+    value: args(1)
+  }, {
+    telemetry: "ASSIGN"
+  })
+};
+
+eventSource(EnvironmentClient.prototype);
+
 /**
  * Connects to a debugger server socket and returns a DebuggerTransport.
  *

toolkit/devtools/server/actors/script.js

@@ -414,7 +414,6 @@ function ThreadActor(aHooks, aGlobal)
 {
   this._state = "detached";
   this._frameActors = [];
-  this._environmentActors = [];
   this._hooks = aHooks;
   this.global = aGlobal;
   this._nestedEventLoops = new EventLoopStack({
@@ -1816,7 +1815,6 @@ ThreadActor.prototype = {
     }
 
     let actor = new EnvironmentActor(aEnvironment, this);
-    this._environmentActors.push(actor);
     aPool.addActor(actor);
     aEnvironment.actor = actor;
 
@@ -2905,6 +2903,29 @@ ObjectActor.prototype = {
     this.release();
     return {};
   },
+
+  /**
+   * Handle a protocol request to provide the lexical scope of a function.
+   *
+   * @param aRequest object
+   *        The protocol request object.
+   */
+  onScope: function OA_onScope(aRequest) {
+    if (this.obj.class !== "Function") {
+      return { error: "objectNotFunction",
+               message: "scope request is only valid for object grips with a" +
+                        " 'Function' class." };
+    }
+
+    let envActor = this.threadActor.createEnvironmentActor(this.obj.environment,
+                                                           this.registeredPool);
+    if (!envActor) {
+      return { error: "notDebuggee",
+               message: "cannot access the environment of this function." };
+    }
+
+    return { from: this.actorID, scope: envActor.form() };
+  }
 };
 
 ObjectActor.prototype.requestTypes = {
@@ -2916,6 +2937,7 @@ ObjectActor.prototype.requestTypes = {
   "ownPropertyNames": ObjectActor.prototype.onOwnPropertyNames,
   "decompile": ObjectActor.prototype.onDecompile,
   "release": ObjectActor.prototype.onRelease,
+  "scope": ObjectActor.prototype.onScope,
 };
 
 
@@ -2934,6 +2956,7 @@ update(PauseScopedObjectActor.prototype, ObjectActor.prototype);
 
 update(PauseScopedObjectActor.prototype, {
   constructor: PauseScopedObjectActor,
+  actorPrefix: "pausedobj",
 
   onOwnPropertyNames:
     PauseScopedActor.withPaused(ObjectActor.prototype.onOwnPropertyNames),
@@ -2951,29 +2974,6 @@ update(PauseScopedObjectActor.prototype, {
   onParameterNames:
     PauseScopedActor.withPaused(ObjectActor.prototype.onParameterNames),
 
-  /**
-   * Handle a protocol request to provide the lexical scope of a function.
-   *
-   * @param aRequest object
-   *        The protocol request object.
-   */
-  onScope: PauseScopedActor.withPaused(function OA_onScope(aRequest) {
-    if (this.obj.class !== "Function") {
-      return { error: "objectNotFunction",
-               message: "scope request is only valid for object grips with a" +
-                        " 'Function' class." };
-    }
-
-    let envActor = this.threadActor.createEnvironmentActor(this.obj.environment,
-                                                           this.registeredPool);
-    if (!envActor) {
-      return { error: "notDebuggee",
-               message: "cannot access the environment of this function." };
-    }
-
-    return { from: this.actorID, scope: envActor.form() };
-  }),
-
   /**
    * Handle a protocol request to promote a pause-lifetime grip to a
    * thread-lifetime grip.
@@ -3004,7 +3004,6 @@ update(PauseScopedObjectActor.prototype, {
 });
 
 update(PauseScopedObjectActor.prototype.requestTypes, {
-  "scope": PauseScopedObjectActor.prototype.onScope,
   "threadGrip": PauseScopedObjectActor.prototype.onThreadGrip,
 });
 
@@ -3447,14 +3446,10 @@ EnvironmentActor.prototype = {
 
     try {
       this.obj.setVariable(aRequest.name, aRequest.value);
-    } catch (e) {
+    } catch (e if e instanceof Debugger.DebuggeeWouldRun) {
-      if (e instanceof Debugger.DebuggeeWouldRun) {
         return { error: "threadWouldRun",
                  cause: e.cause ? e.cause : "setter",
                  message: "Assigning a value would cause the debuggee to run" };
-      }
-      // This should never happen, so let it complain loudly if it does.
-      throw e;
     }
     return { from: this.actorID };
   },

toolkit/devtools/server/actors/webconsole.js

@@ -208,6 +208,33 @@ WebConsoleActor.prototype =
     this.conn = null;
   },
 
+  /**
+   * Create and return an environment actor that corresponds to the provided
+   * Debugger.Environment. This is a straightforward clone of the ThreadActor's
+   * method except that it stores the environment actor in the web console
+   * actor's pool.
+   *
+   * @param Debugger.Environment aEnvironment
+   *        The lexical environment we want to extract.
+   * @return The EnvironmentActor for aEnvironment or undefined for host
+   *         functions or functions scoped to a non-debuggee global.
+   */
+  createEnvironmentActor: function WCA_createEnvironmentActor(aEnvironment) {
+    if (!aEnvironment) {
+      return undefined;
+    }
+
+    if (aEnvironment.actor) {
+      return aEnvironment.actor;
+    }
+
+    let actor = new EnvironmentActor(aEnvironment, this);
+    this._actorPool.addActor(actor);
+    aEnvironment.actor = actor;
+
+    return actor;
+  },
+
   /**
    * Create a grip for the given value.
    *

toolkit/devtools/server/tests/unit/head_dbg.js

@@ -37,6 +37,7 @@ function testExceptionHook(ex) {
   } catch(ex) {
     return {throw: ex}
   }
+  return undefined;
 }
 
 // Convert an nsIScriptError 'aFlags' value into an appropriate string.

toolkit/devtools/server/tests/unit/test_framebindings-07.js

@@ -0,0 +1,64 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+// Test that the EnvironmentClient's getBindings() method works as expected.
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-bindings");
+
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestTabAndResume(gClient, "test-bindings", function(aResponse, aTabClient, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_banana_environment();
+    });
+  });
+  do_test_pending();
+}
+
+function test_banana_environment()
+{
+
+  gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
+    let environment = aPacket.frame.environment;
+    do_check_eq(environment.type, "function");
+
+    let parent = environment.parent;
+    do_check_eq(parent.type, "block");
+
+    let grandpa = parent.parent;
+    do_check_eq(grandpa.type, "function");
+
+    let envClient = gThreadClient.environment(environment);
+    envClient.getBindings(aResponse => {
+      do_check_eq(aResponse.bindings.arguments[0].z.value, "z");
+
+      let parentClient = gThreadClient.environment(parent);
+      parentClient.getBindings(aResponse => {
+        do_check_eq(aResponse.bindings.variables.banana3.value.class, "Function");
+
+        let grandpaClient = gThreadClient.environment(grandpa);
+        grandpaClient.getBindings(aResponse => {
+          do_check_eq(aResponse.bindings.arguments[0].y.value, "y");
+          gThreadClient.resume(() => finishClient(gClient));
+        });
+      });
+    });
+  });
+
+  gDebuggee.eval("\
+        function banana(x) {                                            \n\
+          return function banana2(y) {                                  \n\
+            return function banana3(z) {                                \n\
+              debugger;                                                 \n\
+            };                                                          \n\
+          };                                                            \n\
+        }                                                               \n\
+        banana('x')('y')('z');                                          \n\
+        ");
+}

toolkit/devtools/server/tests/unit/test_objectgrips-10.js

@@ -0,0 +1,72 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+// Test that closures can be inspected.
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-closures");
+
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestTabAndResume(gClient, "test-closures", function(aResponse, aTabClient, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_object_grip();
+    });
+  });
+  do_test_pending();
+}
+
+function test_object_grip()
+{
+  gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) {
+    let person = aPacket.frame.environment.bindings.variables.person;
+
+    do_check_eq(person.value.class, "Object");
+
+    let personClient = gThreadClient.pauseGrip(person.value);
+    personClient.getPrototypeAndProperties(aResponse => {
+      do_check_eq(aResponse.ownProperties.getName.value.class, "Function");
+
+      do_check_eq(aResponse.ownProperties.getAge.value.class, "Function");
+
+      do_check_eq(aResponse.ownProperties.getFoo.value.class, "Function");
+
+      let getNameClient = gThreadClient.pauseGrip(aResponse.ownProperties.getName.value);
+      let getAgeClient = gThreadClient.pauseGrip(aResponse.ownProperties.getAge.value);
+      let getFooClient = gThreadClient.pauseGrip(aResponse.ownProperties.getFoo.value);
+      getNameClient.getScope(aResponse => {
+        do_check_eq(aResponse.scope.bindings.arguments[0].name.value, "Bob");
+
+        getAgeClient.getScope(aResponse => {
+          do_check_eq(aResponse.scope.bindings.arguments[1].age.value, 58);
+
+          getFooClient.getScope(aResponse => {
+            do_check_eq(aResponse.scope.bindings.variables.foo.value, 10);
+
+            gThreadClient.resume(() => finishClient(gClient));
+          });
+        });
+      });
+    });
+
+  });
+
+  gDebuggee.eval("(" + function() {
+    var PersonFactory = function(name, age) {
+        var foo = 10;
+        return {
+          getName: function() { return name; },
+          getAge: function() { return age; },
+          getFoo: function() { foo = Date.now(); return foo; }
+        };
+    };
+    var person = new PersonFactory("Bob", 58);
+    debugger;
+  } + ")()");
+}

toolkit/devtools/server/tests/unit/xpcshell.ini

@@ -145,6 +145,7 @@ reason = bug 820380
 [test_objectgrips-07.js]
 [test_objectgrips-08.js]
 [test_objectgrips-09.js]
+[test_objectgrips-10.js]
 [test_interrupt.js]
 [test_stepping-01.js]
 [test_stepping-02.js]
@@ -158,6 +159,7 @@ reason = bug 820380
 [test_framebindings-04.js]
 [test_framebindings-05.js]
 [test_framebindings-06.js]
+[test_framebindings-07.js]
 [test_pause_exceptions-01.js]
 skip-if = toolkit == "gonk"
 reason = bug 820380