Bug 1514339 - Update Debugger [Frontend v109] [breakpoints] maintain function indexes (#7342). r=davidwalsh
authorJason Laster <jlaster@mozilla.com>
Fri, 14 Dec 2018 14:49:49 -0500
changeset 451239 2ca7f51f6bad90b36f5b54ab0a6e026795bc9d9d
parent 451238 0490d2a7a5864d82bfbe8abcd6d99f6ad6036e07
child 451240 3b61ed43658627960464f2978cc8ea99fb6652c0
push id35230
push userbtara@mozilla.com
push dateWed, 19 Dec 2018 04:55:30 +0000
treeherdermozilla-central@204ab379fb82 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidwalsh
bugs1514339
milestone66.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 1514339 - Update Debugger [Frontend v109] [breakpoints] maintain function indexes (#7342). r=davidwalsh
devtools/client/debugger/new/dist/parser-worker.js
devtools/client/debugger/new/src/actions/breakpoints/index.js
devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
devtools/client/debugger/new/src/utils/breakpoint/astBreakpointLocation.js
devtools/client/debugger/new/src/utils/breakpoint/index.js
devtools/client/debugger/new/test/mochitest/browser.ini
devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-duplicate-functions.js
devtools/client/debugger/new/test/mochitest/examples/doc-duplicate-functions.html
--- a/devtools/client/debugger/new/dist/parser-worker.js
+++ b/devtools/client/debugger/new/dist/parser-worker.js
@@ -1336,22 +1336,27 @@ function getFunctionParameterNames(path)
     });
   }
   return [];
 }
 
 /* eslint-disable complexity */
 function extractSymbol(path, symbols) {
   if ((0, _helpers.isFunction)(path)) {
+    const name = (0, _getFunctionName2.default)(path.node, path.parent);
     symbols.functions.push({
-      name: (0, _getFunctionName2.default)(path.node, path.parent),
+      name,
       klass: (0, _inferClassName.inferClassName)(path),
       location: path.node.loc,
       parameterNames: getFunctionParameterNames(path),
-      identifier: path.node.id
+      identifier: path.node.id,
+      // indicates the occurence of the function in a file
+      // e.g { name: foo, ... index: 4 } is the 4th foo function
+      // in the file
+      index: symbols.functions.filter(f => f.name === name).length
     });
   }
 
   if (t.isJSXElement(path)) {
     symbols.hasJsx = true;
   }
 
   if (t.isGenericTypeAnnotation(path)) {
--- a/devtools/client/debugger/new/src/actions/breakpoints/index.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/index.js
@@ -151,21 +151,20 @@ export function toggleAllBreakpoints(sho
  * @memberof actions/breakpoints
  * @static
  */
 export function toggleBreakpoints(
   shouldDisableBreakpoints: boolean,
   breakpoints: Breakpoint[]
 ) {
   return async ({ dispatch }: ThunkArgs) => {
-    const promises = breakpoints.map(
-      breakpoint =>
-        shouldDisableBreakpoints
-          ? dispatch(disableBreakpoint(breakpoint.location))
-          : dispatch(enableBreakpoint(breakpoint.location))
+    const promises = breakpoints.map(breakpoint =>
+      shouldDisableBreakpoints
+        ? dispatch(disableBreakpoint(breakpoint.location))
+        : dispatch(enableBreakpoint(breakpoint.location))
     );
 
     await Promise.all(promises);
   };
 }
 
 /**
  * Removes all breakpoints
--- a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
@@ -26,21 +26,21 @@ import type {
 } from "../../types";
 
 type BreakpointSyncData = {
   previousLocation: SourceLocation,
   breakpoint: ?Breakpoint
 };
 
 async function makeScopedLocation(
-  { name, offset }: ASTLocation,
+  { name, offset, index }: ASTLocation,
   location: SourceLocation,
   source
 ) {
-  const scope = await findScopeByName(source, name);
+  const scope = await findScopeByName(source, name, index);
   // fallback onto the location line, if the scope is not found
   // note: we may at some point want to delete the breakpoint if the scope
   // disappears
   const line = scope ? scope.location.start.line + offset.line : location.line;
   return {
     line,
     column: location.column,
     sourceUrl: source.url,
--- a/devtools/client/debugger/new/src/utils/breakpoint/astBreakpointLocation.js
+++ b/devtools/client/debugger/new/src/utils/breakpoint/astBreakpointLocation.js
@@ -11,30 +11,35 @@ import type { SourceLocation, Source, AS
 import type { Symbols } from "../../reducers/ast";
 
 export function getASTLocation(
   source: Source,
   symbols: ?Symbols,
   location: SourceLocation
 ): ASTLocation {
   if (source.isWasm || !symbols || symbols.loading) {
-    return { name: undefined, offset: location };
+    return { name: undefined, offset: location, index: 0 };
   }
 
   const scope = findClosestFunction(symbols, location);
   if (scope) {
     // we only record the line, but at some point we may
     // also do column offsets
     const line = location.line - scope.location.start.line;
     return {
       name: scope.name,
-      offset: { line, column: undefined }
+      offset: { line, column: undefined },
+      index: scope.index
     };
   }
-  return { name: undefined, offset: location };
+  return { name: undefined, offset: location, index: 0 };
 }
 
-export async function findScopeByName(source: Source, name: ?string) {
+export async function findScopeByName(
+  source: Source,
+  name: ?string,
+  index: number
+) {
   const symbols = await getSymbols(source.id);
   const functions = symbols.functions;
 
-  return functions.find(node => node.name === name);
+  return functions.find(node => node.name === name && node.index === index);
 }
--- a/devtools/client/debugger/new/src/utils/breakpoint/index.js
+++ b/devtools/client/debugger/new/src/utils/breakpoint/index.js
@@ -128,17 +128,21 @@ export function createBreakpoint(
     hidden,
     generatedLocation,
     astLocation,
     id,
     text,
     originalText
   } = overrides;
 
-  const defaultASTLocation = { name: undefined, offset: location };
+  const defaultASTLocation = {
+    name: undefined,
+    offset: location,
+    index: 0
+  };
   const properties = {
     id,
     condition: condition || null,
     disabled: disabled || false,
     hidden: hidden || false,
     loading: false,
     astLocation: astLocation || defaultASTLocation,
     generatedLocation: generatedLocation || location,
--- a/devtools/client/debugger/new/test/mochitest/browser.ini
+++ b/devtools/client/debugger/new/test/mochitest/browser.ini
@@ -598,16 +598,17 @@ support-files =
   examples/big-sourcemap_files/bundle.js
   examples/big-sourcemap_files/bundle.js.map
   examples/reload/code_reload_1.js
   examples/reload/code_reload_2.js
   examples/reload/doc-reload.html
   examples/reload/sjs_code_reload.sjs
   examples/doc-async.html
   examples/doc-asm.html
+  examples/doc-duplicate-functions.html
   examples/doc-sourcemapped.html
   examples/doc-content-script-sources.html
   examples/doc-scripts.html
   examples/doc-scripts-debugger.html
   examples/doc-script-mutate.html
   examples/doc-script-switching.html
   examples/doc-exceptions.html
   examples/doc-iframes.html
@@ -668,16 +669,17 @@ skip-if = (os == 'win' && os_version == 
 [browser_dbg-sourcemapped-preview.js]
 skip-if = os == "win" # Bug 1448523, Bug 1448450
 [browser_dbg-breaking.js]
 [browser_dbg-breaking-from-console.js]
 [browser_dbg-breakpoints.js]
 [browser_dbg-breakpoints-actions.js]
 [browser_dbg-breakpoints-columns.js]
 [browser_dbg-breakpoints-cond.js]
+[browser_dbg-breakpoints-duplicate-functions.js]
 [browser_dbg-browser-content-toolbox.js]
 skip-if = !e10s || verify # This test is only valid in e10s
 [browser_dbg-breakpoints-reloading.js]
 [browser_dbg-breakpoint-skipping.js]
 [browser_dbg-call-stack.js]
 [browser_dbg-scopes.js]
 [browser_dbg-chrome-create.js]
 skip-if = (verify && !debug && (os == 'linux'))
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-duplicate-functions.js
@@ -0,0 +1,20 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function firstBreakpoint(dbg) {
+  return dbg.selectors.getBreakpointsList(dbg.getState())[0];
+}
+
+// tests to make sure we do not accidentally slide the breakpoint up to the first
+// function with the same name in the file.
+add_task(async function() {
+  const dbg = await initDebugger("doc-duplicate-functions.html", "doc-duplicate-functions");
+  const source = findSource(dbg, "doc-duplicate-functions");
+
+  await selectSource(dbg, source.url);
+  await addBreakpoint(dbg, source.url, 19);
+
+  await reload(dbg, source.url);
+  await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 1);
+  is(firstBreakpoint(dbg).location.line, 19, "Breakpoint is on line 19");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/test/mochitest/examples/doc-duplicate-functions.html
@@ -0,0 +1,24 @@
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<html>
+ <head>
+   <title>Debugger test page</title>
+ </head>
+ <body>
+   <h1> debugger test page2</h1>
+   <script type="text/javascript">
+     function a() {
+       function render() {
+         console.log('a');
+       }
+     }
+
+     function b() {
+       function render() {
+         console.log('b');
+       }
+     }
+   </script>
+ </body>
+</html>