Fix sourcemaps for merged/renamed and minified js files.

Look up sourcemaps proactively for js files when they're added to the runtime, and use the column from sourcemaps.

adapter/adapterProxy.ts

@@ -7,8 +7,10 @@ import * as Utilities from '../webkit/utilities';
 export type EventHandler = (event: DebugProtocol.Event) => void;
 
 export class AdapterProxy {
+    private static INTERNAL_EVENTS = ['scriptParsed'];
+
     public constructor(private _requestTransformers: IDebugTransformer[], private _debugAdapter: IDebugAdapter, private _eventHandler: EventHandler) {
-        this._debugAdapter.registerEventHandler(this._eventHandler);
+        this._debugAdapter.registerEventHandler(event => this.onAdapterEvent(event));
     }
 
     public dispatchRequest(request: DebugProtocol.Request): Promise<any> {
@@ -27,6 +29,20 @@ export class AdapterProxy {
             });
     }
 
+    private onAdapterEvent(event: DebugProtocol.Event): void {
+        // No need for transformers to modify events yet
+        this._requestTransformers.forEach(transformer => {
+            if (event.event in transformer) {
+                transformer[event.event](event);
+            }
+        });
+
+        // Internal events should not be passed back through DebugProtocol
+        if (AdapterProxy.INTERNAL_EVENTS.indexOf(event.event) < 0) {
+            this._eventHandler(event);
+        }
+    }
+
     /**
      * Pass the request arguments through the transformers. They modify the object in place.
      */

adapter/sourceMaps/sourceMapTransformer.ts

@@ -20,28 +20,29 @@ export class SourceMapTransformer implements IDebugTransformer {
         }
     }
 
-    public launch(args: ILaunchRequestArgs): void {
-        // Can html files be sourcemapped? May as well try.
-        if (this._sourceMaps && args.program) {
-            const generatedPath = this._sourceMaps.MapPathFromSource(args.program);
-            if (generatedPath) {
-                args.program = generatedPath;
-            }
-        }
-    }
-
     /**
      * Apply sourcemapping to the setBreakpoints request path/lines
      */
-    public setBreakpoints(args: DebugProtocol.SetBreakpointsArguments, requestSeq: number): void {
+    public setBreakpoints(args: ISetBreakpointsArgs, requestSeq: number): void {
         if (this._sourceMaps && args.source.path) {
             const argsPath = args.source.path;
 
-            args.source.path = this._sourceMaps.MapPathFromSource(argsPath) || argsPath;
-            args.lines = args.lines.map(line => {
+            const mappedPath = this._sourceMaps.MapPathFromSource(argsPath);
+            if (mappedPath) {
+                args.source.path = mappedPath;
+
+                // DebugProtocol doesn't send cols, but they need to be added from sourcemaps
+                args.cols = [];
+                args.lines = args.lines.map((line, i) => {
                     const mapped = this._sourceMaps.MapFromSource(argsPath, line, /*column=*/0);
-                return mapped ? mapped.line : line;
+                    if (mapped) {
+                        args.cols[i] = mapped.column;
+                        return mapped.line;
+                    } else {
+                        return line;
+                    }
                 });
+            }
 
             this._requestSeqToSetBreakpointsArgs.set(requestSeq, JSON.parse(JSON.stringify(args)));
         }
@@ -86,4 +87,12 @@ export class SourceMapTransformer implements IDebugTransformer {
             });
         }
     }
+
+    public scriptParsed(event: DebugProtocol.Event): void {
+        if (this._sourceMaps) {
+            // Send a dummy request just to get this file into the cache. SourceMaps can't trace a source file to a generated file
+            // unless its already in its cache, without falling back on heuristics which may be wrong.
+            this._sourceMaps.MapToSource(event.body.scriptUrl, 0, 0);
+        }
+    }
 }

test/adapter/sourceMaps/sourceMapTransformer.test.ts

@@ -44,32 +44,6 @@ suite('SourceMapTransformer', () => {
         transformerSMDisabled = null;
     });
 
