Bug 1663847 part 1 - Add Debugger.Script.parameterNames getter. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 11 Sep 2020 09:21:46 +0000
changeset 548294 530e5373b9acdef4943c157aa580194a6a4fd4cc
parent 548293 444f4554ea9456909908f2872d1359495d9ff8d3
child 548295 e6c6a83c4de71878ccfa1b3a39d8ace923943695
push id37776
push userbtara@mozilla.com
push dateFri, 11 Sep 2020 15:10:42 +0000
treeherdermozilla-central@b133e2d673e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1663847
milestone82.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 1663847 part 1 - Add Debugger.Script.parameterNames getter. r=tcampbell Simplifies the implementation of Debugger.Object.parameterNames (for functions) and reuses it for scripts. The test is based on the Object-parameterNames.js test with some minor changes and the documentation is based on the Object docs. Devtools code will use this in the next patch. Differential Revision: https://phabricator.services.mozilla.com/D89700
js/src/debugger/Debugger.cpp
js/src/debugger/Debugger.h
js/src/debugger/Object.cpp
js/src/debugger/Object.h
js/src/debugger/Script.cpp
js/src/doc/Debugger/Debugger.Object.md
js/src/doc/Debugger/Debugger.Script.md
js/src/jit-test/tests/debug/Script-parameterNames.js
js/src/jit-test/tests/debug/wasm-04.js
--- a/js/src/debugger/Debugger.cpp
+++ b/js/src/debugger/Debugger.cpp
@@ -170,16 +170,46 @@ bool js::IsInterpretedNonSelfHostedFunct
 }
 
 JSScript* js::GetOrCreateFunctionScript(JSContext* cx, HandleFunction fun) {
   MOZ_ASSERT(IsInterpretedNonSelfHostedFunction(fun));
   AutoRealm ar(cx, fun);
   return JSFunction::getOrCreateScript(cx, fun);
 }
 
