Eliminate multi-callback structure for LibraryLoader.

pkg/compiler/lib/src/library_loader.dart

 // Copyright (c) 2012, 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.
 
 library dart2js.library_loader;
 
 import 'dart:async';
 
 import 'common/names.dart' show Uris;
 import 'common/tasks.dart' show CompilerTask, Measurer;
@@ skipping 11 matching lines @@
         DeferredLoaderGetterElementX,
         ErroneousElementX,
         ExportElementX,
         ImportElementX,
         LibraryElementX,
         LibraryDependencyElementX,
         PrefixElementX,
         SyntheticImportElement;
 import 'enqueue.dart' show DeferredAction;
 import 'environment.dart';
+import 'io/source_file.dart' show SourceFile;
+import 'patch_parser.dart' show PatchParserTask;
 import 'resolved_uri_translator.dart';
 import 'script.dart';
 import 'serialization/serialization.dart' show LibraryDeserializer;
 import 'tree/tree.dart';
 import 'util/util.dart' show Link, LinkBuilder;
 
+import 'package:front_end/src/fasta/scanner.dart' show Token;
+
 typedef Future<Iterable<LibraryElement>> ReuseLibrariesFunction(
     Iterable<LibraryElement> libraries);
 
+typedef Uri PatchResolverFunction(String dartLibraryPath);
+
 /**
  * [CompilerTask] for loading libraries and setting up the import/export scopes.
  *
  * The library loader uses four different kinds of URIs in different parts of
  * the loading process.
  *
  * ## User URI ##
  *
  * A 'user URI' is a URI provided by the user in code and as the main entry URI
  * at the command line. These generally come in 3 versions:
@@ skipping 80 matching lines @@
  * do _not_ resolve to the same library when the package root URI happens to
  * point to the 'packages' folder.
  *
  */
 abstract class LibraryLoaderTask implements LibraryProvider, CompilerTask {
   factory LibraryLoaderTask(
       ResolvedUriTranslator uriTranslator,
       ScriptLoader scriptLoader,
       ElementScanner scriptScanner,
       LibraryDeserializer deserializer,
-      LibraryLoaderListener listener,
+      PatchResolverFunction patchResolverFunc,
+      PatchParserTask patchParser,
       Environment environment,
       DiagnosticReporter reporter,
       Measurer measurer) = _LibraryLoaderTask;
 
   /// Returns all libraries that have been loaded.
   Iterable<LibraryElement> get libraries;
 
-  /// Loads the library specified by the [resolvedUri] and returns its
-  /// [LibraryElement].
+  /// Loads the library specified by the [resolvedUri] and returns the
+  /// [LoadedLibraries] that were loaded to load the specified uri. The
+  /// [LibraryElement] itself can be found by calling
+  /// `loadedLibraries.rootLibrary`

[Siggi Cherem (dart-lang), 2017/03/24 17:40:28]

nit: add a `.` at the end.

[Emily Fortuna, 2017/03/24 18:30:19]

Done.

   ///
   /// If the library is not already loaded, the method creates the
   /// [LibraryElement] for the library and computes the import/export scope,
   /// loading and computing the import/export scopes of all required libraries
   /// in the process. The method handles cyclic dependency between libraries.
   ///
   /// If [skipFileWithPartOfTag] is `true`, `null` is returned if the
   /// compilation unit for [resolvedUri] contains a `part of` tag. This is only
   /// used for analysis through [Compiler.analyzeUri].
-  Future<LibraryElement> loadLibrary(Uri resolvedUri,
+  Future<LoadedLibraries> loadLibrary(Uri resolvedUri,
       {bool skipFileWithPartOfTag: false});
 
   /// Reset the library loader task to prepare for compilation. If provided,
   /// libraries matching [reuseLibrary] are reused.
   ///
   /// This method is used for incremental compilation.
   void reset({bool reuseLibrary(LibraryElement library)});
 
   /// Asynchronous version of [reset].
   Future resetAsync(Future<bool> reuseLibrary(LibraryElement library));
 
   /// Similar to [resetAsync] but [reuseLibrary] maps all libraries to a list
   /// of libraries that can be reused.
   Future<Null> resetLibraries(ReuseLibrariesFunction reuseLibraries);
 
   // TODO(johnniwinther): Move these to a separate interface.
   /// Register a deferred action to be performed during resolution.
   void registerDeferredAction(DeferredAction action);
 
   /// Returns the deferred actions registered since the last call to
   /// [pullDeferredActions].
   Iterable<DeferredAction> pullDeferredActions();
+
+  /// The locations of js patch-files relative to the sdk-descriptors.
+  static const _patchLocations = const <String, String>{
+    "async": "_internal/js_runtime/lib/async_patch.dart",
+    "collection": "_internal/js_runtime/lib/collection_patch.dart",
+    "convert": "_internal/js_runtime/lib/convert_patch.dart",
+    "core": "_internal/js_runtime/lib/core_patch.dart",
+    "developer": "_internal/js_runtime/lib/developer_patch.dart",
+    "io": "_internal/js_runtime/lib/io_patch.dart",
+    "isolate": "_internal/js_runtime/lib/isolate_patch.dart",
+    "math": "_internal/js_runtime/lib/math_patch.dart",
+    "mirrors": "_internal/js_runtime/lib/mirrors_patch.dart",
+    "typed_data": "_internal/js_runtime/lib/typed_data_patch.dart",
+    "_internal": "_internal/js_runtime/lib/internal_patch.dart"
+  };
+
+  /// Returns the location of the patch-file associated with [libraryName]
+  /// resolved from [plaformConfigUri].
+  ///
+  /// Returns null if there is none.
+  static Uri resolvePatchUri(String libraryName, Uri platformConfigUri) {
+    String patchLocation = _patchLocations[libraryName];
+    if (patchLocation == null) return null;
+    return platformConfigUri.resolve(patchLocation);
+  }
 }
 
 /// Interface for an entity that provide libraries. For instance from normal
 /// library loading or from deserialization.
 // TODO(johnniwinther): Use this to integrate deserialized libraries better.
 abstract class LibraryProvider {
   /// Looks up the library with the [canonicalUri].
   LibraryElement lookupLibrary(Uri canonicalUri);
 }
 
@@ skipping 103 matching lines @@
   final ScriptLoader scriptLoader;
 
   /// Provides a diet element model from a script file containing information
   /// about imports and exports. Used when loading libraries from source.
   final ElementScanner scanner;
 
   /// Provides a diet element model for a library. Used when loading libraries
   /// from a serialized form.
   final LibraryDeserializer deserializer;
 
-  /// Hooks to inform others about progress done by this loader.
-  // TODO(sigmund): move away from this.
-  final LibraryLoaderListener listener;
-
   /// Definitions provided via the `-D` command line flags. Used to resolve
   /// conditional imports.
   final Environment environment;
 
+  // TODO(efortuna): Don't pass PatchParserTask here.
+  final PatchParserTask _patchParserTask;
+
+  /// Function that accepts the string name of a library and returns the
+  /// path to the corresponding patch file. This is a function that is passed in
+  /// because our test mock_compiler subclasses compiler.
+  // TODO(efortuna): Refactor mock_compiler to not do this.

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

thanks for documenting so nicely the issue :)

