Bug 1531350 - [release 128] [Breakpoints] Use the union of all mapped and unmapped regions when fetching original breakpoints. (#8014). r=dwalsh
authorLogan Smyth <loganfsmyth@gmail.com>
Thu, 28 Feb 2019 14:35:59 -0500
changeset 520004 b68f9ac11c4f3f78d857212395bb93e12469cbbd
parent 520003 f3bc0a57e1f6af2a2f0c7ed73d6e3a2c827183bc
child 520005 0291de30d95876833df4a415a4a5502d1168130f
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)
reviewersdwalsh
bugs1531350
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 1531350 - [release 128] [Breakpoints] Use the union of all mapped and unmapped regions when fetching original breakpoints. (#8014). r=dwalsh
devtools/client/debugger/new/packages/devtools-source-map/src/index.js
devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
devtools/client/debugger/new/packages/devtools-source-map/src/tests/fixtures/intermingled-sources.js
devtools/client/debugger/new/packages/devtools-source-map/src/tests/fixtures/intermingled-sources.js.map
devtools/client/debugger/new/packages/devtools-source-map/src/tests/source-map.js
devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/index.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/index.js
@@ -72,16 +72,39 @@ export const getAllGeneratedLocations = 
 ): Promise<Array<SourceLocation>> =>
   _getAllGeneratedLocations(location, originalSource);
 
 export const getOriginalLocation = async (
   location: SourceLocation,
   options: locationOptions = {}
 ): Promise<SourceLocation> => _getOriginalLocation(location, options);
 
+export const getGeneratedRangesForOriginal = async (
+  sourceId: SourceId,
+  url: string,
+  mergeUnmappedRegions?: boolean
+): Promise<
+  Array<{
+    start: {
+      line: number,
+      column: number
+    },
+    end: {
+      line: number,
+      column: number
+    }
+  }>
+> =>
+  dispatcher.invoke(
+    "getGeneratedRangesForOriginal",
+    sourceId,
+    url,
+    mergeUnmappedRegions
+  );
+
 export const getFileGeneratedRange = async (
   originalSource: Source
 ): Promise<?{ start: any, end: any }> =>
   dispatcher.invoke("getFileGeneratedRange", originalSource);
 
 export const getLocationScopes = dispatcher.task("getLocationScopes");
 
 export const getOriginalSourceText = async (
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
@@ -321,16 +321,158 @@ async function getOriginalSourceText(
   }
 
   return {
     text,
     contentType: getContentType(originalSource.url || "")
   };
 }
 
