Fwd: Source maps, minified sources, and column breakpoints.

[Eddy Bruel]

---------- Forwarded message ----------
From: Eddy Bruel <ejpb...@mozilla.com>
Date: Mon, Feb 23, 2015 at 3:43 PM
Subject: Source maps, minified sources, and column breakpoints.
To: Nick Fitzgerald <nfitz...@mozilla.com>, James Long <jl...@mozilla.com
>


Hi Nick, James,

I've been trying to merge the latest version of the source-map library with
fx-team. Not surprisingly, this causes some tests to fail. In particular,
browser_dbg_source-maps-03.js (which tests breakpoints on minified sources)
fails, because the breakpoint is never hit.

What we are trying to do is cause the debugger to break on line 30, column
10, by setting a breakpoint on line 30, column 21.

The code_math.map source map has the following entries for line 30:

310->30:10, 317->30:10, 321->30:10, 322->30:22, 323->30:10

We don't have an exact match for column 21, so we use the fuzzy search
algorithm. This should give us the mapping for line 30, column 10. However,
that doesn't make any sense. There is no such thing as *the* mapping for
line 30, column 10, because there is more than one.

What we really want to do here is find all mappings for line 30, column 10,
and set a column breakpoint on each of their column spans (i.e. column
310-316, 317-320, 321, and 323-eol). We currently have no way to do this,
since getGeneratedLocation assumes each original location corresponds to
exactly one generated location.

I have plans to fix this in the near future. I already implemented
allGeneratedPositionsFor so we can implement getAllGeneratedLocations, but
this only works for line breakpoints, not column breakpoints, since it
gives you all generated locations for the given line. We will need to make
additional changes to the source-map library to make this work for column
breakpoints as well.

For now, I'm looking for a stopgap measure to get the tests to pass while I
continue refactoring the breakpoint code. Accepting that we can only find
one mapping for line 30, column 10 for now, which mapping should we use?
With the current version of the source map library, its 317->30:10, which
happens to work. With the new version in place, its 321->30:10 (probably
because we changed the bias in the fuzzy search algorithm), which happens
to not work.

Thing is, I think both those are wrong. If we cannot set a column
breakpoint on all columns in the minified source that correspond to a given
line in an original source, then the next best thing we can do is set a
column breakpoint on the *first* column that corresponds to that line. In
other words, we should be using 310->30:10.

I think we can accomplish this by refactoring the search algorithm in the
source-map library as follows: when we've found either an exact or inexact
match, check if there are other mappings for the same position. If there
are, return either the smallest or largest of these mappings, depending on
the search bias.

Does this sound like a reasonable implementation strategy? I still need to
check if this would make the browser_dbg-source-map-03.js test pass, and
it's not unthinkable that this change would break yet more other things.
Source map handling turns out to be unbelievably finicky.

A propos, I hope this proves that although we need column breakpoints
internally to make breakpoints work in minified sources at all, the way
they are currently implemented is completely wrong. Since we currently
don't need them at the protocol level at all (we do use them in this test,
but there is no reason we couldn't replace them with a line breakpoint
here), I'd like to remove them from the protocol until we get our act
together.