Bug 1557138 Clicking logpoint location in console opens the logpoint editing panel in debugger r=nchevobbe,jlast

I pass `frame.origin` in `viewSourceInDebugger`. When `reason` is logpoint, run `openConditionalPanel`.

Differential Revision: https://phabricator.services.mozilla.com/D35624

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/panel.js

@@ -160,9 +160,14 @@ DebuggerPanel.prototype = {
     return this._actions.selectSourceURL(cx, url, { line, column });
   },
 
-  selectSource(sourceId, line, column) {
+  async selectSource(sourceId, line, column) {
     const cx = this._selectors.getContext(this._getState());
-    return this._actions.selectSource(cx, sourceId, { line, column });
+    const location = { sourceId, line, column };
+
+    await this._actions.selectSource(cx, sourceId, location);
+    if (this._selectors.hasLogpoint(this._getState(), location)) {
+      this._actions.openConditionalPanel(location, true);
+    }
   },
 
   canLoadSource(sourceId) {

devtools/client/debugger/src/components/Editor/ConditionalPanel.js

@@ -12,7 +12,7 @@ import { toEditorLine } from "../../utils/editor";
 import actions from "../../actions";
 
 import {
-  getBreakpointForLocation,
+  getBreakpoint,
   getConditionalPanelLocation,
   getLogPointStatus,
   getContext,
@@ -231,7 +231,7 @@ const mapStateToProps = state => {
   const location = getConditionalPanelLocation(state);
   return {
     cx: getContext(state),
-    breakpoint: getBreakpointForLocation(state, location),
+    breakpoint: getBreakpoint(state, location),
     location,
     log: getLogPointStatus(state),
   };

devtools/client/debugger/src/reducers/breakpoints.js

@@ -178,8 +178,12 @@ export function getBreakpointCount(state: OuterState): number {
 
 export function getBreakpoint(
   state: OuterState,
-  location: SourceLocation
+  location: ?SourceLocation
 ): ?Breakpoint {
+  if (!location) {
+    return undefined;
+  }
+
   const breakpoints = getBreakpointsMap(state);
   return breakpoints[makeBreakpointId(location)];
 }
@@ -208,9 +212,9 @@ export function getBreakpointsForSource(
 
 export function getBreakpointForLocation(
   state: OuterState,
-  location: SourceLocation | null
+  location: ?SourceLocation
 ): ?Breakpoint {
-  if (!location || !location.sourceId) {
+  if (!location) {
     return undefined;
   }
 
@@ -226,4 +230,12 @@ export function getHiddenBreakpoint(state: OuterState): ?Breakpoint {
   return breakpoints.find(bp => bp.options.hidden);
 }
 
+export function hasLogpoint(
+  state: OuterState,
+  location: ?SourceLocation
+): ?string {
+  const breakpoint = getBreakpoint(state, location);
+  return breakpoint && breakpoint.options.logValue;
+}
+
 export default update;

devtools/client/shared/components/Frame.js

@@ -204,11 +204,11 @@ class Frame extends Component {
       }
     }
 
-    // If the message comes from a logPoint or conditional breakpoint,
+    // If the message comes from a logPoint,
     // prefix the source location accordingly
-    if (frame.origin) {
+    if (frame.options) {
       let locationPrefix;
-      if (frame.origin === "logPoint") {
+      if (frame.options.logPoint) {
         locationPrefix = "Logpoint @ ";
       }
 

devtools/client/shared/components/test/mochitest/test_frame_01.html

@@ -333,10 +333,10 @@ window.onload = async function () {
     // a prefix should render before source
     await checkFrameComponent({
       frame: {
-        origin: "logPoint",
         source: "http://myfile.com/mahscripts.js",
         line: 55,
         column: 10,
+        options: { logPoint: true },
       }
     }, {
       locationPrefix: "Logpoint @ ",

devtools/client/shared/view-source.js

@@ -52,7 +52,7 @@ exports.viewSourceInStyleEditor = async function(
  * @param {number} sourceLine
  * @param {number} sourceColumn
  * @param {string} sourceID
- * @param {string} [reason=unknown]
+ * @param {(string|object)} [reason=unknown]
  *
  * @return {Promise<boolean>}
  */

devtools/client/webconsole/test/browser/browser.ini

@@ -91,6 +91,9 @@ support-files =
   test-location-debugger-link-console-log.js
   test-location-debugger-link-errors.js
   test-location-debugger-link.html
+  test-location-debugger-link-logpoint-1.js
+  test-location-debugger-link-logpoint-2.js
+  test-location-debugger-link-logpoint.html
   test-location-styleeditor-link-1.css
   test-location-styleeditor-link-2.css
   test-location-styleeditor-link-minified.css
@@ -359,6 +362,7 @@ skip-if = fission
 skip-if = fission
 [browser_webconsole_keyboard_accessibility.js]
 [browser_webconsole_location_debugger_link.js]
+[browser_webconsole_location_logpoint_debugger_link.js]
 [browser_webconsole_location_scratchpad_link.js]
 [browser_webconsole_location_styleeditor_link.js]
 [browser_webconsole_logErrorInPage.js]

devtools/client/webconsole/test/browser/browser_webconsole_location_logpoint_debugger_link.js

@@ -0,0 +1,187 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test clicking locations of logpoint logs and errors will open corresponding
+// conditional panels in the debugger.
+
+"use strict";
+
+const TEST_URI =
+  "http://example.com/browser/devtools/client/webconsole/" +
+  "test/browser/test-location-debugger-link-logpoint.html";
+
+add_task(async function() {
+  // On e10s, the exception thrown in test-location-debugger-link-errors.js
+  // is triggered in child process and is ignored by test harness
+  if (!Services.appinfo.browserTabsRemoteAutostart) {
+    expectUncaughtException();
+  }
+
+  // Eliminate interference from "saved" breakpoints
+  // when running the test multiple times
+  await clearDebuggerPreferences();
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  info("Open the Debugger panel");
+  await openDebugger();
+
+  const toolbox = gDevTools.getToolbox(hud.target);
+  const dbg = createDebuggerContext(toolbox);
+  await selectSource(dbg, "test-location-debugger-link-logpoint-1.js");
+
+  info("Add a logpoint with an invalid expression");
+  await setLogPoint(dbg, 9, "undefinedVariable");
+
+  info("Add a logpoint with a valid expression");
+  await setLogPoint(dbg, 10, "`a is ${a}`");
+
+  await assertEditorLogpoint(dbg, 9, { hasLog: true });
+  await assertEditorLogpoint(dbg, 10, { hasLog: true });
+
+  info("Close the file in the debugger");
+  await closeTab(dbg, "test-location-debugger-link-logpoint-1.js");
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function");
+  await invokeInTab("add");
+
+  info("Wait for two messages");
+  await waitFor(() => findMessages(hud, "").length === 2);
+
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "[Logpoint threw]: undefinedVariable is not defined",
+    true,
+    false,
+    false,
+    "undefinedVariable"
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "a is 1",
+    true,
+    false,
+    false,
+    "`a is ${a}`"
+  );
+
+  // Test clicking location of a removed logpoint, or a newly added breakpoint
+  // at an old logpoint's location will only highlight its line
+  info("Remove the logpoints");
+  const source = await findSource(
+    dbg,
+    "test-location-debugger-link-logpoint-1.js"
+  );
+  await removeBreakpoint(dbg, source.id, 9);
+  await removeBreakpoint(dbg, source.id, 10);
+  await addBreakpoint(dbg, "test-location-debugger-link-logpoint-1.js", 10);
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "[Logpoint threw]: undefinedVariable is not defined",
+    true,
+    9,
+    12
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+  await testOpenInDebugger(hud, toolbox, "a is 1", true, 10, 12);
+});
+
+// Test clicking locations of logpoints from different files
+add_task(async function() {
+  if (!Services.appinfo.browserTabsRemoteAutostart) {
+    expectUncaughtException();
+  }
+
+  await clearDebuggerPreferences();
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  info("Open the Debugger panel");
+  await openDebugger();
+
+  const toolbox = gDevTools.getToolbox(hud.target);
+  const dbg = createDebuggerContext(toolbox);
+
+  info("Add a logpoint to the first file");
+  await selectSource(dbg, "test-location-debugger-link-logpoint-1.js");
+  await setLogPoint(dbg, 10, "`a is ${a}`");
+
+  info("Add a logpoint to the second file");
+  await selectSource(dbg, "test-location-debugger-link-logpoint-2.js");
+  await setLogPoint(dbg, 10, "`c is ${c}`");
+
+  info("Selecting the console");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function from the first file");
+  await invokeInTab("add");
+
+  info("Wait for the first message");
+  await waitFor(() => findMessages(hud, "").length === 1);
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "a is 1",
+    true,
+    false,
+    false,
+    "`a is ${a}`"
+  );
+
+  info("Selecting the console again");
+  await toolbox.selectTool("webconsole");
+
+  info("Call the function from the second file");
+  await invokeInTab("subtract");
+
+  info("Wait for the second message");
+  await waitFor(() => findMessages(hud, "").length === 2);
+  await testOpenInDebugger(
+    hud,
+    toolbox,
+    "c is 1",
+    true,
+    false,
+    false,
+    "`c is ${c}`"
+  );
+});
+
+async function setLogPoint(dbg, index, expression) {
+  rightClickElement(dbg, "gutter", index);
+  selectContextMenuItem(
+    dbg,
+    `${selectors.addLogItem},${selectors.editLogItem}`
+  );
+  const onBreakpointSet = waitForDispatch(dbg, "SET_BREAKPOINT");
+  await typeInPanel(dbg, expression);
+  await onBreakpointSet;
+}
+
+function getLineEl(dbg, line) {
+  const lines = dbg.win.document.querySelectorAll(".CodeMirror-code > div");
+  return lines[line - 1];
+}
+
+function assertEditorLogpoint(dbg, line, { hasLog = false } = {}) {
+  const hasLogClass = getLineEl(dbg, line).classList.contains("has-log");
+
+  ok(
+    hasLogClass === hasLog,
+    `Breakpoint log ${hasLog ? "exists" : "does not exist"} on line ${line}`
+  );
+}

devtools/client/webconsole/test/browser/head.js

@@ -381,17 +381,18 @@ function waitForNodeMutation(node, observeConfig = {}) {
 }
 
 /**
- * Search for a given message.  When found, simulate a click on the
+ * Search for a given message. When found, simulate a click on the
  * message's location, checking to make sure that the debugger opens
- * the corresponding URL.
+ * the corresponding URL. If the message was generated by a logpoint,
+ * check if the corresponding logpoint editing panel is opened.
  *
  * @param {Object} hud
  *        The webconsole
  * @param {Object} toolbox
  *        The toolbox
  * @param {String} text
- *        The text to search for.  This should be contained in the
- *        message.  The searching is done with @see findMessage.
+ *        The text to search for. This should be contained in the
+ *        message. The searching is done with @see findMessage.
  * @param {boolean} expectUrl
  *        Whether the URL in the opened source should match the link, or whether
  *        it is expected to be null.
@@ -399,6 +400,8 @@ function waitForNodeMutation(node, observeConfig = {}) {
  *        It indicates if there is the need to check the line.
  * @param {boolean} expectColumn
  *        It indicates if there is the need to check the column.
+ * @param {String} logPointExpr
+ *        The logpoint expression
  */
 async function testOpenInDebugger(
   hud,
@@ -406,7 +409,8 @@ async function testOpenInDebugger(
   text,
   expectUrl = true,
   expectLine = true,
-  expectColumn = true
+  expectColumn = true,
+  logPointExpr = undefined
 ) {
   info(`Finding message for open-in-debugger test; text is "${text}"`);
   const messageNode = await waitFor(() => findMessage(hud, text));
@@ -420,7 +424,8 @@ async function testOpenInDebugger(
     frameLinkNode,
     expectUrl,
     expectLine,
-    expectColumn
+    expectColumn,
+    logPointExpr
   );
 }
 
@@ -433,7 +438,8 @@ async function checkClickOnNode(
   frameLinkNode,
   expectUrl,
   expectLine,
-  expectColumn
+  expectColumn,
+  logPointExpr
 ) {
   info("checking click on node location");
 
@@ -485,6 +491,22 @@ async function checkClickOnNode(
       "expected source column"
     );
   }
+
+  if (logPointExpr !== undefined && logPointExpr !== "") {
+    const inputEl = dbg.panelWin.document.activeElement;
+    is(
+      inputEl.tagName,
+      "TEXTAREA",
+      "The textarea of logpoint panel is focused"
+    );
+
+    const inputValue = inputEl.parentElement.parentElement.innerText.trim();
+    is(
+      inputValue,
+      logPointExpr,
+      "The input in the open logpoint panel matches the logpoint expression"
+    );
+  }
 }
 
 /**

devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-1.js

@@ -0,0 +1,15 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+function add() {
+  const a = 1;
+  const b = 2;
+
+  return a + b;
+}
+
+add();

devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint-2.js

@@ -0,0 +1,15 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+function subtract() {
+  const c = 1;
+  const d = 2;
+
+  return c - d;
+}
+
+subtract();

devtools/client/webconsole/test/browser/test-location-debugger-link-logpoint.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8" />
+    <title>
+      Web Console test for opening logpoint message links in Debugger
+    </title>
+    <!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+    <script
+      type="text/javascript"
+      src="test-location-debugger-link-logpoint-1.js"
+    ></script>
+    <script
+      type="text/javascript"
+      src="test-location-debugger-link-logpoint-2.js"
+    ></script>
+  </head>
+  <body>
+    <p>Web Console test for opening logpoint message links in Debugger.</p>
+    <button onclick="add()">Add</button>
+    <button onclick="subtract()">Subtract</button>
+  </body>
+</html>

devtools/client/webconsole/utils/messages.js

@@ -247,7 +247,7 @@ function transformConsoleAPICallPacket(packet) {
     : null;
 
   if (type === "logPointError" || type === "logPoint") {
-    frame.origin = "logPoint";
+    frame.options = { logPoint: true };
   }
 
   return new ConsoleMessage({