Bug 1526557 - Specify correct source location in logpoint messages, r=lsmyth.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 28 Feb 2019 06:25:10 -1000
changeset 519994 7497f62741416da24f15c59a6a01d3a64953ca86
parent 519993 e2a5031c85cd9c7bb7c87c8afe4510bc3711c436
child 519995 07ac96eae9f2c515a204ea33947e05aeca62dc5b
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
--- 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,16 +1,16 @@
 /* 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.
  */
 
 var gDebuggee;
 var gClient;
 var gThreadClient;
 
 function run_test() {
   initTestDebuggerServer();
@@ -22,39 +22,47 @@ function run_test() {
                              gThreadClient = threadClient;
                              test_simple_breakpoint();
                            });
   });
   do_test_pending();
 }
 
 function test_simple_breakpoint() {
+  const rootActor = gClient.transport._serverConnection.rootActor;
+  const threadActor = rootActor._parameters.tabList._targetActors[0].threadActor;
+
+  let lastMessage;
+  threadActor._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
@@ -22,41 +22,49 @@ function run_test() {
                              gThreadClient = threadClient;
                              test_simple_breakpoint();
                            });
   });
   do_test_pending();
 }
 
 function test_simple_breakpoint() {
+  const rootActor = gClient.transport._serverConnection.rootActor;
+  const threadActor = rootActor._parameters.tabList._targetActors[0].threadActor;
+
+  let lastMessage;
+  threadActor._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);
 }