James Long (:jlongster) Reporter
	Description • 9 years ago

I discovered this while working bug 1230345 and updating the browser_dbg_source-maps-01.js test.

Here it's testing setting a breakpoint on a blank line of a sourcemapped source: https://dxr.mozilla.org/mozilla-central/source/devtools/client/debugger/test/mochitest/browser_dbg_source-maps-01.js#82

Here is the blank line in the coffeescript source: https://dxr.mozilla.org/mozilla-central/source/devtools/client/debugger/test/mochitest/code_binary_search.coffee#7

The test is trying to make sure that we do breakpoint sliding properly on sourcemapping sources, and it should "slide" to the 8th line. Everything *seems* to work.

But not breakpoint sliding ever actually occurs. What happens is we end up calling `getAllGeneratedLocations` in TabSources for line 7 of the coffeescript source, which calls `map.allGeneratedPositionsFor` on the sourcemap. What should this return for a blank line?

Here's the sourcemap visualized: https://goo.gl/FPC9P5

Looking at the mapping at the bottom, we shouldn't be getting back any generated positions. I would think we would get back an empty array.

However, if I log the output of `map.allGeneratedPositionsFor` given line 7 for this source, I get:

line: 9 column: 1 lastColumn: 2
line: 17 column: 5 lastColumn: Infinity
line: 9 column: 11 lastColumn: Infinity
line: 9 column: 5 lastColumn: 10
line: 9 column: 4 lastColumn: 4
line: 9 column: 3 lastColumn: 3

It seems like the source-map library itself it doing the sliding, but it really shouldn't.

	Eddy Bruel [:ejpbruel]
	Comment 1 • 9 years ago

I took a look at the implementation of map.allGeneratedPositionsFor. The way it works is like this:

We start by finding the mapping that is the greatest lower bound for the line we are looking for. Note that this mapping *could* be for a line that's below the line we are looking for, if the latter line is empty (which is what you're observing).

If we do find a greatest lower bound, we then iterate over the mappings until either we run out of mappings, or we we find a mapping for a different line. Because the mapping we started out with was the greatest lower bound, this should either give us all mappings for the line we were looking for, or the closest non-empty line if the line was empty.

I admit it's kind of messy to have the source-map library do it's own form of breakpoint sliding in addition to what the debugger does, but this didn't turn out to be a problem in practice. Of course, now that we want to get rid of breakpoint sliding, it's a different story.

Luckily, this looks like something we could easily fix by adding a single check to allGeneratePositionsFor.

	Eddy Bruel [:ejpbruel]
	Comment 2 • 9 years ago

To save you some work, I've filed a pull request that should fix the issue for you:
https://github.com/mozilla/source-map/pull/220

	Tom Tromey :tromey
	Comment 3 • 8 years ago

Upstream patch stalled.