-    suite('launch()', () => {
-        test('modifies args.path when present', () => {
-            const args = <ILaunchRequestArgs>{ workingDirectory: 'C:/code', program: 'authored.ts' };
-            const expected = <ILaunchRequestArgs>{ workingDirectory: 'C:/code', program: 'runtime.js' };
-
-            transformer.launch(args);
-            assert.deepEqual(args, expected);
-        });
-
-        test('doesn\'t do anything when args.path is missing, e.g. args.url was set', () => {
-            const args = <ILaunchRequestArgs>{ workingDirectory: 'C:/code' };
-            const expected = <ILaunchRequestArgs>{ workingDirectory: 'C:/code' };
-
-            transformer.launch(args);
-            assert.deepEqual(args, expected);
-        });
-
-        test('doesn\'t do anything when sourcemaps are disabled', () => {
-            const args = <ILaunchRequestArgs>{ workingDirectory: 'C:/code' };
-            const expected = <ILaunchRequestArgs>{ workingDirectory: 'C:/code' };
-
-            transformerSMDisabled.launch(args);
-            assert.deepEqual(args, expected);
-        });
-    });
-
     suite('setBreakpoints()', () => {
         function createArgs(path: string, lines: number[]): DebugProtocol.SetBreakpointsArguments {
             return {

testapp/.vscode/launch.json

@@ -1,18 +1,25 @@
 {
     "version": "0.1.0",
+    //"openDebug": "server=4711",
     "configurations": [
         {
             "name": "test chrome",
             "type": "webkit",
             "program": "out/client/index.html",
             //"runtimeArgs": ["http://localhost:8080/out/client/index.html"],
-            "sourceMaps": false,
+            "sourceMaps": true,
             "outDir": "out"
         },
         {
             "name": "attach to chrome",
             "type": "webkit",
             "port": 9222
+        },
+        {
+            "name": "node",
+            "type": "node",
+            "program": "out/client/test3.js",
+            "sourceMaps": true
         }
     ]
 }

webkit/webKitAdapterInterfaces.d.ts

@@ -17,6 +17,11 @@ interface IAttachRequestArgs extends DebugProtocol.AttachRequestArguments {
     address: string;
 }
 
+interface ISetBreakpointsArgs extends DebugProtocol.SetBreakpointsArguments {
+    /** DebugProtocol does not send cols, maybe it will someday, but this is used internally when a location is sourcemapped */
+    cols?: number[];
+}
+
 /*
  * The ResponseBody interfaces are copied from debugProtocol.d.ts which defines these inline in the Response interfaces.
  * They should always match those interfaces, see the original for comments.

webkit/webKitDebugAdapter.ts

@@ -201,6 +201,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
         const clientUrl = this.webkitUrlToClientUrl(script.url);
         this._scriptsByUrl.set(clientUrl, script);
         this._scriptsById.set(script.scriptId, script);
+        this._eventHandler(new Event('scriptParsed', { scriptUrl: clientUrl }));
 
         if (this._pendingBreakpointsByUrl.has(clientUrl)) {
             const pendingBreakpoint = this._pendingBreakpointsByUrl.get(clientUrl);
@@ -234,7 +235,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
         this._attach(args.port);
     }
 
-    public setBreakpoints(args: DebugProtocol.SetBreakpointsArguments): Promise<SetBreakpointsResponseBody> {
+    public setBreakpoints(args: ISetBreakpointsArgs): Promise<SetBreakpointsResponseBody> {
         let targetScript: WebKitProtocol.Debugger.Script;
         if (args.source.path) {
             targetScript = this._scriptsByUrl.get(canonicalizeUrl(args.source.path));
@@ -246,7 +247,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
             // DebugProtocol sends all current breakpoints for the script. Clear all scripts for the breakpoint then add all of them
             const setBreakpointsPFailOnError = this._setBreakpointsRequestQ
                 .then(() => this.clearAllBreakpoints(targetScript.scriptId))
-                .then(() => this._addBreakpoints(args.source.path, targetScript.scriptId, args.lines))
+                .then(() => this._addBreakpoints(args.source.path, targetScript.scriptId, args.lines, args.cols))
                 .then(responses => ({ breakpoints: this._webkitBreakpointResponsesToODPBreakpoints(targetScript, responses, args.lines) }));
 
             const setBreakpointsPTimeout = Utilities.promiseTimeout(setBreakpointsPFailOnError, /*timeoutMs*/2000, 'Set breakpoints request timed out');
@@ -266,10 +267,10 @@ export class WebKitDebugAdapter implements IDebugAdapter {
     }
 
 
-    private _addBreakpoints(sourcePath: string, scriptId: WebKitProtocol.Debugger.ScriptId, lines: number[]): Promise<WebKitProtocol.Debugger.SetBreakpointResponse[]> {
+    private _addBreakpoints(sourcePath: string, scriptId: WebKitProtocol.Debugger.ScriptId, lines: number[], cols?: number[]): Promise<WebKitProtocol.Debugger.SetBreakpointResponse[]> {
         // Call setBreakpoint for all breakpoints in the script simultaneously
         const responsePs = lines
-            .map(lineNumber => this._webKitConnection.debugger_setBreakpoint({ scriptId: scriptId, lineNumber }));
+            .map((lineNumber, i) => this._webKitConnection.debugger_setBreakpoint({ scriptId: scriptId, lineNumber, columnNumber: cols ? cols[i] : 0 }));
 
         // Join all setBreakpoint requests to a single promise
         return Promise.all(responsePs);