Bug 1384259: Remove unneeded checks from DebuggerFrame_getScript. r=ttromey
authorJim Blandy <jimb@mozilla.com>
Tue, 25 Jul 2017 12:38:17 -0700
changeset 419983 71511633aae746122e6345797486250dfe292503
parent 419982 1db44c2e79032184d3fb96d34e84052aa73b603a
child 419984 e9224528ba04e008087766d6bc2d0e10eea4c195
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttromey
bugs1384259
milestone56.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 1384259: Remove unneeded checks from DebuggerFrame_getScript. r=ttromey The code first calls AbstractFramePtr::isFunctionFrame, and if that's true, checks the callee with JSFunction::isInterpreted. There's no way to get an isFunctionFrame whose callee isn't isInterpreted, so the check is unnecessary.
js/src/jit-test/tests/debug/Frame-live-04.js
js/src/jit-test/tests/debug/Frame-live-05.js
js/src/jit-test/tests/debug/Frame-script-environment-nondebuggee.js
js/src/vm/Debugger.cpp
--- a/js/src/jit-test/tests/debug/Frame-live-04.js
+++ b/js/src/jit-test/tests/debug/Frame-live-04.js
@@ -1,10 +1,12 @@
 // frame.live is false for frames discarded during uncatchable error unwinding.
 
+load(libdir + 'asserts.js');
+
 var g = newGlobal();
 var dbg = Debugger(g);
 var hits = 0;
 var snapshot;
 dbg.onDebuggerStatement = function (frame) {
     var stack = [];
     for (var f = frame; f; f = f.older) {
         if (f.type === "call" && f.script !== null)
@@ -18,10 +20,12 @@ dbg.onDebuggerStatement = function (fram
 };
 
 g.eval("function z() { debugger; }");
 g.eval("function y() { z(); }");
 g.eval("function x() { y(); }");
 assertEq(g.eval("debugger; 'ok';"), "ok");
 assertEq(hits, 2);
 assertEq(snapshot.length, 3);
-for (var i = 0; i < snapshot.length; i++)
+for (var i = 0; i < snapshot.length; i++) {
     assertEq(snapshot[i].live, false);
+    assertThrowsInstanceOf(() => frame.script, Error);
+}
--- a/js/src/jit-test/tests/debug/Frame-live-05.js
+++ b/js/src/jit-test/tests/debug/Frame-live-05.js
@@ -1,10 +1,12 @@
 // frame.live is false for frames removed after their compartments stopped being debuggees.
 
+load(libdir + 'asserts.js');
+
 var g1 = newGlobal();
 var g2 = newGlobal();
 var dbg = Debugger(g1, g2);
 var hits = 0;
 var snapshot = [];
 dbg.onDebuggerStatement = function (frame) {
     if (hits++ === 0) {
         assertEq(frame.eval("x();"), null);
@@ -20,10 +22,12 @@ dbg.onDebuggerStatement = function (fram
 
 g1.eval("function z() { debugger; }");
 g2.z = g1.z;
 g2.eval("function y() { z(); }");
 g2.eval("function x() { y(); }");
 assertEq(g2.eval("debugger; 'ok';"), "ok");
 assertEq(hits, 2);
 assertEq(snapshot.length, 3);
-for (var i = 0; i < snapshot.length; i++)
+for (var i = 0; i < snapshot.length; i++) {
     assertEq(snapshot[i].live, false);
+    assertThrowsInstanceOf(() => frame.script, Error);
+}
--- a/js/src/jit-test/tests/debug/Frame-script-environment-nondebuggee.js
+++ b/js/src/jit-test/tests/debug/Frame-script-environment-nondebuggee.js
@@ -8,17 +8,17 @@ var dbg = new Debugger;
 var log;
 dbg.onDebuggerStatement = function (frame) {
   log += frame.type;
   // Initially, 'frame' is a debuggee frame, and we should be able to see its script and environment.
   assertEq(frame.script instanceof Debugger.Script, true);
   assertEq(frame.environment instanceof Debugger.Environment, true);
 
   // If we make g no longer a debuggee, then trying to touch the frame at
-  // all show throw.
+  // all should throw.
   dbg.removeDebuggee(g);
   assertThrowsInstanceOf(() => frame.script, Error);
   assertThrowsInstanceOf(() => frame.environment, Error);
 }
 
 g.eval('function f() { debugger; }');
 
 log = '';
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -8780,38 +8780,39 @@ static bool
 DebuggerFrame_getScript(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_FRAME(cx, argc, vp, "get script", args, thisobj, frame);
     Debugger* debug = Debugger::fromChildJSObject(thisobj);
 
     RootedObject scriptObject(cx);
     if (frame.isFunctionFrame()) {
         RootedFunction callee(cx, frame.callee());
-        if (callee->isInterpreted()) {
-            RootedScript script(cx, callee->nonLazyScript());
-            scriptObject = debug->wrapScript(cx, script);
-            if (!scriptObject)
-                return false;
-        }
+        MOZ_ASSERT(callee->isInterpreted());
+        RootedScript script(cx, callee->nonLazyScript());
+        scriptObject = debug->wrapScript(cx, script);
+        if (!scriptObject)
+            return false;
     } else if (frame.isWasmDebugFrame()) {
         RootedWasmInstanceObject instance(cx, frame.wasmInstance()->object());
         scriptObject = debug->wrapWasmScript(cx, instance);
         if (!scriptObject)
             return false;
     } else {
         /*
          * We got eval, JS_Evaluate*, or JS_ExecuteScript non-function script
          * frames.
          */
         RootedScript script(cx, frame.script());
         scriptObject = debug->wrapScript(cx, script);
         if (!scriptObject)
             return false;
     }
-    args.rval().setObjectOrNull(scriptObject);
+
+    MOZ_ASSERT(scriptObject);
+    args.rval().setObject(*scriptObject);
     return true;
 }
 
 /* static */ bool
 DebuggerFrame::offsetGetter(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGGER_FRAME(cx, argc, vp, "get offset", args, frame);