Bug 1526557 - Specify correct source location in logpoint messages, r=lsmyth.
☠☠ backed out by d11fd5d6bda6 ☠ ☠
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 26 Feb 2019 17:11:52 -1000
changeset 519565 67bef7d63d865cd66c8f7c836ed8a0b90da8215e
parent 519564 09f23a363606830752a90a88dc452dd6d58ec10e
child 519566 d11fd5d6bda6aca0e7e5786a45471726dfdddb9a
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsmyth
bugs1526557
milestone67.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 1526557 - Specify correct source location in logpoint messages, r=lsmyth.
devtools/server/actors/breakpoint.js
devtools/server/tests/unit/test_logpoint-01.js
devtools/server/tests/unit/test_logpoint-02.js
devtools/server/tests/unit/testactors.js
--- a/devtools/server/actors/breakpoint.js
+++ b/devtools/server/actors/breakpoint.js
@@ -108,16 +108,30 @@ BreakpointActor.prototype = {
             };
             this.threadActor._parent._consoleActor.onConsoleAPICall(message);
           });
         }
       }
     }
   },
 
+  // Get a string message to display when a frame evaluation throws.
+  getThrownMessage(completion) {
+    try {
+      if (completion.throw.getOwnPropertyDescriptor) {
+        return completion.throw.getOwnPropertyDescriptor("message").value;
+      } else if (completion.toString) {
+        return completion.toString();
+      }
+    } catch (ex) {
+      // ignore
+    }
+    return "Unknown exception";
+  },
+
   /**
    * Check if this breakpoint has a condition that doesn't error and
    * evaluates to true in frame.
    *
    * @param frame Debugger.Frame
    *        The frame to evaluate the condition in
    * @returns Object
    *          - result: boolean|undefined
@@ -127,30 +141,19 @@ BreakpointActor.prototype = {
    *          - message: string
    *            If the condition throws, this is the thrown message.
    */
   checkCondition: function(frame, condition) {
     const completion = frame.eval(condition);
     if (completion) {
       if (completion.throw) {
         // The evaluation failed and threw
-        let message = "Unknown exception";
-        try {
-          if (completion.throw.getOwnPropertyDescriptor) {
-            message = completion.throw.getOwnPropertyDescriptor("message")
-                      .value;
-          } else if (completion.toString) {
-            message = completion.toString();
-          }
-        } catch (ex) {
-          // ignore
-        }
         return {
           result: true,
-          message: message,
+          message: this.getThrownMessage(completion),
         };
       } else if (completion.yield) {
         assert(false, "Shouldn't ever get yield completions from an eval");
       } else {
         return { result: !!completion.return };
       }
     }
     // The evaluation was killed (possibly by the slow script dialog)
@@ -183,51 +186,65 @@ BreakpointActor.prototype = {
     // the spot at which popping started, ignore it.  See bug 970469.
     const locationAtFinish = frame.onPop && frame.onPop.generatedLocation;
     if (locationAtFinish &&
         locationAtFinish.generatedLine === generatedLine &&
         locationAtFinish.generatedColumn === generatedColumn) {
       return undefined;
     }
 
-    const reason = {};
+    const reason = { type: "breakpoint", actors: [ this.actorID ] };
     const { condition, logValue } = this.options || {};
 
-    if (!condition && !logValue) {
-      reason.type = "breakpoint";
-      // TODO: add the rest of the breakpoints on that line (bug 676602).
-      reason.actors = [ this.actorID ];
-    } else {
-      // When replaying, breakpoints with log values are handled separately.
-      if (logValue && this.threadActor.dbg.replaying) {
-        return undefined;
-      }
+    // When replaying, breakpoints with log values are handled via
+    // _updateOptionsForScript.
+    if (logValue && this.threadActor.dbg.replaying) {
+      return undefined;
+    }
 
-      let condstr = condition;
-      if (logValue) {
-        // In the non-replaying case, log values are handled by treating them as
-        // conditions. console.log() never returns true so we will not pause.
-        condstr = condition
-          ? `(${condition}) && console.log(${logValue})`
-          : `console.log(${logValue})`;
-      }
-      const { result, message } = this.checkCondition(frame, condstr);
+    if (condition) {
+      const { result, message } = this.checkCondition(frame, condition);
 
       if (result) {
-        if (!message) {
-          reason.type = "breakpoint";
-        } else {
+        if (message) {
           reason.type = "breakpointConditionThrown";
           reason.message = message;
         }
-        reason.actors = [ this.actorID ];
       } else {
         return undefined;
       }
     }
+
+    if (logValue) {
+      const completion = frame.eval(logValue);
+      let value;
+      if (!completion) {
+        // The evaluation was killed (possibly by the slow script dialog).
+        value = "Log value evaluation incomplete";
+      } else if ("return" in completion) {
+        value = completion.return;
+      } else {
+        value = this.getThrownMessage(completion);
+      }
+      if (value && typeof value.unsafeDereference === "function") {
+        value = value.unsafeDereference();
+      }
+
+      const message = {
+        filename: url,
+        lineNumber: generatedLine,
+        columnNumber: generatedColumn,
+        "arguments": [value],
+      };
+      this.threadActor._parent._consoleActor.onConsoleAPICall(message);
+
+      // Never stop at log points.
+      return undefined;
+    }
+
     return this.threadActor._pauseAndRespond(frame, reason);
   },
 
   delete: function() {
     // Remove from the breakpoint store.
     this.threadActor.breakpointActorMap.deleteActor(this.location);
     this.threadActor.threadLifetimePool.removeActor(this);
     // Remove the actual breakpoint from the associated scripts.
--- a/devtools/server/tests/unit/test_logpoint-01.js
+++ b/devtools/server/tests/unit/test_logpoint-01.js
@@ -1,18 +1,20 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 /* eslint-disable no-shadow, max-nested-callbacks */
 
 "use strict";
 
 /**
- * Check that logpoints call console.log.
+ * Check that logpoints generate console messages.
  */
 
+const { getLastThreadActor } = require("xpcshell-test/testactors");
+
 var gDebuggee;
 var gClient;
 var gThreadClient;
 
 function run_test() {
   initTestDebuggerServer();
   gDebuggee = addTestGlobal("test-logpoint");
   gClient = new DebuggerClient(DebuggerServer.connectPipe());
@@ -22,39 +24,44 @@ function run_test() {
                              gThreadClient = threadClient;
                              test_simple_breakpoint();
                            });
   });
   do_test_pending();
 }
 
 function test_simple_breakpoint() {
+  let lastMessage;
+  getLastThreadActor()._parent._consoleActor = {
+    onConsoleAPICall(message) {
+      lastMessage = message;
+    },
+  };
+
   gThreadClient.addOneTimeListener("paused", async function(event, packet) {
     const source = await getSourceById(
       gThreadClient,
       packet.frame.where.actor
     );
 
     // Set a logpoint which should invoke console.log.
     gThreadClient.setBreakpoint({
       sourceUrl: source.url,
-      line: 4,
+      line: 3,
     }, { logValue: "a" });
 
     // Execute the rest of the code.
     gThreadClient.resume();
   });
 
-  // Sandboxes don't have a console available so we add our own.
   /* eslint-disable */
-  Cu.evalInSandbox("var console = { log: v => { this.logValue = v } };\n" + // 1
-                   "debugger;\n" + // 2
-                   "var a = 'three';\n" +  // 3
-                   "var b = 2;\n", // 4
+  Cu.evalInSandbox("debugger;\n" + // 1
+                   "var a = 'three';\n" +  // 2
+                   "var b = 2;\n", // 3
                    gDebuggee,
                    "1.8",
                    "test.js",
                    1);
   /* eslint-enable */
 
-  Assert.equal(gDebuggee.logValue, "three");
+  Assert.equal(lastMessage.arguments[0], "three");
   finishClient(gClient);
 }
--- a/devtools/server/tests/unit/test_logpoint-02.js
+++ b/devtools/server/tests/unit/test_logpoint-02.js
@@ -3,16 +3,18 @@
 /* eslint-disable no-shadow, max-nested-callbacks */
 
 "use strict";
 
 /**
  * Check that conditions are respected when specified in a logpoint.
  */
 
+const { getLastThreadActor } = require("xpcshell-test/testactors");
+
 var gDebuggee;
 var gClient;
 var gThreadClient;
 
 function run_test() {
   initTestDebuggerServer();
   gDebuggee = addTestGlobal("test-logpoint");
   gClient = new DebuggerClient(DebuggerServer.connectPipe());
@@ -22,41 +24,46 @@ function run_test() {
                              gThreadClient = threadClient;
                              test_simple_breakpoint();
                            });
   });
   do_test_pending();
 }
 
 function test_simple_breakpoint() {
+  let lastMessage;
+  getLastThreadActor()._parent._consoleActor = {
+    onConsoleAPICall(message) {
+      lastMessage = message;
+    },
+  };
+
   gThreadClient.addOneTimeListener("paused", async function(event, packet) {
     const source = await getSourceById(
       gThreadClient,
       packet.frame.where.actor
     );
 
     // Set a logpoint which should invoke console.log.
     gThreadClient.setBreakpoint({
       sourceUrl: source.url,
-      line: 5,
+      line: 4,
     }, { logValue: "a", condition: "a === 5" });
 
     // Execute the rest of the code.
     gThreadClient.resume();
   });
 
-  // Sandboxes don't have a console available so we add our own.
   /* eslint-disable */
-  Cu.evalInSandbox("var console = { log: v => { this.logValue = v } };\n" + // 1
-                   "debugger;\n" + // 2
-                   "var a = 1;\n" +  // 3
-                   "while (a < 10) {\n" + // 4
-                   "  a++;\n" + // 5
+  Cu.evalInSandbox("debugger;\n" + // 1
+                   "var a = 1;\n" +  // 2
+                   "while (a < 10) {\n" + // 3
+                   "  a++;\n" + // 4
                    "}",
                    gDebuggee,
                    "1.8",
                    "test.js",
                    1);
   /* eslint-enable */
 
-  Assert.equal(gDebuggee.logValue, 5);
+  Assert.equal(lastMessage.arguments[0], 5);
   finishClient(gClient);
 }
--- a/devtools/server/tests/unit/testactors.js
+++ b/devtools/server/tests/unit/testactors.js
@@ -77,21 +77,27 @@ exports.createRootActor = function creat
     tabList: new TestTabList(connection),
     globalActorFactories: ActorRegistry.globalActorFactories,
   });
 
   root.applicationType = "xpcshell-tests";
   return root;
 };
 
+var gLastThreadActor;
+
+exports.getLastThreadActor = function() {
+  return gLastThreadActor;
+};
+
 function TestTargetActor(connection, global) {
   this.conn = connection;
   this._global = global;
   this._global.wrappedJSObject = global;
-  this.threadActor = new ThreadActor(this, this._global);
+  this.threadActor = gLastThreadActor = new ThreadActor(this, this._global);
   this.conn.addActor(this.threadActor);
   this._attached = false;
   this._extraActors = {};
   this.makeDebugger = makeDebugger.bind(null, {
     findDebuggees: () => [this._global],
     shouldAddNewGlobalAsDebuggee: g => {
       if (gAllowNewThreadGlobals) {
         return true;