Remove redundancy in source map entries.

R=johnniwinther@google.com

Review URL: https://codereview.chromium.org//228003005

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@34866 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart b/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart
index f5052c3..a419a7b 100644
--- a/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart
+++ b/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart
@@ -1696,9 +1696,9 @@
     // [:getLine:] to transform offsets to line numbers in [SourceMapBuilder].
     SourceFile compiledFile = new StringSourceFile(null, code);
     SourceMapBuilder sourceMapBuilder =
-            new SourceMapBuilder(sourceMapUri, fileUri);
+            new SourceMapBuilder(sourceMapUri, fileUri, compiledFile);
     buffer.forEachSourceLocation(sourceMapBuilder.addMapping);
-    String sourceMap = sourceMapBuilder.build(compiledFile);
+    String sourceMap = sourceMapBuilder.build();
     compiler.outputProvider(name, 'js.map')
         ..add(sourceMap)
         ..close();
diff --git a/sdk/lib/_internal/compiler/implementation/source_map_builder.dart b/sdk/lib/_internal/compiler/implementation/source_map_builder.dart
index 3910615..16697e9 100644
--- a/sdk/lib/_internal/compiler/implementation/source_map_builder.dart
+++ b/sdk/lib/_internal/compiler/implementation/source_map_builder.dart
@@ -20,6 +20,7 @@
   final Uri uri;
   final Uri fileUri;
 
+  SourceFile targetFile;
   List<SourceMapEntry> entries;
 
   Map<String, int> sourceUrlMap;
@@ -35,7 +36,7 @@
   int previousSourceNameIndex;
   bool firstEntryInLine;
 
-  SourceMapBuilder(this.uri, this.fileUri) {
+  SourceMapBuilder(this.uri, this.fileUri, this.targetFile) {
     entries = new List<SourceMapEntry>();
 
     sourceUrlMap = new Map<String, int>();
@@ -52,7 +53,59 @@
     firstEntryInLine = true;
   }
 
+  resetPreviousSourceLocation() {
+    previousSourceUrlIndex = 0;
+    previousSourceLine = 0;
+    previousSourceColumn = 0;
+    previousSourceNameIndex = 0;
+  }
+
+  updatePreviousSourceLocation(SourceFileLocation sourceLocation) {
+    previousSourceLine = sourceLocation.getLine();
+    previousSourceColumn = sourceLocation.getColumn();
+    String sourceUrl = sourceLocation.getSourceUrl();
+    previousSourceUrlIndex = indexOf(sourceUrlList, sourceUrl, sourceUrlMap);
+    String sourceName = sourceLocation.getSourceName();
+    if (sourceName != null) {
+      previousSourceNameIndex =
+          indexOf(sourceNameList, sourceName, sourceNameMap);
+    }
+  }
+
+  bool sameAsPreviousLocation(SourceFileLocation sourceLocation) {
+    if (sourceLocation == null) {
+      return true;
+    }
+    int sourceUrlIndex =
+        indexOf(sourceUrlList, sourceLocation.getSourceUrl(), sourceUrlMap);
+    return
+       sourceUrlIndex == previousSourceUrlIndex &&
+       sourceLocation.getLine() == previousSourceLine &&
+       sourceLocation.getColumn() == previousSourceColumn;
+  }
+
   void addMapping(int targetOffset, SourceFileLocation sourceLocation) {
+
+    bool sameLine(int position, otherPosition) {
+      return targetFile.getLine(position) == targetFile.getLine(otherPosition);
+    }
+
+    if (!entries.isEmpty && sameLine(targetOffset, entries.last.targetOffset)) {
+      if (sameAsPreviousLocation(sourceLocation)) {
+        // The entry points to the same source location as the previous entry in
+        // the same line, hence it is not needed for the source map.
+        //
+        // TODO(zarah): Remove this check and make sure that [addMapping] is not
+        // called for this position. Instead, when consecutive lines in the
+        // generated code point to the same source location, record this and use
+        // it to generate the entries of the source map.
+        return;
+      }
+    }
+
+    if (sourceLocation != null) {
+      updatePreviousSourceLocation(sourceLocation);
+    }
     entries.add(new SourceMapEntry(sourceLocation, targetOffset));
   }
 
@@ -69,7 +122,8 @@
     buffer.write(']');
   }
 