I have a couple ideas we can try, but we can totally delay these for a separate
CL:
 - idea 1: make this field non-final and public, then the mock compiler can set
it directly and we can set it's default implementation in a way that allows us
to remove the extra indirection with apiimpl.

 - idea 2: make patchLocations and platofmConfigUri a parameter of the loader
task instead. The algorithm in the mock-compiler is not that different than what
we have here, so if we pass a special value for configUri and the map, we might
have something clean to do.

 - idea 3: make the mock compiler use a different path for the PATCH_CORE, to
match exactly what's expected in the map we use here.


I like #2 or #3 - if #3 is simple enough, I'd go with that. If we weren't moving
to load from kernel, I'd make the compiler more configurable about where
patch-files are with #2, but seems unnecessary at this moment.

[Emily Fortuna, 2017/03/24 18:30:19]

makes sense. upcoming CL!

+  final PatchResolverFunction _patchResolverFunc;
+
   List<DeferredAction> _deferredActions = <DeferredAction>[];
 
   final DiagnosticReporter reporter;
 
   _LibraryLoaderTask(
       this.uriTranslator,
       this.scriptLoader,
       this.scanner,
       this.deserializer,
-      this.listener,
+      this._patchResolverFunc,
+      this._patchParserTask,
       this.environment,
       this.reporter,
       Measurer measurer)
       : super(measurer);
 
   String get name => 'LibraryLoader';
 
   final Map<Uri, LibraryElement> libraryCanonicalUriMap =
       new Map<Uri, LibraryElement>();
   final Map<Uri, LibraryElement> libraryResourceUriMap =
       new Map<Uri, LibraryElement>();
   final Map<String, LibraryElement> libraryNames =
       new Map<String, LibraryElement>();
 
