fixes #182, avoid injecting a duplicate link rel=stylesheet

[email protected]

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

Author: John Messerly

pkg/dev_compiler/lib/src/codegen/html_codegen.dart

@@ -43,26 +43,44 @@ String generateEntryHtml(HtmlSourceNode root, CompilerOptions options) {
   if (options.outputDart) return '${document.outerHtml}\n';
 
   var libraries = [];
-  var resources = [];
+  var resources = new Set();
   visitInPostOrder(root, (n) {
     if (n is DartSourceNode) libraries.add(n);
     if (n is ResourceSourceNode) resources.add(n);
   }, includeParts: false);
 
-  String mainLibraryName;
+  root.htmlResourceNodes.forEach((element, resource) {
+    // Make sure we don't try and add this node again.
+    resources.remove(resource);
+
+    var resourcePath = resourceOutputPath(resource.uri, root.uri);
+    if (resource.cachingHash != null) {
+      resourcePath = _addHash(resourcePath, resource.cachingHash);
+    }
+    var attrs = element.attributes;
+    if (attrs.containsKey('href')) {
+      attrs['href'] = resourcePath;
+    } else if (attrs.containsKey('src')) {
+      attrs['src'] = resourcePath;
+    }
+  });
+
   var fragment = new DocumentFragment();
   for (var resource in resources) {
     var resourcePath = resourceOutputPath(resource.uri, root.uri);
     var ext = path.extension(resourcePath);
     if (resource.cachingHash != null) {
       resourcePath = _addHash(resourcePath, resource.cachingHash);
     }
-    if (ext == '.css') {
-      fragment.nodes.add(_cssInclude(resourcePath));
-    } else if (ext == '.js') {
+    if (ext == '.js') {
       fragment.nodes.add(_libraryInclude(resourcePath));
+    } else if (ext == '.css') {
+      var stylesheetLink = '<link rel="stylesheet" href="$resourcePath">\n';
+      fragment.nodes.add(parseFragment(stylesheetLink));
     }
   }
+
+  String mainLibraryName;
   for (var lib in libraries) {
     var info = lib.info;
     if (info == null) continue;
@@ -82,10 +100,6 @@ String generateEntryHtml(HtmlSourceNode root, CompilerOptions options) {
 Node _libraryInclude(String jsUrl) =>
     parseFragment('<script src="$jsUrl"></script>\n');
 
-/// A tag that loads the .css code.
-Node _cssInclude(String cssUrl) =>
-    parseFragment('<link rel="stylesheet" href="$cssUrl">\n');
-
 /// A script tag that invokes the main function on the entry point library.
 Node _invokeMain(String mainLibraryName) {
   var code = mainLibraryName == null

pkg/dev_compiler/lib/src/dependency_graph.dart

@@ -5,7 +5,7 @@
 /// Tracks the shape of the import/export graph and dependencies between files.
 library dev_compiler.src.dependency_graph;
 
-import 'dart:collection' show HashSet;
+import 'dart:collection' show HashSet, HashMap;
 
 import 'package:analyzer/analyzer.dart' show parseDirectives;
 import 'package:analyzer/src/generated/ast.dart'
@@ -21,7 +21,7 @@ import 'package:analyzer/src/generated/ast.dart'
 import 'package:analyzer/src/generated/engine.dart'
     show ParseDartTask, AnalysisContext;
 import 'package:analyzer/src/generated/source.dart' show Source, SourceKind;
-import 'package:html/dom.dart' show Document, Node;
+import 'package:html/dom.dart' show Document, Node, Element;
 import 'package:html/parser.dart' as html;
 import 'package:logging/logging.dart' show Logger, Level;
 import 'package:path/path.dart' as path;
@@ -167,6 +167,10 @@ class HtmlSourceNode extends SourceNode {
   /// Parsed document, updated whenever [update] is invoked.
   Document document;
 
+  /// Tracks resource files referenced from HTML nodes, e.g.
+  /// `<link rel=stylesheet href=...>` and `<img src=...>`
+  final htmlResourceNodes = new HashMap<Element, ResourceSourceNode>();
+
   HtmlSourceNode(SourceGraph graph, Uri uri, Source source)
       : runtimeDeps = graph.runtimeDeps,
         super(graph, uri, source);
@@ -199,16 +203,24 @@ class HtmlSourceNode extends SourceNode {
         scripts = newScripts;
       }
 
+      // TODO(jmesserly): simplify the design here. Ideally we wouldn't need
+      // to track user-defined CSS, images, etc. Also we don't have a clear
+      // way to distinguish runtime injected resources, like messages.css, from
+      // user-defined files.
+      htmlResourceNodes.clear();
       var newResources = new Set<SourceNode>();
       for (var resource in graph.resources) {
         newResources.add(graph.nodeFromUri(uri.resolve(resource)));
       }
       for (var tag in document.querySelectorAll('link[rel="stylesheet"]')) {
-        newResources
-            .add(graph.nodeFromUri(uri.resolve(tag.attributes['href'])));
+        var res = graph.nodeFromUri(uri.resolve(tag.attributes['href']));
+        htmlResourceNodes[tag] = res;
+        newResources.add(res);
       }
-      for (var tag in document.querySelectorAll('img')) {
-        newResources.add(graph.nodeFromUri(uri.resolve(tag.attributes['src'])));
+      for (var tag in document.querySelectorAll('img[src]')) {
+        var res = graph.nodeFromUri(uri.resolve(tag.attributes['src']));
+        htmlResourceNodes[tag] = res;
+        newResources.add(res);
       }
       if (!_same(newResources, resources)) {
         structureChanged = true;

pkg/dev_compiler/test/codegen/expect/sunflower/sunflower.html

@@ -23,8 +23,7 @@ <h1>drfibonacci's Sunflower Spectacular</h1>
       <p id="notes"> </p>
     </footer>
 
-    <link rel="stylesheet" href="sunflower.css">
-<script src="dev_compiler/runtime/harmony_feature_check.js"></script>
+    <script src="dev_compiler/runtime/harmony_feature_check.js"></script>
 <script src="dev_compiler/runtime/dart_runtime.js"></script>
 <script src="dev_compiler/runtime/dart/core.js"></script>
 <script src="dev_compiler/runtime/dart/collection.js"></script>