fix(dart/transform): Don't set ReflectionCapabilities over an async gap

Update the transformer's `TemplateCompiler` phase to avoid setting
`reflector.reflectionCapabilities`, allowing asynchronous
operations, and restoring the original value, which allows
`reflector.reflectionCapabilities` to get into a bad state.

modules/angular2/src/transform/template_compiler/generator.dart

@@ -6,8 +6,9 @@ import 'package:angular2/src/change_detection/parser/lexer.dart' as ng;
 import 'package:angular2/src/change_detection/parser/parser.dart' as ng;
 import 'package:angular2/src/core/compiler/proto_view_factory.dart';
 import 'package:angular2/src/render/api.dart';
-import 'package:angular2/src/render/dom/compiler/compiler.dart';
+import 'package:angular2/src/render/dom/compiler/compile_pipeline.dart';
 import 'package:angular2/src/render/dom/compiler/template_loader.dart';
+import 'package:angular2/src/render/dom/view/property_setter_factory.dart';
 import 'package:angular2/src/services/xhr.dart' show XHR;
 import 'package:angular2/src/reflection/reflection.dart';
 import 'package:angular2/src/services/url_resolver.dart';
@@ -17,7 +18,7 @@ import 'package:barback/barback.dart';
 
 import 'change_detector_codegen.dart' as change;
 import 'compile_step_factory.dart';
-import 'recording_reflection_capabilities.dart';
+import 'reflection_capabilities.dart';
 import 'reflector_register_codegen.dart' as reg;
 import 'view_definition_creator.dart';
 
@@ -39,34 +40,21 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
     var result = await extractor.extractTemplates(viewDefEntry.viewDef);
     if (result == null) continue;
 
+    if (generateRegistrations) {
+      // TODO(kegluneq): Generate these getters & setters based on the
+      // `ProtoViewDto` rather than querying the `ReflectionCapabilities`.
       registrations.generate(result.recording);
+    }
     if (result.protoView != null && generateChangeDetectors) {
-      var savedReflectionCapabilities = reflector.reflectionCapabilities;
+      var saved = reflector.reflectionCapabilities;
-      var recordingCapabilities = new RecordingReflectionCapabilities();
+      reflector.reflectionCapabilities = const NullReflectionCapabilities();
-      reflector.reflectionCapabilities = recordingCapabilities;
-
       var defs = getChangeDetectorDefinitions(viewDefEntry.hostMetadata,
           result.protoView, viewDefEntry.viewDef.directives);
       for (var i = 0; i < defs.length; ++i) {
         changeDetectorClasses.generate('${rType.typeName}',
             '_${rType.typeName}_ChangeDetector$i', defs[i]);
       }
-
+      reflector.reflectionCapabilities = saved;
-      // Check that getters, setters, methods are the same as above.
-      assert(recordingCapabilities.getterNames
-          .containsAll(result.recording.getterNames));
-      assert(result.recording.getterNames
-          .containsAll(recordingCapabilities.getterNames));
-      assert(recordingCapabilities.setterNames
-          .containsAll(result.recording.setterNames));
-      assert(result.recording.setterNames
-          .containsAll(recordingCapabilities.setterNames));
-      assert(recordingCapabilities.methodNames
-          .containsAll(result.recording.methodNames));
-      assert(result.recording.methodNames
-          .containsAll(recordingCapabilities.methodNames));
-
-      reflector.reflectionCapabilities = savedReflectionCapabilities;
     }
   }
 
@@ -88,25 +76,40 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
 /// template code if necessary, and determines what values will be
 /// reflectively accessed from that template.
 class _TemplateExtractor {
-  final RenderCompiler _compiler;
+  final CompileStepFactory _factory;
+  final TemplateLoader _loader;
 
-  _TemplateExtractor(XHR xhr) : _compiler = new DomCompiler(
+  _TemplateExtractor(XHR xhr)
-          new CompileStepFactory(new ng.Parser(new ng.Lexer())),
+      : _factory = new CompileStepFactory(new ng.Parser(new ng.Lexer())),
-          new TemplateLoader(xhr, new UrlResolver()));
+        _loader = new TemplateLoader(xhr, new UrlResolver());
 
   Future<_ExtractResult> extractTemplates(ViewDefinition viewDef) async {
     // Check for "imperative views".
     if (viewDef.template == null && viewDef.absUrl == null) return null;
 
+    var templateEl = await _loader.load(viewDef);
+
+    // NOTE(kegluneq): Since this is a global, we must not have any async
+    // operations between saving and restoring it, otherwise we can get into
+    // a bad state. See issue #2359 for additional context.
     var savedReflectionCapabilities = reflector.reflectionCapabilities;
     var recordingCapabilities = new RecordingReflectionCapabilities();
     reflector.reflectionCapabilities = recordingCapabilities;
 
-    // TODO(kegluneq): Rewrite url to inline `template` where possible.
+    var subtaskPromises = [];
-    var protoViewDto = await _compiler.compile(viewDef);
+    var pipeline =
+        new CompilePipeline(_factory.createSteps(viewDef, subtaskPromises));
+
+    var compileElements = pipeline.process(
+        templateEl, ProtoViewDto.COMPONENT_VIEW_TYPE, viewDef.componentId);
+    var protoViewDto = compileElements[0].inheritedProtoView
+        .build(new PropertySetterFactory());
 
     reflector.reflectionCapabilities = savedReflectionCapabilities;
-    return new _ExtractResult(recordingCapabilities, protoViewDto);
+
+    return Future
+        .wait(subtaskPromises)
+        .then((_) => new _ExtractResult(recordingCapabilities, protoViewDto));
   }
 }
 