-  LibraryDependencyHandler currentHandler;
-
   Iterable<LibraryElement> get libraries => libraryCanonicalUriMap.values;
 
   LibraryElement lookupLibrary(Uri canonicalUri) {
     return libraryCanonicalUriMap[canonicalUri];
   }
 
   void reset({bool reuseLibrary(LibraryElement library)}) {
     measure(() {
-      assert(currentHandler == null);
-
       Iterable<LibraryElement> reusedLibraries = null;
       if (reuseLibrary != null) {
         reusedLibraries = measureSubtask(_reuseLibrarySubtaskName, () {
           // Call [toList] to force eager calls to [reuseLibrary].
           return libraryCanonicalUriMap.values.where(reuseLibrary).toList();
         });
       }
 
       resetImplementation(reusedLibraries);
     });
   }
 
   void resetImplementation(Iterable<LibraryElement> reusedLibraries) {
     measure(() {
       libraryCanonicalUriMap.clear();
       libraryResourceUriMap.clear();
       libraryNames.clear();
 
       if (reusedLibraries != null) {
         reusedLibraries.forEach(mapLibrary);
       }
     });
   }
 
   Future resetAsync(Future<bool> reuseLibrary(LibraryElement library)) {
     return measure(() {
-      assert(currentHandler == null);
-
       Future<LibraryElement> wrapper(LibraryElement library) {
         try {
           return reuseLibrary(library)
               .then((bool reuse) => reuse ? library : null);
         } catch (exception, trace) {
           reporter.onCrashInUserCode(
               'Uncaught exception in reuseLibrary', exception, trace);
           rethrow;
         }
       }
 
       List<Future<LibraryElement>> reusedLibrariesFuture = measureSubtask(
           _reuseLibrarySubtaskName,
           () => libraryCanonicalUriMap.values.map(wrapper).toList());
 
       return Future
           .wait(reusedLibrariesFuture)
           .then((Iterable<LibraryElement> reusedLibraries) {
         resetImplementation(reusedLibraries.where((e) => e != null));
       });
     });
   }
 
   Future<Null> resetLibraries(
       Future<Iterable<LibraryElement>> reuseLibraries(
           Iterable<LibraryElement> libraries)) {
-    assert(currentHandler == null);
     return measureSubtask(_reuseLibrarySubtaskName, () {
       return new Future<Iterable<LibraryElement>>(() {
         // Wrap in Future to shield against errors in user code.
         return reuseLibraries(libraryCanonicalUriMap.values);
       }).catchError((exception, StackTrace trace) {
         reporter.onCrashInUserCode(
             'Uncaught exception in reuseLibraries', exception, trace);
         throw exception; // Async rethrow.
       }).then((Iterable<LibraryElement> reusedLibraries) {
         measure(() {
           resetImplementation(reusedLibraries);
         });
       });
     });
   }
 
   /// Insert [library] in the internal maps. Used for compiler reuse.
   void mapLibrary(LibraryElement library) {
     libraryCanonicalUriMap[library.canonicalUri] = library;
 
     Uri resourceUri = library.entryCompilationUnit.script.resourceUri;
     libraryResourceUriMap[resourceUri] = library;
 
     if (library.hasLibraryName) {
       String name = library.libraryName;
       libraryNames[name] = library;
     }
   }
 