+ArrayObject* js::GetFunctionParameterNamesArray(JSContext* cx,
+                                                HandleFunction fun) {
+  RootedValueVector names(cx);
+
+  // The default value for each argument is |undefined|.
+  if (!names.growBy(fun->nargs())) {
+    return nullptr;
+  }
+
+  if (IsInterpretedNonSelfHostedFunction(fun) && fun->nargs() > 0) {
+    RootedScript script(cx, GetOrCreateFunctionScript(cx, fun));
+    if (!script) {
+      return nullptr;
+    }
+
+    MOZ_ASSERT(fun->nargs() == script->numArgs());
+
+    PositionalFormalParameterIter fi(script);
+    for (size_t i = 0; i < fun->nargs(); i++, fi++) {
+      MOZ_ASSERT(fi.argumentSlot() == i);
+      if (JSAtom* atom = fi.name()) {
+        cx->markAtom(atom);
+        names[i].setString(atom);
+      }
+    }
+  }
+
+  return NewDenseCopiedArray(cx, names.length(), names.begin());
+}
+
 bool js::ValueToIdentifier(JSContext* cx, HandleValue v, MutableHandleId id) {
   if (!ToPropertyKey(cx, v, id)) {
     return false;
   }
   if (!JSID_IS_ATOM(id) || !IsIdentifier(JSID_TO_ATOM(id))) {
     RootedValue val(cx, v);
     ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK, val,
                      nullptr, "not an identifier");
--- a/js/src/debugger/Debugger.h
+++ b/js/src/debugger/Debugger.h
@@ -1584,16 +1584,17 @@ bool Debugger::observesGlobal(GlobalObje
   return debuggees.has(debuggee);
 }
 
 MOZ_MUST_USE bool ReportObjectRequired(JSContext* cx);
 
 JSObject* IdVectorToArray(JSContext* cx, Handle<IdVector> ids);
 bool IsInterpretedNonSelfHostedFunction(JSFunction* fun);
 JSScript* GetOrCreateFunctionScript(JSContext* cx, HandleFunction fun);
+ArrayObject* GetFunctionParameterNamesArray(JSContext* cx, HandleFunction fun);
 bool ValueToIdentifier(JSContext* cx, HandleValue v, MutableHandleId id);
 bool ValueToStableChars(JSContext* cx, const char* fnname, HandleValue value,
                         JS::AutoStableStringChars& stableChars);
 bool ParseEvalOptions(JSContext* cx, HandleValue value, EvalOptions& options);
 
 Result<Completion> DebuggerGenericEval(
     JSContext* cx, const mozilla::Range<const char16_t> chars,
     HandleObject bindings, const EvalOptions& options, Debugger* dbg,
--- a/js/src/debugger/Object.cpp
+++ b/js/src/debugger/Object.cpp
@@ -348,38 +348,24 @@ bool DebuggerObject::CallData::displayNa
 }
 
 bool DebuggerObject::CallData::parameterNamesGetter() {
   if (!object->isDebuggeeFunction()) {
     args.rval().setUndefined();
     return true;
   }
 
-  Rooted<StringVector> names(cx, StringVector(cx));
-  if (!DebuggerObject::getParameterNames(cx, object, &names)) {
-    return false;
-  }
-
-  RootedArrayObject obj(cx, NewDenseFullyAllocatedArray(cx, names.length()));
-  if (!obj) {
+  RootedFunction referent(cx, &object->referent()->as<JSFunction>());
+
+  ArrayObject* arr = GetFunctionParameterNamesArray(cx, referent);
+  if (!arr) {
     return false;
   }
 
-  obj->ensureDenseInitializedLength(cx, 0, names.length());
-  for (size_t i = 0; i < names.length(); ++i) {
-    Value v;
-    if (names[i]) {
-      v = StringValue(names[i]);
-    } else {
-      v = UndefinedValue();
-    }
-    obj->setDenseElement(i, v);
-  }
-
-  args.rval().setObject(*obj);
+  args.rval().setObject(*arr);
   return true;
 }
 
 bool DebuggerObject::CallData::scriptGetter() {
   Debugger* dbg = object->owner();
 
   if (!referent->is<JSFunction>()) {
     args.rval().setUndefined();
@@ -1762,55 +1748,16 @@ double DebuggerObject::promiseLifetime()
 
 double DebuggerObject::promiseTimeToResolution() const {
   MOZ_ASSERT(promiseState() != JS::PromiseState::Pending);
 
   return promise()->timeToResolution();
 }
 
 /* static */
-bool DebuggerObject::getParameterNames(JSContext* cx,
-                                       HandleDebuggerObject object,
-                                       MutableHandle<StringVector> result) {
-  MOZ_ASSERT(object->isDebuggeeFunction());
-
-  RootedFunction referent(cx, &object->referent()->as<JSFunction>());
-
-  if (!result.growBy(referent->nargs())) {
-    return false;
-  }
-  if (IsInterpretedNonSelfHostedFunction(referent)) {
-    RootedScript script(cx, GetOrCreateFunctionScript(cx, referent));
-    if (!script) {
-      return false;
-    }
-
-    MOZ_ASSERT(referent->nargs() == script->numArgs());
-
-    if (referent->nargs() > 0) {
-      PositionalFormalParameterIter fi(script);
-      for (size_t i = 0; i < referent->nargs(); i++, fi++) {
-        MOZ_ASSERT(fi.argumentSlot() == i);
-        JSAtom* atom = fi.name();
-        if (atom) {
-          cx->markAtom(atom);
-        }
-        result[i].set(atom);
-      }
-    }
-  } else {
-    for (size_t i = 0; i < referent->nargs(); i++) {
-      result[i].set(nullptr);
-    }
-  }
-
-  return true;
-}
-
-/* static */
 bool DebuggerObject::getBoundTargetFunction(
     JSContext* cx, HandleDebuggerObject object,
     MutableHandleDebuggerObject result) {
   MOZ_ASSERT(object->isBoundFunction());
 
   RootedFunction referent(cx, &object->referent()->as<JSFunction>());
   Debugger* dbg = object->owner();
 
--- a/js/src/debugger/Object.h
+++ b/js/src/debugger/Object.h
@@ -44,19 +44,16 @@ class DebuggerObject : public NativeObje
                                 HandleNativeObject debugger);
 
   void trace(JSTracer* trc);
 
   // Properties
   static MOZ_MUST_USE bool getClassName(JSContext* cx,
                                         HandleDebuggerObject object,
                                         MutableHandleString result);
-  static MOZ_MUST_USE bool getParameterNames(
-      JSContext* cx, HandleDebuggerObject object,
-      MutableHandle<StringVector> result);
   static MOZ_MUST_USE bool getBoundTargetFunction(
       JSContext* cx, HandleDebuggerObject object,
       MutableHandleDebuggerObject result);
   static MOZ_MUST_USE bool getBoundThis(JSContext* cx,
                                         HandleDebuggerObject object,
                                         MutableHandleValue result);
   static MOZ_MUST_USE bool getBoundArguments(JSContext* cx,
                                              HandleDebuggerObject object,
--- a/js/src/debugger/Script.cpp
+++ b/js/src/debugger/Script.cpp
@@ -213,16 +213,17 @@ struct MOZ_STACK_CLASS DebuggerScript::C
     return true;
   }
 
   bool getIsGeneratorFunction();
   bool getIsAsyncFunction();
   bool getIsFunction();
   bool getIsModule();
   bool getDisplayName();
+  bool getParameterNames();
   bool getUrl();
   bool getStartLine();
   bool getStartColumn();
   bool getLineCount();
   bool getSource();
   bool getSourceStart();
   bool getSourceLength();
   bool getMainOffset();
@@ -317,16 +318,36 @@ bool DebuggerScript::CallData::getDispla
   RootedValue namev(cx, StringValue(name));
   if (!dbg->wrapDebuggeeValue(cx, &namev)) {
     return false;
   }
   args.rval().set(namev);
   return true;
 }
 
+bool DebuggerScript::CallData::getParameterNames() {
+  if (!ensureScriptMaybeLazy()) {
+    return false;
+  }
+
+  RootedFunction fun(cx, referent.as<BaseScript*>()->function());
+  if (!fun) {
+    args.rval().setUndefined();
+    return true;
+  }
+
+  ArrayObject* arr = GetFunctionParameterNamesArray(cx, fun);
+  if (!arr) {
+    return false;
+  }
+
+  args.rval().setObject(*arr);
+  return true;
+}
+
 bool DebuggerScript::CallData::getUrl() {
   if (!ensureScriptMaybeLazy()) {
     return false;
   }
 
   Rooted<BaseScript*> script(cx, referent.as<BaseScript*>());
 
   if (script->filename()) {
@@ -2314,16 +2335,17 @@ bool DebuggerScript::construct(JSContext
 }
 
 const JSPropertySpec DebuggerScript::properties_[] = {
     JS_DEBUG_PSG("isGeneratorFunction", getIsGeneratorFunction),
     JS_DEBUG_PSG("isAsyncFunction", getIsAsyncFunction),
     JS_DEBUG_PSG("isFunction", getIsFunction),
     JS_DEBUG_PSG("isModule", getIsModule),
     JS_DEBUG_PSG("displayName", getDisplayName),
+    JS_DEBUG_PSG("parameterNames", getParameterNames),
     JS_DEBUG_PSG("url", getUrl),
     JS_DEBUG_PSG("startLine", getStartLine),
     JS_DEBUG_PSG("startColumn", getStartColumn),
     JS_DEBUG_PSG("lineCount", getLineCount),
     JS_DEBUG_PSG("source", getSource),
     JS_DEBUG_PSG("sourceStart", getSourceStart),
     JS_DEBUG_PSG("sourceLength", getSourceLength),
     JS_DEBUG_PSG("mainOffset", getMainOffset),
--- a/js/src/doc/Debugger/Debugger.Object.md
+++ b/js/src/doc/Debugger/Debugger.Object.md
@@ -113,39 +113,39 @@ assigned to <i>a</i>. For example:
 function h() {
   var i = function() {};    // display name: h/i
   f(function () {});        // display name: h/<
 }
 var s = f(function () {});  // display name: s<
 ```
 
 ### `parameterNames`
-If the referent is a debuggee function, the names of the its parameters,
+If the referent is a debuggee function, the names of its parameters,
 as an array of strings. If the referent is not a debuggee function, or
 not a function at all, this is `undefined`.
 
 If the referent is a host function for which parameter names are not
 available, return an array with one element per parameter, each of which
 is `undefined`.
 
 If the referent is a function proxy, return an empty array.
 
-If the referent uses destructuring parameters, then the array's elements
-reflect the structure of the parameters. For example, if the referent is
-a function declared in this way:
+If the function uses destructuring parameters, the corresponding array elements
+are `undefined`. For example, if the referent is a function declared in this
+way:
 
 ```js
 function f(a, [b, c], {d, e:f}) { ... }
 ```
 
 then this `Debugger.Object` instance's `parameterNames` property would
 have the value:
 
 ```js
-["a", ["b", "c"], {d:"d", e:"f"}]
+["a", undefined, undefined]
 ```
 
 ### `script`
 If the referent is a function that is debuggee code, this is that
 function's script, as a [`Debugger.Script`][script] instance. If the
 referent is a function proxy or not debuggee code, this is `undefined`.
 
 ### `environment`
--- a/js/src/doc/Debugger/Debugger.Script.md
+++ b/js/src/doc/Debugger/Debugger.Script.md
@@ -134,16 +134,38 @@ function h() {
   var i = function() {};    // display name: h/i
   f(function () {});        // display name: h/<
 }
 var s = f(function () {});  // display name: s<
 ```
 
 **If the instance refers to WebAssembly code**, throw a `TypeError`.
 
+### `parameterNames`
+**If the instance refers to a `JSScript`**, the names of its parameters,
+as an array of strings. If the script is not a function script this is
+`undefined`.
+
+If the function uses destructuring parameters, the corresponding array elements
+are `undefined`. For example, if the referent is a function script declared in this
+way:
+
+```js
+function f(a, [b, c], {d, e:f}) { ... }
+```
+
+then this `Debugger.Script` instance's `parameterNames` property would
+have the value:
+
+```js
+["a", undefined, undefined]
+```
+
+**If the instance refers to WebAssembly code**, throw a `TypeError`.
+
 ### `url`
 **If the instance refers to a `JSScript`**, the filename or URL from which
 this script's code was loaded. For scripts created by `eval` or the
 `Function` constructor, this may be a synthesized filename, starting with a
 valid URL and followed by information tracking how the code was introduced
 into the system; the entire string is not a valid URL. For
 `Function.prototype`'s script, this is `null`. If this `Debugger.Script`'s
 `source` property is non-`null`, then this is equal to `source.url`.
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-parameterNames.js
@@ -0,0 +1,36 @@
+load(libdir + "asserts.js");
+
+var g = newGlobal({newCompartment: true});
+var dbg = new Debugger;
+var gDO = dbg.addDebuggee(g);
+
+function check(expr, expected) {
+  let completion = gDO.executeInGlobal(expr);
+  if (completion.throw)
+    throw completion.throw.unsafeDereference();
+
+  let fn = completion.return;
+  if (expected === undefined)
+    assertEq(fn.script.parameterNames, undefined);
+  else
+    assertDeepEq(fn.script.parameterNames, expected);
+}
+
+check('(function () {})', []);
+check('(function (x) {})', ["x"]);
+check('(function (x = 1) {})', ["x"]);
+check('(function (a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x,y,z) {})',
+      ["a","b","c","d","e","f","g","h","i","j","k","l","m",
+       "n","o","p","q","r","s","t","u","v","w","x","y","z"]);
+check('(function (a, [b, c], {d, e:f}) { })',
+      ["a", undefined, undefined]);
+check('(async function (a, b, c) {})', ["a", "b", "c"]);
+check('(async function* (d, e, f) {})', ["d", "e", "f"]);
+
+// Non-function scripts have |undefined| parameterNames.
+var log = [];
+dbg.onEnterFrame = function(frame) {
+  log.push(frame.script.parameterNames);
+};
+g.evaluate("function foo(a, b) { return eval('1'); }; foo();");
+assertDeepEq(log, [undefined, ["a", "b"], undefined]); // global, function, eval
--- a/js/src/jit-test/tests/debug/wasm-04.js
+++ b/js/src/jit-test/tests/debug/wasm-04.js
@@ -11,16 +11,17 @@ var s;
 dbg.onNewScript = (script) => {
   s = script;
 }
 
 g.eval(`o = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary('(module (func) (export "" (func 0)))')));`);
 assertEq(s.format, "wasm");
 
 assertThrowsInstanceOf(() => s.displayName, Error);
+assertThrowsInstanceOf(() => s.parameterNames, Error);
 assertThrowsInstanceOf(() => s.url, Error);
 assertThrowsInstanceOf(() => s.sourceStart, Error);
 assertThrowsInstanceOf(() => s.sourceLength, Error);
 assertThrowsInstanceOf(() => s.global, Error);
 assertThrowsInstanceOf(() => s.getChildScripts(), Error);
 assertThrowsInstanceOf(() => s.getAllOffsets(), Error);
 assertThrowsInstanceOf(() => s.getBreakpoint(0), Error);
 assertThrowsInstanceOf(() => s.getOffsetsCoverage(), Error);