modules/angular2/src/transform/template_compiler/reflection_capabilities.dart

@@ -1,18 +1,12 @@
-library angular2.transform.template_compiler.recording_reflection_capabilities;
+library angular2.transform.template_compiler.reflection_capabilities;
 
 import 'package:angular2/src/reflection/reflection_capabilities.dart';
 import 'package:angular2/src/reflection/types.dart';
 
-/// ReflectionCapabilities object that records requests for `getter`s,
+/// ReflectionCapabilities object that responds to all requests for `getter`s,
-/// `setter`s, and `method`s so these can be code generated rather than
+/// `setter`s, and `method`s with `null`.
-/// reflectively accessed at runtime.
+class NullReflectionCapabilities implements ReflectionCapabilities {
-class RecordingReflectionCapabilities implements ReflectionCapabilities {
+  const NullReflectionCapabilities();
-  /// The names of all requested `getter`s.
-  final Set<String> getterNames = new Set<String>();
-  /// The names of all requested `setter`s.
-  final Set<String> setterNames = new Set<String>();
-  /// The names of all requested `method`s.
-  final Set<String> methodNames = new Set<String>();
 
   _notImplemented(String name) => throw 'Not implemented: $name';
 
@@ -24,22 +18,40 @@ class RecordingReflectionCapabilities implements ReflectionCapabilities {
 
   List annotations(typeOrFunc) => _notImplemented('annotations');
 
-  GetterFn getter(String name) {
+  GetterFn getter(String name) => _nullGetter;
-    getterNames.add(name);
-    return _nullGetter;
-  }
 
-  SetterFn setter(String name) {
+  SetterFn setter(String name) => _nullSetter;
-    setterNames.add(name);
-    return _nullSetter;
-  }
 
-  MethodFn method(String name) {
+  MethodFn method(String name) => _nullMethod;
-    methodNames.add(name);
-    return _nullMethod;
-  }
 }
 
 _nullGetter(Object p) => null;
 _nullSetter(Object p, v) => null;
 _nullMethod(Object p, List a) => null;
+
+/// ReflectionCapabilities object that records requests for `getter`s,
+/// `setter`s, and `method`s so these can be code generated rather than
+/// reflectively accessed at runtime.
+class RecordingReflectionCapabilities extends NullReflectionCapabilities {
+  /// The names of all requested `getter`s.
+  final Set<String> getterNames = new Set<String>();
+  /// The names of all requested `setter`s.
+  final Set<String> setterNames = new Set<String>();
+  /// The names of all requested `method`s.
+  final Set<String> methodNames = new Set<String>();
+
+  GetterFn getter(String name) {
+    getterNames.add(name);
+    return super.getter(name);
+  }
+
+  SetterFn setter(String name) {
+    setterNames.add(name);
+    return super.setter(name);
+  }
+
+  MethodFn method(String name) {
+    methodNames.add(name);
+    return super.method(name);
+  }
+}

modules/angular2/src/transform/template_compiler/reflector_register_codegen.dart

@@ -2,7 +2,7 @@ library angular2.transform.template_compiler.reflector_register_codegen;
 
 import 'package:angular2/src/transform/common/names.dart';
 import 'package:angular2/src/transform/common/property_utils.dart' as prop;
-import 'recording_reflection_capabilities.dart';
+import 'reflection_capabilities.dart';
 
 class Codegen {
   final StringBuffer _buf = new StringBuffer();

modules/angular2/test/transform/template_compiler/all_tests.dart

@@ -103,7 +103,6 @@ void allTests() {
       output = await process(new AssetId('a', inputPath));
       _formatThenExpectEquals(output, expected);
     });
-
   });
 }