Bug 863089 - replace Debugger.Script.getOffsetLine with getOffsetLocation; r=fitzgen

devtools/server/actors/common.js

@@ -463,31 +463,6 @@ GeneratedLocation.prototype = {
 
 exports.GeneratedLocation = GeneratedLocation;
 
-// TODO bug 863089: use Debugger.Script.prototype.getOffsetColumn when it is
-// implemented.
-exports.getOffsetColumn = function getOffsetColumn(aOffset, aScript) {
-  let bestOffsetMapping = null;
-  for (let offsetMapping of aScript.getAllColumnOffsets()) {
-    if (!bestOffsetMapping ||
-        (offsetMapping.offset <= aOffset &&
-         offsetMapping.offset > bestOffsetMapping.offset)) {
-      bestOffsetMapping = offsetMapping;
-    }
-  }
-
-  if (!bestOffsetMapping) {
-    // XXX: Try not to completely break the experience of using the debugger for
-    // the user by assuming column 0. Simultaneously, report the error so that
-    // there is a paper trail if the assumption is bad and the debugging
-    // experience becomes wonky.
-    reportError(new Error("Could not find a column for offset " + aOffset
-                          + " in the script " + aScript));
-    return 0;
-  }
-
-  return bestOffsetMapping.columnNumber;
-}
-
 /**
  * A method decorator that ensures the actor is in the expected state before
  * proceeding. If the actor is not in the expected state, the decorated method

devtools/server/actors/script.js

@@ -3633,7 +3633,7 @@ function hackDebugger(Debugger) {
     configurable: true,
     get: function() {
       if (this.script) {
-        return this.script.getOffsetLine(this.offset);
+        return this.script.getOffsetLocation(this.offset).lineNumber;
       } else {
         return null;
       }

devtools/server/actors/utils/TabSources.js

@@ -9,7 +9,7 @@ const Services = require("Services");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 const { dbg_assert, fetch } = DevToolsUtils;
 const EventEmitter = require("devtools/shared/event-emitter");
-const { OriginalLocation, GeneratedLocation, getOffsetColumn } = require("devtools/server/actors/common");
+const { OriginalLocation, GeneratedLocation } = require("devtools/server/actors/common");
 const { resolve } = require("promise");
 
 loader.lazyRequireGetter(this, "SourceActor", "devtools/server/actors/script", true);
@@ -548,10 +548,12 @@ TabSources.prototype = {
     if (!aFrame || !aFrame.script) {
       return new GeneratedLocation();
     }
+    let {lineNumber, columnNumber} =
+        aFrame.script.getOffsetLocation(aFrame.offset);
     return new GeneratedLocation(
       this.createNonSourceMappedActor(aFrame.script.source),
-      aFrame.script.getOffsetLine(aFrame.offset),
-      getOffsetColumn(aFrame.offset, aFrame.script)
+      lineNumber,
+      columnNumber
     );
   },
 

devtools/server/tests/unit/test_blackboxing-01.js

@@ -60,7 +60,7 @@ const testBlackBox = Task.async(function* () {
   yield runTest(
     function onSteppedLocation(aLocation) {
       do_check_eq(aLocation.source.url, SOURCE_URL);
-      do_check_eq(aLocation.line, 3);
+      do_check_eq(aLocation.line, 4);
     },
     function onDebuggerStatementFrames(aFrames) {
       for (let f of aFrames) {

js/examples/jorendb.js

@@ -62,7 +62,7 @@ Debugger.Frame.prototype.frameDescription = function frameDescription() {
 
 Debugger.Frame.prototype.positionDescription = function positionDescription() {
     if (this.script) {
-        var line = this.script.getOffsetLine(this.offset);
+        var line = this.script.getOffsetLocation(this.offset).lineNumber;
         if (this.script.url)
             return this.script.url + ":" + line;
         return "line " + line;
@@ -83,7 +83,7 @@ Object.defineProperty(Debugger.Frame.prototype, "line", {
         enumerable: false,
         get: function() {
             if (this.script)
-                return this.script.getOffsetLine(this.offset);
+                return this.script.getOffsetLocation(this.offset).lineNumber;
             else
                 return null;
         }

js/src/doc/Debugger/Debugger.Script.md

@@ -209,9 +209,14 @@ methods of other kinds of objects.
     points to source line <i>line</i>. If the script contains no executable
     code at that line, the array returned is empty.
 
-<code>getOffsetLine(<i>offset</i>)</code>
-:   Return the source code line responsible for the bytecode at
-    <i>offset</i> in this script.
+<code>getOffsetLocation(<i>offset</i>)</code>
+:   Return an object describing the source code location responsible for the
+    bytecode at <i>offset</i> in this script.  The object has the
+    following properties:
+
+    * lineNumber: the line number for which offset is an entry point
+
+    * columnNumber: the column number for which offset is an entry point
 
 `getOffsetsCoverage()`:
 :   Return `null` or an array which contains informations about the coverage of

js/src/jit-test/tests/debug/Frame-onStep-lines-01.js

@@ -13,11 +13,11 @@ var offsets;
 dbg.onDebuggerStatement = function (frame) {
     var script = frame.script;
     offsets = script.getAllOffsets();
-    print("debugger line: " + script.getOffsetLine(frame.offset));
+    print("debugger line: " + script.getOffsetLocation(frame.offset).lineNumber);
     print("original lines: " + uneval(Object.keys(offsets)));
     if (doSingleStep) {
 	frame.onStep = function onStepHandler() {
-	    var line = script.getOffsetLine(this.offset);
+	    var line = script.getOffsetLocation(this.offset).lineNumber;
 	    delete offsets[line];
 	};
     }

js/src/jit-test/tests/debug/Script-getLineOffsets-02.js

@@ -12,7 +12,7 @@ dbg.onDebuggerStatement = function (frame) {
         var offs = s.getLineOffsets(g.line0 + line);
         var h = handler(line);
         for (var i = 0; i < offs.length; i++) {
-            assertEq(s.getOffsetLine(offs[i]), g.line0 + line);
+            assertEq(s.getOffsetLocation(offs[i]).lineNumber, g.line0 + line);
             s.setBreakpoint(offs[i], h);
         }
     }

js/src/jit-test/tests/debug/Script-getLineOffsets-03.js

@@ -10,7 +10,7 @@ dbg.onDebuggerStatement = function (frame) {
     var lineno = g.line0 + 2;
     var offs = s.getLineOffsets(lineno);
     for (var i = 0; i < offs.length; i++) {
-        assertEq(s.getOffsetLine(offs[i]), lineno);
+        assertEq(s.getOffsetLocation(offs[i]).lineNumber, lineno);
         s.setBreakpoint(offs[i], {hit: function () { log += 'B'; }});
     }
     log += 'A';

js/src/jit-test/tests/debug/Script-getLineOffsets-04.js

@@ -9,7 +9,7 @@ dbg.onDebuggerStatement = function (frame) {
     var lineno = g.line0 + where;
     var offs = s.getLineOffsets(lineno);
     for (var i = 0; i < offs.length; i++) {
-        assertEq(s.getOffsetLine(offs[i]), lineno);
+        assertEq(s.getOffsetLocation(offs[i]).lineNumber, lineno);
         s.setBreakpoint(offs[i], {hit: function () { g.log += 'B'; }});
     }
     g.log += 'A';

js/src/jit-test/tests/debug/Script-getLineOffsets-05.js

@@ -10,14 +10,14 @@ dbg.onDebuggerStatement = function (frame) {
     lineno = g.line0 + where;
     offs = s.getLineOffsets(lineno);
     for (var i = 0; i < offs.length; i++) {
-        assertEq(s.getOffsetLine(offs[i]), lineno);
+        assertEq(s.getOffsetLocation(offs[i]).lineNumber, lineno);
         s.setBreakpoint(offs[i], {hit: function () { g.log += 'B'; }});
     }
 
     lineno++;
     offs = s.getLineOffsets(lineno);
     for (var i = 0; i < offs.length; i++) {
-        assertEq(s.getOffsetLine(offs[i]), lineno);
+        assertEq(s.getOffsetLocation(offs[i]).lineNumber, lineno);
         s.setBreakpoint(offs[i], {hit: function () { g.log += 'C'; }});
     }
 

js/src/jit-test/tests/debug/Script-getLineOffsets-06.js

@@ -10,7 +10,7 @@ dbg.onDebuggerStatement = function (frame) {
     var lineno = g.line0 + where;
     offs = s.getLineOffsets(lineno);
     for (var i = 0; i < offs.length; i++) {
-        assertEq(s.getOffsetLine(offs[i]), lineno);
+        assertEq(s.getOffsetLocation(offs[i]).lineNumber, lineno);
         s.setBreakpoint(offs[i], {hit: function (frame) { g.log += 'B'; }});
     }
     g.log += 'A';

js/src/jit-test/tests/debug/Script-getOffsetLine-01.js

@@ -1,11 +1,11 @@
-// Basic getOffsetLine test, using Error.lineNumber as the gold standard.
+// Basic getOffsetLocation test, using Error.lineNumber as the gold standard.
 
 var g = newGlobal();
 var dbg = Debugger(g);
 var hits;
 dbg.onDebuggerStatement = function (frame) {
     var knownLine = frame.eval("line").return;
-    assertEq(frame.script.getOffsetLine(frame.offset), knownLine);
+    assertEq(frame.script.getOffsetLocation(frame.offset).lineNumber, knownLine);
     hits++;
 };
 

js/src/jit-test/tests/debug/Script-getOffsetLine-02.js

@@ -1,4 +1,4 @@
-// Frame.script/offset and Script.getOffsetLine work in non-top frames.
+// Frame.script/offset and Script.getOffsetLocation work in non-top frames.
 
 var g = newGlobal();
 var dbg = Debugger(g);
@@ -6,7 +6,7 @@ var hits = 0;
 dbg.onDebuggerStatement = function (frame) {
     var a = [];
     for (; frame.type == "call"; frame = frame.older)
-        a.push(frame.script.getOffsetLine(frame.offset) - g.line0);
+        a.push(frame.script.getOffsetLocation(frame.offset).lineNumber - g.line0);
     assertEq(a.join(","), "1,2,3,4");
     hits++;
 };

js/src/jit-test/tests/debug/Script-getOffsetLocation.js

@@ -0,0 +1,27 @@
+// getOffsetLocation agrees with getAllColumnOffsets
+
+var global = newGlobal();
+Debugger(global).onDebuggerStatement = function (frame) {
+    var script = frame.eval("f").return.script;
+    script.getAllColumnOffsets().forEach(function (entry) {
+        var {lineNumber, columnNumber, offset} = entry;
+        var location = script.getOffsetLocation(offset);
+        assertEq(location.lineNumber, lineNumber);
+        assertEq(location.columnNumber, columnNumber);
+    });
+};
+
+function test(body) {
+  print("Test: " + body);
+  global.eval(`function f(n) { ${body} } debugger;`);
+  global.f(3);
+}
+
+test("for (var i = 0; i < n; ++i) ;");
+test("var w0,x1=3,y2=4,z3=9");
+test("print(n),print(n),print(n)");
+test("var o={a:1,b:2,c:3}");
+test("var a=[1,2,n]");
+
+global.eval("function ppppp() { return 1; }");
+test("1 && ppppp(ppppp()) && new Error()");

js/src/jit-test/tests/debug/Source-introductionScript-01.js

@@ -32,7 +32,7 @@ log = '';
 g.evaluate('debugger; eval("\\n\\ndebugger;");', { lineNumber: 1812 });
 assertEq(log, 'de1de2');
 assertEq(introduced.introductionScript, introducer);
-assertEq(introducer.getOffsetLine(introduced.introductionOffset), 1812);
+assertEq(introducer.getOffsetLocation(introduced.introductionOffset).lineNumber, 1812);
 
 // Indirect eval, while the frame is live.
 dbg.onDebuggerStatement = function (frame) {
@@ -60,7 +60,7 @@ log = '';
 g.evaluate('debugger; (0,eval)("\\n\\ndebugger;");', { lineNumber: 1066 });
 assertEq(log, 'de1de2');
 assertEq(introduced.introductionScript, introducer);
-assertEq(introducer.getOffsetLine(introduced.introductionOffset), 1066);
+assertEq(introducer.getOffsetLocation(introduced.introductionOffset).lineNumber, 1066);
 
 // Function constructor.
 dbg.onDebuggerStatement = function (frame) {
@@ -71,8 +71,8 @@ dbg.onDebuggerStatement = function (frame) {
     log += 'i';
     var source = frame.script.source;
     assertEq(source.introductionScript, outerScript);
-    assertEq(outerScript.getOffsetLine(source.introductionOffset),
-             outerScript.getOffsetLine(outerOffset));
+    assertEq(outerScript.getOffsetLocation(source.introductionOffset).lineNumber,
+             outerScript.getOffsetLocation(outerOffset).lineNumber);
   };
 };
 log = '';
@@ -91,7 +91,7 @@ var fDO = gDO.executeInGlobal('debugger; Function("origami;")', { lineNumber: 16
 var source = fDO.script.source;
 assertEq(log, 'F2');
 assertEq(source.introductionScript, introducer);
-assertEq(introducer.getOffsetLine(source.introductionOffset), 1685);
+assertEq(introducer.getOffsetLocation(source.introductionOffset).lineNumber, 1685);
 
 // If the introduction script is in a different global from the script it
 // introduced, we don't record it.

js/src/jit-test/tests/debug/Source-introductionScript-02.js

@@ -15,7 +15,7 @@ function outerHandler(frame) {
     var source = frame.script.source;
     var introScript = source.introductionScript;
     assertEq(introScript, outerScript);
-    assertEq(introScript.getOffsetLine(source.introductionOffset), 1234);
+    assertEq(introScript.getOffsetLocation(source.introductionOffset).lineNumber, 1234);
   };
 };
 

js/src/jit-test/tests/debug/breakpoint-01.js

@@ -11,7 +11,7 @@ var handler = {
 var dbg = Debugger(g);
 dbg.onDebuggerStatement = function (frame) {
     g.s += '0';
-    var line0 = frame.script.getOffsetLine(frame.offset);
+    var line0 = frame.script.getOffsetLocation(frame.offset).lineNumber;
     var offs = frame.script.getLineOffsets(line0 + 2);
     for (var i = 0; i < offs.length; i++)
         frame.script.setBreakpoint(offs[i], handler);

js/src/jit-test/tests/debug/surfaces-offsets.js

@@ -6,27 +6,27 @@ var g = newGlobal();
 var dbg = Debugger(g);
 var hits;
 dbg.onDebuggerStatement = function (frame) {
-    assertEq(frame.script.getOffsetLine(frame.offset), g.line);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(String(frame.offset)); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(Object(frame.offset)); }, Error);
+    assertEq(frame.script.getOffsetLocation(frame.offset).lineNumber, g.line);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(String(frame.offset)).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(Object(frame.offset)).lineNumber; }, Error);
 
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(-1); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(1000000); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(0.25); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(+Infinity); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(-Infinity); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(NaN); }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(-1).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(1000000).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(0.25).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(+Infinity).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(-Infinity).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(NaN).lineNumber; }, Error);
 
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(false); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(true); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(undefined); }, Error);
-    assertThrowsInstanceOf(function () { frame.script.getOffsetLine(); }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(false).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(true).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation(undefined).lineNumber; }, Error);
+    assertThrowsInstanceOf(function () { frame.script.getOffsetLocation().lineNumber; }, Error);
 
     // We assume that at least one whole number between 0 and frame.offset is invalid.
     assertThrowsInstanceOf(
         function () {
             for (var i = 0; i < frame.offset; i++)
-                frame.script.getOffsetLine(i);
+                frame.script.getOffsetLocation(i).lineNumber;
         },
         Error);
 

js/src/vm/Debugger.cpp

@@ -4844,20 +4844,6 @@ ScriptOffset(JSContext* cx, JSScript* script, const Value& v, size_t* offsetp)
     return true;
 }
 
-static bool
-DebuggerScript_getOffsetLine(JSContext* cx, unsigned argc, Value* vp)
-{
-    THIS_DEBUGSCRIPT_SCRIPT(cx, argc, vp, "getOffsetLine", args, obj, script);
-    if (!args.requireAtLeast(cx, "Debugger.Script.getOffsetLine", 1))
-        return false;
-    size_t offset;
-    if (!ScriptOffset(cx, script, args[0], &offset))
-        return false;
-    unsigned lineno = PCToLineNumber(script, script->offsetToPC(offset));
-    args.rval().setNumber(lineno);
-    return true;
-}
-
 namespace {
 
 class BytecodeRangeWithPosition : private BytecodeRange
@@ -5087,6 +5073,37 @@ class FlowGraphSummary {
 
 } /* anonymous namespace */
 
+static bool
+DebuggerScript_getOffsetLocation(JSContext* cx, unsigned argc, Value* vp)
+{
+    THIS_DEBUGSCRIPT_SCRIPT(cx, argc, vp, "getOffsetLocation", args, obj, script);
+    if (!args.requireAtLeast(cx, "Debugger.Script.getOffsetLocation", 1))
+        return false;
+    size_t offset;
+    if (!ScriptOffset(cx, script, args[0], &offset))
+        return false;
+
+    RootedPlainObject result(cx, NewBuiltinClassInstance<PlainObject>(cx));
+    if (!result)
+        return false;
+
+    BytecodeRangeWithPosition r(cx, script);
+    while (!r.empty() && r.frontOffset() < offset)
+        r.popFront();
+
+    RootedId id(cx, NameToId(cx->names().lineNumber));
+    RootedValue value(cx, NumberValue(r.frontLineNumber()));
+    if (!DefineProperty(cx, result, id, value))
+        return false;
+
+    value = NumberValue(r.frontColumnNumber());
+    if (!DefineProperty(cx, result, cx->names().columnNumber, value))
+        return false;
+
+    args.rval().setObject(*result);
+    return true;
+}
+
 static bool
 DebuggerScript_getAllOffsets(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -5646,7 +5663,7 @@ static const JSFunctionSpec DebuggerScript_methods[] = {
     JS_FN("getAllOffsets", DebuggerScript_getAllOffsets, 0, 0),
     JS_FN("getAllColumnOffsets", DebuggerScript_getAllColumnOffsets, 0, 0),
     JS_FN("getLineOffsets", DebuggerScript_getLineOffsets, 1, 0),
-    JS_FN("getOffsetLine", DebuggerScript_getOffsetLine, 0, 0),
+    JS_FN("getOffsetLocation", DebuggerScript_getOffsetLocation, 0, 0),
     JS_FN("setBreakpoint", DebuggerScript_setBreakpoint, 2, 0),
     JS_FN("getBreakpoints", DebuggerScript_getBreakpoints, 1, 0),
     JS_FN("clearBreakpoint", DebuggerScript_clearBreakpoint, 1, 0),