-  String build(SourceFile targetFile) {
+  String build() {
+    resetPreviousSourceLocation();
     StringBuffer mappingsBuffer = new StringBuffer();
     entries.forEach((SourceMapEntry entry) => writeEntry(entry, targetFile,
                                                          mappingsBuffer));
@@ -127,12 +181,9 @@
 
     int sourceUrlIndex = indexOf(sourceUrlList, sourceUrl, sourceUrlMap);
     encodeVLQ(output, sourceUrlIndex - previousSourceUrlIndex);
-    previousSourceUrlIndex = sourceUrlIndex;
-
     encodeVLQ(output, sourceLine - previousSourceLine);
-    previousSourceLine = sourceLine;
     encodeVLQ(output, sourceColumn - previousSourceColumn);
-    previousSourceColumn = sourceColumn;
+    updatePreviousSourceLocation(entry.sourceLocation);
 
     if (sourceName == null) {
       return;
@@ -140,7 +191,7 @@
 
     int sourceNameIndex = indexOf(sourceNameList, sourceName, sourceNameMap);
     encodeVLQ(output, sourceNameIndex - previousSourceNameIndex);
-    previousSourceNameIndex = sourceNameIndex;
+
   }
 
   int indexOf(List<String> list, String value, Map<String, int> map) {
diff --git a/tests/compiler/dart2js/source_map_consistency_helper.dart b/tests/compiler/dart2js/source_map_consistency_helper.dart
deleted file mode 100644
index 37a75a9..0000000
--- a/tests/compiler/dart2js/source_map_consistency_helper.dart
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import 'dart:io';
-import 'dart:convert';
-import 'dart:async';
-
-import 'package:path/path.dart' as path;
-import 'package:expect/expect.dart';
-import 'package:source_maps/source_maps.dart';
-
-checkConsistency(Uri outUri) {
-  print('Accessing $outUri');
-  File sourceFile = new File.fromUri(outUri);
-  Expect.isTrue(sourceFile.existsSync());
-  String mapName = getMapReferenceFromJsOutput(sourceFile.readAsStringSync());
-  Uri mapUri = outUri.resolve(mapName);
-  print('Accessing $mapUri');
-  File mapFile = new File.fromUri(mapUri);
-  Expect.isTrue(mapFile.existsSync());
-  SingleMapping sourceMap = new SingleMapping.fromJson(
-      JSON.decode(mapFile.readAsStringSync()));
-  Expect.equals(outUri, mapUri.resolve(sourceMap.targetUrl));
-  print('Checking sources');
-  sourceMap.urls.forEach((String url) {
-    Expect.isTrue(new File.fromUri(mapUri.resolve(url)).existsSync());
-  });
-}
-
-String getMapReferenceFromJsOutput(String file) {
-  List<String> out = file.split('\n');
-  String mapReference = out[out.length - 3]; // #sourceMappingURL=<url>
-  Expect.isTrue(mapReference.startsWith('//# sourceMappingURL='));
-  return mapReference.substring(mapReference.indexOf('=') + 1);
-}
-
-copyDirectory(Directory sourceDir, Directory destinationDir) {
-  sourceDir.listSync().forEach((FileSystemEntity element) {
-    String newPath = path.join(destinationDir.path,
-                               path.basename(element.path));
-    if (element is File) {
-      element.copySync(newPath);
-    } else if (element is Directory) {
-      Directory newDestinationDir = new Directory(newPath);
-      newDestinationDir.createSync();
-      copyDirectory(element, newDestinationDir);
-    }
-  });
-}
-
-Future<Directory> createTempDir() {
-  return Directory.systemTemp
-      .createTemp('sourceMap_test-')
-      .then((Directory dir) {
-    return dir;
-  });
-}
\ No newline at end of file
diff --git a/tests/compiler/dart2js/source_map_d2js_consistency_test.dart b/tests/compiler/dart2js/source_map_d2js_validity_test.dart
similarity index 92%
rename from tests/compiler/dart2js/source_map_d2js_consistency_test.dart
rename to tests/compiler/dart2js/source_map_d2js_validity_test.dart
index 4b25201..cbc5ed8 100644
--- a/tests/compiler/dart2js/source_map_d2js_consistency_test.dart
+++ b/tests/compiler/dart2js/source_map_d2js_validity_test.dart
@@ -4,9 +4,10 @@
 
 import 'dart:async';
 import 'dart:io';
+
 import 'package:async_helper/async_helper.dart';
 
-import 'source_map_consistency_helper.dart';
+import 'source_map_validator_helper.dart';
 import '../../../sdk/lib/_internal/compiler/implementation/dart2js.dart' as entry;
 
 void main() {
@@ -19,7 +20,7 @@
       return result.then((_) {
         Uri uri =
             new Uri.file('${tmpDir.path}/out.js', windows: Platform.isWindows);
-        checkConsistency(uri);
+        validateSourceMap(uri);
 
         print("Deleting '${tmpDir.path}'.");
         tmpDir.deleteSync(recursive: true);
diff --git a/tests/compiler/dart2js/source_map_pub_build_consistency_test.dart b/tests/compiler/dart2js/source_map_pub_build_validity_test.dart
similarity index 90%
rename from tests/compiler/dart2js/source_map_pub_build_consistency_test.dart
rename to tests/compiler/dart2js/source_map_pub_build_validity_test.dart
index f8dceb5..8aabf5d 100644
--- a/tests/compiler/dart2js/source_map_pub_build_consistency_test.dart
+++ b/tests/compiler/dart2js/source_map_pub_build_validity_test.dart
@@ -7,7 +7,7 @@
 import 'package:path/path.dart' as path;
 import 'package:async_helper/async_helper.dart';
 
-import 'source_map_consistency_helper.dart';
+import 'source_map_validator_helper.dart';
 
 void main() {
   asyncTest(() => createTempDir().then((Directory tmpDir) {
@@ -24,7 +24,7 @@
     return Process.run(command, ['build','--mode=debug'],
         workingDirectory: tmpDir.path).then((process) {
       print(process.stdout);
-      checkConsistency(new Uri.file(file, windows: Platform.isWindows));
+      validateSourceMap(new Uri.file(file, windows: Platform.isWindows));
       print("Deleting '${tmpDir.path}'.");
       tmpDir.deleteSync(recursive: true);
     });
diff --git a/tests/compiler/dart2js/source_map_validator_helper.dart b/tests/compiler/dart2js/source_map_validator_helper.dart
new file mode 100644
index 0000000..934a63c
--- /dev/null
+++ b/tests/compiler/dart2js/source_map_validator_helper.dart
@@ -0,0 +1,89 @@
+// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+import 'dart:convert';
+import 'dart:async';
+
+import 'package:path/path.dart' as path;
+import 'package:expect/expect.dart';
+import 'package:source_maps/source_maps.dart';
+
+validateSourceMap(Uri targetUri) {
+  Uri mapUri = getMapUri(targetUri);
+  SingleMapping sourceMap = getSourceMap(mapUri);
+  checkFileReferences(targetUri, mapUri, sourceMap);
+  checkRedundancy(sourceMap);
+}
+
+checkFileReferences(Uri targetUri, Uri mapUri, SingleMapping sourceMap) {
+  Expect.equals(targetUri, mapUri.resolve(sourceMap.targetUrl));
+  print('Checking sources');
+  sourceMap.urls.forEach((String url) {
+    Expect.isTrue(new File.fromUri(mapUri.resolve(url)).existsSync());
+  });
+}
+
+checkRedundancy(SingleMapping sourceMap) {
+  sourceMap.lines.forEach((TargetLineEntry line) {
+    TargetEntry previous = null;
+    for (TargetEntry next in line.entries) {
+      if (previous != null) {
+        Expect.isFalse(sameSourcePoint(previous, next),
+            '$previous and $next are consecutive entries on line $line in the '
+            'source map but point to same source locations');
+      }
+      previous = next;
+    }
+  });
+}
+
+sameSourcePoint(TargetEntry entry, TargetEntry otherEntry) {
+  return
+      (entry.sourceUrlId == otherEntry.sourceUrlId) &&
+      (entry.sourceLine == otherEntry.sourceLine) &&
+      (entry.sourceColumn == otherEntry.sourceColumn) &&
+      (entry.sourceNameId == otherEntry.sourceNameId);
+}
+
+Uri getMapUri(Uri targetUri) {
+  print('Accessing $targetUri');
+  File targetFile = new File.fromUri(targetUri);
+  Expect.isTrue(targetFile.existsSync());
+  List<String> target = targetFile.readAsStringSync().split('\n');
+  String mapReference = target[target.length - 3]; // #sourceMappingURL=<url>
+  Expect.isTrue(mapReference.startsWith('//# sourceMappingURL='));
+  String mapName = mapReference.substring(mapReference.indexOf('=') + 1);
+  return targetUri.resolve(mapName);
+}
+
+SingleMapping getSourceMap(Uri mapUri) {
+  print('Accessing $mapUri');
+  File mapFile = new File.fromUri(mapUri);
+  Expect.isTrue(mapFile.existsSync());
+  return new SingleMapping.fromJson(
+      JSON.decode(mapFile.readAsStringSync()));
+}
+
+copyDirectory(Directory sourceDir, Directory destinationDir) {
+  sourceDir.listSync().forEach((FileSystemEntity element) {
+    String newPath = path.join(destinationDir.path,
+                               path.basename(element.path));
+    if (element is File) {
+      element.copySync(newPath);
+    } else if (element is Directory) {
+      Directory newDestinationDir = new Directory(newPath);
+      newDestinationDir.createSync();
+      copyDirectory(element, newDestinationDir);
+    }
+  });
+}
+
+Future<Directory> createTempDir() {
+  return Directory.systemTemp
+      .createTemp('sourceMap_test-')
+      .then((Directory dir) {
+    return dir;
+  });
+}