Bug 795173: Make Debugger.Object.prototype.evalInGlobalWithBindings pass the right 'this' value. r=jorendorff

This also adds a test for Debugger.Frame.prototype.evalWithBindings, to
make sure it puts new variables on the right object; I couldn't find any
existing tests for the non-strict case (Frame-evalWithBindings-11.js
handles the strict mode case).

js/src/jit-test/tests/debug/Frame-evalWithBindings-11.js

@@ -0,0 +1,18 @@
+// var statements in non-strict evalWithBindings code behave like non-strict direct eval.
+var g = newGlobal();
+var dbg = new Debugger(g);
+var log;
+dbg.onDebuggerStatement = function (frame) {
+  log += 'd';
+  assertEq(frame.evalWithBindings("var i = v; 42;", { v: 'inner' }).return, 42);
+};
+
+g.i = 'outer';
+log = '';
+assertEq(g.eval('debugger; i;'), 'inner');
+assertEq(log, 'd');
+
+g.j = 'outer';
+log = '';
+assertEq(g.eval('debugger; j;'), 'outer');
+assertEq(log, 'd');

js/src/jit-test/tests/debug/Object-evalInGlobal-06.js

@@ -0,0 +1,8 @@
+// Debugger.Object.prototype.evalInGlobal sets 'this' to the global.
+
+var dbg = new Debugger;
+var g1 = newGlobal();
+var g1w = dbg.addDebuggee(g1);
+
+assertEq(g1w.evalInGlobal('this').return, g1w);
+assertEq(g1w.evalInGlobalWithBindings('this', { x:42 }).return, g1w);

js/src/jsdbgapi.cpp

@@ -747,8 +747,12 @@ JS_EvaluateUCInStackFrame(JSContext *cx, JSStackFrame *fpArg,
 
     StackFrame *fp = Valueify(fpArg);
 
+    if (!ComputeThis(cx, fp))
+        return false;
+    RootedValue thisv(cx, fp->thisValue());
+
     js::AutoCompartment ac(cx, env);
-    return EvaluateInEnv(cx, env, fp, chars, length, filename, lineno, rval);
+    return EvaluateInEnv(cx, env, thisv, fp, chars, length, filename, lineno, rval);
 }
 
 JS_PUBLIC_API(JSBool)

js/src/vm/Debugger.cpp

@@ -3390,25 +3390,27 @@ DebuggerFrame_setOnPop(JSContext *cx, unsigned argc, Value *vp)
     return true;
 }
 
+/*
+ * Evaluate |chars[0..length-1]| in the environment |env|, treating that
+ * source as appearing starting at |lineno| in |filename|. Store the return
+ * value in |*rval|. Use |thisv| as the 'this' value.
+ *
+ * If |fp| is non-NULL, evaluate as for a direct eval in that frame; |env|
+ * must be either |fp|'s DebugScopeObject, or some extension of that
+ * environment; either way, |fp|'s scope is where newly declared variables
+ * go. In this case, |fp| must have a computed 'this' value, equal to |thisv|.
+ */
 JSBool
-js::EvaluateInEnv(JSContext *cx, Handle<Env*> env, StackFrame *fp, const jschar *chars,
-                  unsigned length, const char *filename, unsigned lineno, Value *rval)
+js::EvaluateInEnv(JSContext *cx, Handle<Env*> env, HandleValue thisv, StackFrame *fp,
+                  const jschar *chars, unsigned length, const char *filename, unsigned lineno,
+                  Value *rval)
 {
     assertSameCompartment(cx, env, fp);
+    JS_ASSERT_IF(fp, thisv.get() == fp->thisValue());
 
     JS_ASSERT(!IsPoisonedPtr(chars));
     SkipRoot skip(cx, &chars);
 
-    RootedValue thisv(cx);
-    if (fp) {
-        /* Execute assumes an already-computed 'this" value. */
-        if (!ComputeThis(cx, fp))
-            return false;
-        thisv = fp->thisValue();
-    } else {
-        thisv = ObjectValue(*env);
-    }
-
     /*
      * NB: This function breaks the assumption that the compiler can see all
      * calls and properly compute a static level. In practice, any non-zero
@@ -3480,9 +3482,20 @@ DebuggerGenericEval(JSContext *cx, const char *fullMethodName,
     else
         ac.construct(cx, scope);
 
-    Rooted<Env *> env(cx, fp ? GetDebugScopeForFrame(cx, fp) : scope);
-    if (!env)
-        return false;
+    RootedValue thisv(cx);
+    Rooted<Env *> env(cx);
+    if (fp) {
+        /* ExecuteInEnv requires 'fp' to have a computed 'this" value. */
+        if (!ComputeThis(cx, fp))
+            return false;
+        thisv = fp->thisValue();
+        env = GetDebugScopeForFrame(cx, fp);
+        if (!env)
+            return false;
+    } else {
+        thisv = ObjectValue(*scope);
+        env = scope;
+    }
 
     /* If evalWithBindings, create the inner environment. */
     if (bindings) {
@@ -3505,7 +3518,7 @@ DebuggerGenericEval(JSContext *cx, const char *fullMethodName,
     /* Run the code and produce the completion value. */
     Value rval;
     JS::Anchor<JSString *> anchor(stable);
-    bool ok = EvaluateInEnv(cx, env, fp, stable->chars(), stable->length(),
+    bool ok = EvaluateInEnv(cx, env, thisv, fp, stable->chars(), stable->length(),
                             "debugger eval code", 1, &rval);
     return dbg->receiveCompletionValue(ac, ok, rval, vp);
 }

js/src/vm/Debugger.h

@@ -523,8 +523,9 @@ Debugger::onNewScript(JSContext *cx, JSScript *script, GlobalObject *compileAndG
 }
 
 extern JSBool
-EvaluateInEnv(JSContext *cx, Handle<Env*> env, StackFrame *fp, const jschar *chars,
-              unsigned length, const char *filename, unsigned lineno, Value *rval);
+EvaluateInEnv(JSContext *cx, Handle<Env*> env, HandleValue thisv, StackFrame *fp,
+              const jschar *chars, unsigned length, const char *filename, unsigned lineno,
+              Value *rval);
 
 }