+/**
+ * Find the set of ranges on the generated file that map from the original
+ * file's locations.
+ *
+ * @param sourceId - The original ID of the file we are processing.
+ * @param url - The original URL of the file we are processing.
+ * @param mergeUnmappedRegions - If unmapped regions are encountered between
+ *   two mappings for the given original file, allow the two mappings to be
+ *   merged anyway. This is useful if you are more interested in the general
+ *   contiguous ranges associated with a file, rather than the specifics of
+ *   the ranges provided by the sourcemap.
+ */
+const GENERATED_MAPPINGS = new WeakMap();
+async function getGeneratedRangesForOriginal(
+  sourceId: SourceId,
+  url: string,
+  mergeUnmappedRegions: boolean = false
+): Promise<
+  Array<{
+    start: {
+      line: number,
+      column: number
+    },
+    end: {
+      line: number,
+      column: number
+    }
+  }>
+> {
+  assert(isOriginalId(sourceId), "Source is not an original source");
+
+  const map = await getSourceMap(originalToGeneratedId(sourceId));
+  if (!map) {
+    return [];
+  }
+
+  if (!COMPUTED_SPANS.has(map)) {
+    COMPUTED_SPANS.add(map);
+    map.computeColumnSpans();
+  }
+
+  const cachedGeneratedMappingsForOriginal = GENERATED_MAPPINGS.get(map);
+  if (cachedGeneratedMappingsForOriginal) {
+    return cachedGeneratedMappingsForOriginal;
+  }
+
+  // Gather groups of mappings on the generated file, with new groups created
+  // if we cross a mapping for a different file.
+  let currentGroup = [];
+  const originalGroups = [currentGroup];
+  map.eachMapping(
+    mapping => {
+      if (mapping.source === url) {
+        currentGroup.push({
+          start: {
+            line: mapping.generatedLine,
+            column: mapping.generatedColumn
+          },
+          end: {
+            line: mapping.generatedLine,
+            // The lastGeneratedColumn value is an inclusive value so we add
+            // one to it to get the exclusive end position.
+            column: mapping.lastGeneratedColumn + 1
+          }
+        });
+      } else if (
+        typeof mapping.source === "string" &&
+        currentGroup.length > 0
+      ) {
+        // If there is a URL, but it is for a _different_ file, we create a
+        // new group of mappings so that we can tell
+        currentGroup = [];
+        originalGroups.push(currentGroup);
+      }
+    },
+    null,
+    SourceMapConsumer.GENERATED_ORDER
+  );
+
+  const generatedMappingsForOriginal = [];
+  if (mergeUnmappedRegions) {
+    // If we don't care about excluding unmapped regions, then we just need to
+    // create a range that is the fully encompasses each group, ignoring the
+    // empty space between each individual range.
+    for (const group of originalGroups) {
+      if (group.length > 0) {
+        generatedMappingsForOriginal.push({
+          start: group[0].start,
+          end: group[group.length - 1].end
+        });
+      }
+    }
+  } else {
+    let lastEntry;
+    for (const group of originalGroups) {
+      lastEntry = null;
+      for (const { start, end } of group) {
+        const lastEnd = lastEntry
+          ? wrappedMappingPosition(lastEntry.end)
+          : null;
+
+        // If this entry comes immediately after the previous one, extend the
+        // range of the previous entry instead of adding a new one.
+        if (
+          lastEntry &&
+          lastEnd &&
+          lastEnd.line === start.line &&
+          lastEnd.column === start.column
+        ) {
+          lastEntry.end = end;
+        } else {
+          const newEntry = { start, end };
+          generatedMappingsForOriginal.push(newEntry);
+          lastEntry = newEntry;
+        }
+      }
+    }
+  }
+
+  GENERATED_MAPPINGS.set(map, generatedMappingsForOriginal);
+  return generatedMappingsForOriginal;
+}
+
+function wrappedMappingPosition(pos: {
+  line: number,
+  column: number
+}): {
+  line: number,
+  column: number
+} {
+  if (pos.column !== Infinity) {
+    return pos;
+  }
+
+  // If the end of the entry consumes the whole line, treat it as wrapping to
+  // the next line.
+  return {
+    line: pos.line + 1,
+    column: 0
+  };
+}
+
 async function getFileGeneratedRange(
   originalSource: Source
 ): Promise<?{ start: any, end: any }> {
   assert(isOriginalId(originalSource.id), "Source is not an original source");
 
   const map = await getSourceMap(originalToGeneratedId(originalSource.id));
   if (!map) {
     return;
@@ -389,13 +531,14 @@ module.exports = {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
   getOriginalSourceText,
+  getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   applySourceMap,
   clearSourceMaps,
   hasMappedSource
 };
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/tests/fixtures/intermingled-sources.js
@@ -0,0 +1,28 @@
+"use strict";
+
+var decl = function () {
+  var _ref = _asyncToGenerator( /*#__PURE__*/regeneratorRuntime.mark(function _callee() {
+    return regeneratorRuntime.wrap(function _callee$(_context) {
+      while (1) {
+        switch (_context.prev = _context.next) {
+          case 0:
+            console.log("2");
+
+          case 1:
+          case "end":
+            return _context.stop();
+        }
+      }
+    }, _callee, this);
+  }));
+
+  return function decl() {
+    return _ref.apply(this, arguments);
+  };
+}();
+
+function _asyncToGenerator(fn) { return function () { var gen = fn.apply(this, arguments); return new Promise(function (resolve, reject) { function step(key, arg) { try { var info = gen[key](arg); var value = info.value; } catch (error) { reject(error); return; } if (info.done) { resolve(value); } else { return Promise.resolve(value).then(function (value) { step("next", value); }, function (err) { step("throw", err); }); } } return step("next"); }); }; }
+
+console.log("1");
+
+console.log("3");
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/tests/fixtures/intermingled-sources.js.map
@@ -0,0 +1,8 @@
+{
+  "version":3,
+  "file":"output.js",
+  "mappings":";;;qEAEA;AAAA;AAAA;AAAA;AAAA;AACE,oBAAQ,GAAR,CAAY,GAAZ;;AADF;AAAA;AAAA;AAAA;AAAA;AAAA;AAAA,G;;kBAAe,I;;;;;;;AAFf,QAAQ,GAAR,CAAY,GAAZ;;AAMA,QAAQ,GAAR,CAAY,GAAZ",
+  "sources":["input.js"],
+  "sourcesContent":["console.log(\"1\")\n\nasync function decl(){\n  console.log(\"2\");\n}\n\nconsole.log(\"3\");"],
+  "names":[]
+}
\ No newline at end of file
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/tests/source-map.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/tests/source-map.js
@@ -4,16 +4,17 @@
 
 jest.mock("devtools-utils/src/network-request");
 const networkRequest = require("devtools-utils/src/network-request");
 
 const {
   getOriginalURLs,
   hasMappedSource,
   getOriginalLocation,
+  getGeneratedRangesForOriginal,
   clearSourceMaps
 } = require("../source-map");
 
 const { setupBundleFixture } = require("./helpers");
 
 describe("source maps", () => {
   beforeEach(() => {
     clearSourceMaps();
@@ -46,16 +47,103 @@ describe("source maps", () => {
     });
 
     test("Non-existing sourceRoot resolution with relative URLs", async () => {
       const urls = await setupBundleFixture("noroot2");
       expect(urls).toEqual(["http://example.com/heart.js"]);
     });
   });
 
+  describe("getGeneratedRangesForOriginal", () => {
+    test("the overall generated ranges on the source", async () => {
+      const urls = await setupBundleFixture("intermingled-sources");
+
+      const ranges = await getGeneratedRangesForOriginal(
+        "intermingled-sources.js/originalSource-01",
+        urls[0]
+      );
+
+      expect(ranges).toEqual([
+        {
+          start: {
+            line: 4,
+            column: 69
+          },
+          end: {
+            line: 9,
+            column: Infinity
+          }
+        },
+        {
+          start: {
+            line: 11,
+            column: 0
+          },
+          end: {
+            line: 17,
+            column: 3
+          }
+        },
+        {
+          start: {
+            line: 19,
+            column: 18
+          },
+          end: {
+            line: 19,
+            column: 22
+          }
+        },
+        {
+          start: {
+            line: 26,
+            column: 0
+          },
+          end: {
+            line: 26,
+            column: Infinity
+          }
+        },
+        {
+          start: {
+            line: 28,
+            column: 0
+          },
+          end: {
+            line: 28,
+            column: Infinity
+          }
+        }
+      ]);
+    });
+
+    test("the merged generated ranges on the source", async () => {
+      const urls = await setupBundleFixture("intermingled-sources");
+
+      const ranges = await getGeneratedRangesForOriginal(
+        "intermingled-sources.js/originalSource-01",
+        urls[0],
+        true
+      );
+
+      expect(ranges).toEqual([
+        {
+          start: {
+            line: 4,
+            column: 69
+          },
+          end: {
+            line: 28,
+            column: Infinity
+          }
+        }
+      ]);
+    });
+  });
+
   describe("hasMappedSource", async () => {
     test("has original location", async () => {
       await setupBundleFixture("bundle");
       const location = {
         sourceId: "bundle.js",
         line: 49
       };
       const isMapped = await hasMappedSource(location);
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
@@ -7,16 +7,17 @@ const {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
   getOriginalSourceText,
+  getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   clearSourceMaps,
   applySourceMap
 } = require("./source-map");
 
 const { getOriginalStackFrames } = require("./utils/getOriginalStackFrames");
 const { setAssetRootURL } = require("./utils/wasmAsset");
@@ -33,13 +34,14 @@ self.onmessage = workerHandler({
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
   getOriginalSourceText,
   getOriginalStackFrames,
+  getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   applySourceMap,
   clearSourceMaps
 });
--- a/devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
@@ -45,25 +45,47 @@ export function setBreakpointPositions(l
   return async ({ getState, dispatch, client, sourceMaps }: ThunkArgs) => {
     let sourceId = location.sourceId;
     if (getBreakpointPositionsForSource(getState(), sourceId)) {
       return;
     }
 
     let source = getSourceFromId(getState(), sourceId);
 
-    let range;
+    let results = {};
     if (isOriginalId(sourceId)) {
-      range = await sourceMaps.getFileGeneratedRange(source);
+      const ranges = await sourceMaps.getGeneratedRangesForOriginal(
+        sourceId,
+        source.url,
+        true
+      );
       sourceId = originalToGeneratedId(sourceId);
       source = getSourceFromId(getState(), sourceId);
-    }
 
-    const results = await client.getBreakpointPositions(
-      source.actors[0],
-      range
-    );
+      // Note: While looping here may not look ideal, in the vast majority of
+      // cases, the number of ranges here should be very small, and is quite
+      // likely to only be a single range.
+      for (const range of ranges) {
+        // Wrap infinite end positions to the next line to keep things simple
+        // and because we know we don't care about the end-line whitespace
+        // in this case.
+        if (range.end.column === Infinity) {
+          range.end.line += 1;
+          range.end.column = 0;
+        }
+
+        const bps = await client.getBreakpointPositions(
+          source.actors[0],
+          range
+        );
+        for (const line in bps) {
+          results[line] = (results[line] || []).concat(bps[line]);
+        }
+      }
+    } else {
+      results = await client.getBreakpointPositions(source.actors[0]);
+    }
 
     let positions = convertToList(results, sourceId);
     positions = await mapLocations(positions, getState(), source, sourceMaps);
     return dispatch({ type: "ADD_BREAKPOINT_POSITIONS", positions, location });
   };
 }