-  Future<LibraryElement> loadLibrary(Uri resolvedUri,
-      {bool skipFileWithPartOfTag: false}) {
-    return measure(() {
-      assert(currentHandler == null);
-      // TODO(johnniwinther): Ensure that currentHandler correctly encloses the
-      // loading of a library cluster.
-      currentHandler = new LibraryDependencyHandler(this);
-      return createLibrary(
-              currentHandler, null, resolvedUri, NO_LOCATION_SPANNABLE,
-              skipFileWithPartOfTag: skipFileWithPartOfTag)
-          .then((LibraryElement library) {
-        if (library == null) {
-          currentHandler = null;
-          return null;
-        }
-        return reporter.withCurrentElement(library, () {
-          return measure(() {
-            currentHandler.computeExports();
-            LoadedLibraries loadedLibraries = new _LoadedLibraries(library,
-                currentHandler.newLibraries, currentHandler.nodeMap, this);
-            currentHandler = null;
-            return listener
-                .onLibrariesLoaded(loadedLibraries)
-                .then((_) => library);
-          });
+  Future<LoadedLibraries> loadLibrary(Uri resolvedUri,
+      {bool skipFileWithPartOfTag: false}) async {

[Siggi Cherem (dart-lang), 2017/03/24 17:40:28]

seems like you don't need async here

[Emily Fortuna, 2017/03/24 18:30:19]

Done.

+    return measure(() async {
+      LibraryDependencyHandler loader = new LibraryDependencyHandler(this);

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

nit: maybe rename loader as handler?

[Emily Fortuna, 2017/03/24 18:30:19]

Done.

+      LibraryElement library = await createLibrary(
+          loader, null, resolvedUri, NO_LOCATION_SPANNABLE,
+          skipFileWithPartOfTag: skipFileWithPartOfTag);
+      if (library == null) {

[Siggi Cherem (dart-lang), 2017/03/24 17:40:28]

style nit: we don't use the {} on this fast-exit code, just:

  if (library == null) return null;

[Emily Fortuna, 2017/03/24 18:30:19]

Done.

+        return null;
+      }
+      return reporter.withCurrentElement(library, () {
+        return measure(() {
+          loader.computeExports();
+          return new _LoadedLibraries(library, loader.newLibraries, this);
         });
       });
     });
   }
 
   /**
    * Processes the library tags in [library].
    *
    * The imported/exported libraries are loaded and processed recursively but
    * the import/export scopes are not set up.
@@ skipping 72 matching lines @@
           Part part = tag;
           StringNode uri = part.uri;
           Uri resolvedUri = base.resolve(uri.dartString.slowToString());
           tagState.checkTag(TagState.PART, part, reporter);
           return scanPart(part, resolvedUri, library);
         } else {
           reporter.internalError(tag, "Unhandled library tag.");
         }
       });
     }).then((_) {
-      return listener.onLibraryScanned(library, handler);

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

woo hoo!

-    }).then((_) {
       return reporter.withCurrentElement(library, () {
         checkDuplicatedLibraryName(library);
 
         // Import dart:core if not already imported.
         if (!importsDartCore && library.canonicalUri != Uris.dart_core) {
           return createLibrary(handler, null, Uris.dart_core, library)
               .then((LibraryElement coreLibrary) {
             handler.registerDependency(
                 library,
                 new SyntheticImportElement(
@@ skipping 89 matching lines @@
   }
 
   /// Loads the deserialized [library] with the [handler].
   ///
   /// All libraries imported or exported transitively from [library] will be
   /// loaded as well.
   Future<LibraryElement> loadDeserializedLibrary(
       LibraryDependencyHandler handler, LibraryElement library) {
     libraryCanonicalUriMap[library.canonicalUri] = library;
     handler.registerNewLibrary(library);
-    return listener.onLibraryScanned(library, handler).then((_) {
-      return Future.forEach(library.imports, (ImportElement import) {
-        Uri resolvedUri = library.canonicalUri.resolveUri(import.uri);
+    return Future.forEach(library.imports, (ImportElement import) {
+      Uri resolvedUri = library.canonicalUri.resolveUri(import.uri);
+      return createLibrary(handler, library, resolvedUri, library);
+    }).then((_) {
+      return Future.forEach(library.exports, (ExportElement export) {
+        Uri resolvedUri = library.canonicalUri.resolveUri(export.uri);
         return createLibrary(handler, library, resolvedUri, library);
       }).then((_) {
-        return Future.forEach(library.exports, (ExportElement export) {
-          Uri resolvedUri = library.canonicalUri.resolveUri(export.uri);
-          return createLibrary(handler, library, resolvedUri, library);
-        }).then((_) {
-          // TODO(johnniwinther): Shouldn't there be an [ImportElement] for the
-          // implicit import of dart:core?
-          return createLibrary(handler, library, Uris.dart_core, library);
-        }).then((_) => library);
-      });
+        // TODO(johnniwinther): Shouldn't there be an [ImportElement] for the
+        // implicit import of dart:core?
+        return createLibrary(handler, library, Uris.dart_core, library);
+      }).then((_) => library);
     });
   }
 
   Future<Script> _readScript(
       Spannable spannable, Uri readableUri, Uri resolvedUri) {
     if (readableUri == null) {
       return new Future.value(new Script.synthetic(resolvedUri));
     } else {
       return scriptLoader.readScript(readableUri, spannable);
     }
   }
 
   /**
    * Create (or reuse) a library element for the library specified by the
    * [resolvedUri].
    *
    * If a new library is created, the [handler] is notified.
    */
   Future<LibraryElement> createLibrary(LibraryDependencyHandler handler,
       LibraryElement importingLibrary, Uri resolvedUri, Spannable spannable,
-      {bool skipFileWithPartOfTag: false}) {
+      {bool skipFileWithPartOfTag: false}) async {
     Uri readableUri =
         uriTranslator.translate(importingLibrary, resolvedUri, spannable);
     LibraryElement library = libraryCanonicalUriMap[resolvedUri];
     if (library != null) {
       return new Future.value(library);
     }
-    return deserializer.readLibrary(resolvedUri).then((LibraryElement library) {
-      if (library != null) {
-        return loadDeserializedLibrary(handler, library);
-      }
-      return reporter.withCurrentElement(importingLibrary, () {
-        return _readScript(spannable, readableUri, resolvedUri)
-            .then((Script script) {
-          if (script == null) return null;
-          LibraryElement element =
-              createLibrarySync(handler, script, resolvedUri);
-          CompilationUnitElementX compilationUnit =
-              element.entryCompilationUnit;
-          if (compilationUnit.partTag != null) {
-            if (skipFileWithPartOfTag) {
-              // TODO(johnniwinther): Avoid calling [listener.onLibraryCreated]
-              // for this library.
-              libraryCanonicalUriMap.remove(resolvedUri);
-              return null;
-            }
-            if (importingLibrary == null) {
-              DiagnosticMessage error = reporter.withCurrentElement(
-                  compilationUnit,
-                  () => reporter.createMessage(
-                      compilationUnit.partTag, MessageKind.MAIN_HAS_PART_OF));
-              reporter.reportError(error);
-            } else {
-              DiagnosticMessage error = reporter.withCurrentElement(
-                  compilationUnit,
-                  () => reporter.createMessage(
-                      compilationUnit.partTag, MessageKind.IMPORT_PART_OF));
-              DiagnosticMessage info = reporter.withCurrentElement(
-                  importingLibrary,
-                  () => reporter.createMessage(
-                      spannable, MessageKind.IMPORT_PART_OF_HERE));
-              reporter.reportError(error, [info]);
-            }
+    library = await deserializer.readLibrary(resolvedUri);
+    if (library != null) {
+      return loadDeserializedLibrary(handler, library);
+    }
+    return reporter.withCurrentElement(importingLibrary, () {
+      return _readScript(spannable, readableUri, resolvedUri)
+          .then((Script script) async {
+        if (script == null) return null;
+        LibraryElement element =
+            createLibrarySync(handler, script, resolvedUri);
+        CompilationUnitElementX compilationUnit = element.entryCompilationUnit;
+        if (compilationUnit.partTag != null) {
+          if (skipFileWithPartOfTag) {
+            // TODO(johnniwinther): Avoid calling
+            // [compiler.processLoadedLibraries] with this library.
+            libraryCanonicalUriMap.remove(resolvedUri);
+            return null;
           }
-          return processLibraryTags(handler, element).then((_) {
-            reporter.withCurrentElement(element, () {
-              handler.registerLibraryExports(element);
-            });
-            return element;
-          });
+          if (importingLibrary == null) {
+            DiagnosticMessage error = reporter.withCurrentElement(
+                compilationUnit,
+                () => reporter.createMessage(
+                    compilationUnit.partTag, MessageKind.MAIN_HAS_PART_OF));
+            reporter.reportError(error);
+          } else {
+            DiagnosticMessage error = reporter.withCurrentElement(
+                compilationUnit,
+                () => reporter.createMessage(
+                    compilationUnit.partTag, MessageKind.IMPORT_PART_OF));
+            DiagnosticMessage info = reporter.withCurrentElement(
+                importingLibrary,
+                () => reporter.createMessage(
+                    spannable, MessageKind.IMPORT_PART_OF_HERE));
+            reporter.reportError(error, [info]);
+          }
+        }
+        await processLibraryTags(handler, element);
+        reporter.withCurrentElement(element, () {
+          handler.registerLibraryExports(element);
         });
+
+        if (element.isPlatformLibrary &&

[Siggi Cherem (dart-lang), 2017/03/24 17:40:28]

so nice to see patching done while loading, yay!

Minor suggestion: consider adding a tiny method with this logic to make it
self-documenting. 

  await patchLibraryIfNecessary(handler, element);

[Emily Fortuna, 2017/03/24 18:30:20]

done! I aaaaaalmost did that in my cleanup process, but then I was like "nah".
Done now!

+            // Don't patch library currently disallowed.
+            !element.isSynthesized &&
+            !element.isPatched &&
+            // Don't patch deserialized libraries.
+            !deserializer.isDeserialized(element)) {
+          // Apply patch, if any.
+          Uri patchUri = _patchResolverFunc(element.canonicalUri.path);
+          if (patchUri != null) {
+            await _patchParserTask.patchLibrary(handler, patchUri, element);
+          }
+        }
+        return element;
       });
     });
   }
 
   LibraryElement createLibrarySync(
       LibraryDependencyHandler handler, Script script, Uri resolvedUri) {
     LibraryElement element = new LibraryElementX(script, resolvedUri);
     return reporter.withCurrentElement(element, () {
       if (handler != null) {
         handler.registerNewLibrary(element);
@@ skipping 566 matching lines @@
       assert(invariant(library, importingNode != null,
           message: "$library has not been registered"));
       importingNode.registerImportDependency(libraryDependency, loadedLibrary);
     }
   }
 
   /**
    * Registers [library] for the processing of its import/export scope.
    */
   void registerNewLibrary(LibraryElement library) {
-    task.listener.onLibraryCreated(library);
     _newLibraries.add(library);
     if (!library.exportsHandled) {
       nodeMap[library] = new LibraryDependencyNode(library);
     }
   }
 
   /**
    * Registers all top-level entities of [library] as starting point for the
    * fixed-point computation of the import/export scopes.
    */
   void registerLibraryExports(LibraryElement library) {
     nodeMap[library].registerInitialExports();
   }
 
   Future processLibraryTags(LibraryElement library) {
     return task.processLibraryTags(this, library);
   }
 }
 
-/// Information on the bulk of newly loaded libraries through a call to
+/// Information on the set libraries loaded as a result of a call to
 /// [LibraryLoader.loadLibrary].
 abstract class LoadedLibraries {
-  /// The uri passed to [LibraryLoader.loadLibrary].
-  Uri get rootUri;
+  /// The accesss the library object created corresponding to the library
+  /// passed to [LibraryLoader.loadLibrary].
+  LibraryElement get rootLibrary;
 
   /// Returns `true` if a library with canonical [uri] was loaded in this bulk.
   bool containsLibrary(Uri uri);
 
   /// Returns the library with canonical [uri] that was loaded in this bulk.
   LibraryElement getLibrary(Uri uri);
 
   /// Applies all libraries in this bulk to [f].
   void forEachLibrary(f(LibraryElement library));
 
   /// Applies all imports chains of [uri] in this bulk to [callback].
   ///
   /// The argument [importChainReversed] to [callback] contains the chain of
   /// imports uris that lead to importing [uri] starting in [uri] and ending in
-  /// [rootUri].
+  /// the uri that was passed in with [loadLibrary].
   ///
   /// [callback] is called once for each chain of imports leading to [uri] until
   /// [callback] returns `false`.
   void forEachImportChain(Uri uri,
       {bool callback(Link<Uri> importChainReversed)});
 }
 
 class _LoadedLibraries implements LoadedLibraries {
   final _LibraryLoaderTask task;
   final LibraryElement rootLibrary;
   final Map<Uri, LibraryElement> loadedLibraries = <Uri, LibraryElement>{};
-  final Map<LibraryElement, LibraryDependencyNode> nodeMap;
+  final List<LibraryElement> _newLibraries;
 
-  _LoadedLibraries(this.rootLibrary, Iterable<LibraryElement> libraries,
-      this.nodeMap, this.task) {
-    libraries.forEach((LibraryElement loadedLibrary) {
+  _LoadedLibraries(this.rootLibrary, this._newLibraries, this.task) {
+    _newLibraries.forEach((LibraryElement loadedLibrary) {
       loadedLibraries[loadedLibrary.canonicalUri] = loadedLibrary;
     });
   }
 
-  Uri get rootUri => rootLibrary.canonicalUri;
-
   bool containsLibrary(Uri uri) => loadedLibraries.containsKey(uri);
 
   LibraryElement getLibrary(Uri uri) => loadedLibraries[uri];
 
-  void forEachLibrary(f(LibraryElement library)) => nodeMap.keys.forEach(f);
+  void forEachLibrary(f(LibraryElement library)) => _newLibraries.forEach(f);
 
   void forEachImportChain(Uri targetUri,
       {bool callback(Link<Uri> importChainReversed)}) {
     bool aborted = false;
 
     /// Map from libraries to the set of (unreversed) paths to [uri].
     Map<LibraryElement, Iterable<Link<Uri>>> suffixChainMap =
         <LibraryElement, Iterable<Link<Uri>>>{};
 
     /// Computes the set of (unreversed) paths to [targetUri].
@@ skipping 58 matching lines @@
         }
         suffixes.add(const Link<Uri>().prepend(canonicalUri));
       }
       suffixChainMap[library] = suffixes;
       return;
     }
 
     computeSuffixes(rootLibrary, const Link<Uri>());
   }
 
-  String toString() => 'root=$rootLibrary,libraries=${loadedLibraries.keys}';
+  String toString() => 'root=$rootLibrary,libraries=${_newLibraries}';
 }
 
 // TODO(sigmund): remove ScriptLoader & ElementScanner. Such abstraction seems
 // rather low-level. It might be more practical to split the library-loading
 // task itself.  The task would continue to do the work of recursively loading
 // dependencies, but it can delegate to a set of subloaders how to do the actual
 // loading. We would then have a list of subloaders that use different
 // implementations: in-memory cache, deserialization, scanning from files.
 //
 // For example, the API might look like this:
@@ skipping 32 matching lines @@
   /// Load script from a readable [uri], report any errors using the location of
   /// the given [spannable].
   Future<Script> readScript(Uri uri, [Spannable spannable]);
 }
 
 /// API used by the library loader to synchronously scan a library or
 /// compilation unit and ensure that their library tags are computed.
 abstract class ElementScanner {
   void scanLibrary(LibraryElement library);
   void scanUnit(CompilationUnitElement unit);
-}
-
-/// TODO(sigmund): remove this abstraction. Ideally the loader can produce the
-/// LoadedLibraries results once, and the compiler and choose what to do with
-/// it instead.
-abstract class LibraryLoaderListener {
-  /// Called after a request to load a library. The [results] will include all
-  /// transitive libraries loaded as a result of the initial request.
-  Future onLibrariesLoaded(LoadedLibraries results);
-
-  /// Called whenever a library element is created.
-  void onLibraryCreated(LibraryElement library);
-
-  /// Called whenever a library is scanned from a script file.
-  Future onLibraryScanned(LibraryElement library, LibraryLoader loader);
+  Token scanFile(SourceFile file);

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

looks like you don't need scanFile after all? could we remove it here and in
compiler.dart?

[Emily Fortuna, 2017/03/24 18:30:20]

yes thanks.

 }
 
 const _reuseLibrarySubtaskName = "Reuse library";