refactor(change_detect): Move (de)hydrate methods into superclass

Move the implementation of `(de)hydrate`, `hydrated`, and
`detectChangesInRecords` into `AbstractChangeDetector`.

Add comments clarifying the contract between `AbstractChangeDetector`
and its subclasses.

Closes #3245

modules/angular2/src/change_detection/abstract_change_detector.ts

@@ -1,5 +1,6 @@
 import {isPresent, BaseException} from 'angular2/src/facade/lang';
 import {List, ListWrapper} from 'angular2/src/facade/collection';
+import {ChangeDetectionUtil} from './change_detection_util';
 import {ChangeDetectorRef} from './change_detector_ref';
 import {DirectiveRecord} from './directive_record';
 import {ChangeDetector, ChangeDispatcher} from './interfaces';
@@ -34,7 +35,7 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
   protos: List<ProtoRecord>;
 
   constructor(public id: string, dispatcher: ChangeDispatcher, protos: List<ProtoRecord>,
-              directiveRecords: List<DirectiveRecord>) {
+              directiveRecords: List<DirectiveRecord>, public modeOnHydrate: string) {
     this.ref = new ChangeDetectorRef(this);
     this.directiveRecords = directiveRecords;
     this.dispatcher = dispatcher;
@@ -75,16 +76,59 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
     if (this.mode === CHECK_ONCE) this.mode = CHECKED;
   }
 
-  detectChangesInRecords(throwOnChange: boolean): void {}
+  // This method is not intended to be overridden. Subclasses should instead provide an
+  // implementation of `detectChangesInRecordsInternal` which does the work of detecting changes
+  // and which this method will call.
+  // This method expects that `detectChangesInRecordsInternal` will set the property
+  // `this.currentProto` to whichever [ProtoRecord] is currently being processed. This is to
+  // facilitate error reporting.
+  detectChangesInRecords(throwOnChange: boolean): void {
+    if (!this.hydrated()) {
+      ChangeDetectionUtil.throwDehydrated();
+    }
+    try {
+      this.detectChangesInRecordsInternal(throwOnChange);
+    } catch (e) {
+      this.throwError(this.currentProto, e, e.stack);
+    }
+  }
 
-  hydrate(context: T, locals: Locals, directives: any, pipes: any): void {}
+  // Subclasses should override this method to perform any work necessary to detect and report
+  // changes. For example, changes should be reported via `ChangeDetectionUtil.addChange`, lifecycle
+  // methods should be called, etc.
+  // This implementation should also set `this.currentProto` to whichever [ProtoRecord] is
+  // currently being processed to facilitate error reporting. See {@link #detectChangesInRecords}.
+  detectChangesInRecordsInternal(throwOnChange: boolean): void {}
 
+  // This method is not intended to be overridden. Subclasses should instead provide an
+  // implementation of `hydrateDirectives`.
+  hydrate(context: T, locals: Locals, directives: any, pipes: any): void {
+    this.mode = this.modeOnHydrate;
+    this.context = context;
+    this.locals = locals;
+    this.pipes = pipes;
+    this.hydrateDirectives(directives);
+    this.alreadyChecked = false;
+  }
+
+  // Subclasses should override this method to hydrate any directives.
   hydrateDirectives(directives: any): void {}
 
-  dehydrate(): void {}
+  // This method is not intended to be overridden. Subclasses should instead provide an
+  // implementation of `dehydrateDirectives`.
+  dehydrate(): void {
+    this.dehydrateDirectives(true);
+    this.context = null;
+    this.locals = null;
+    this.pipes = null;
+  }
 
+  // Subclasses should override this method to dehydrate any directives. This method should reverse
+  // any work done in `hydrateDirectives`.
   dehydrateDirectives(destroyPipes: boolean): void {}
 
+  hydrated(): boolean { return this.context !== null; }
+
   callOnAllChangesDone(): void {}
 
   _detectChangesInLightDomChildren(throwOnChange: boolean): void {

modules/angular2/src/change_detection/change_detection_jit_generator.ts

@@ -6,7 +6,7 @@ import {ChangeDetectionUtil} from './change_detection_util';
 import {DirectiveIndex, DirectiveRecord} from './directive_record';
 
 import {ProtoRecord, RecordType} from './proto_record';
-import {CONTEXT_INDEX, CodegenNameUtil, sanitizeName} from './codegen_name_util';
+import {CodegenNameUtil, sanitizeName} from './codegen_name_util';
 
 
 /**
@@ -27,7 +27,7 @@ export class ChangeDetectorJITGenerator {
   _names: CodegenNameUtil;
   _typeName: string;
 
-  constructor(public id: string, public changeDetectionStrategy: string,
+  constructor(public id: string, private changeDetectionStrategy: string,
               public records: List<ProtoRecord>, public directiveRecords: List<any>,
               private generateCheckNoChanges: boolean) {
     this._names = new CodegenNameUtil(this.records, this.directiveRecords, UTIL);
@@ -37,24 +37,15 @@ export class ChangeDetectorJITGenerator {
   generate(): Function {
     var classDefinition = `
       var ${this._typeName} = function ${this._typeName}(dispatcher, protos, directiveRecords) {
-        ${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)}, dispatcher, protos, directiveRecords);
+        ${ABSTRACT_CHANGE_DETECTOR}.call(
+            this, ${JSON.stringify(this.id)}, dispatcher, protos, directiveRecords,
+            "${ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy)}");
         this.dehydrateDirectives(false);
       }
 
       ${this._typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype);
 
-      ${this._typeName}.prototype.detectChangesInRecords = function(throwOnChange) {
-        if (!this.hydrated()) {
-          ${UTIL}.throwDehydrated();
-        }
-        try {
-          this.__detectChangesInRecords(throwOnChange);
-        } catch (e) {
-          this.throwError(${this._names.getCurrentProtoName()}, e, e.stack);
-        }
-      }
-
-      ${this._typeName}.prototype.__detectChangesInRecords = function(throwOnChange) {
+      ${this._typeName}.prototype.detectChangesInRecordsInternal = function(throwOnChange) {
         ${this._names.getCurrentProtoName()} = null;
 
         ${this._names.genInitLocals()}
@@ -72,30 +63,10 @@ export class ChangeDetectorJITGenerator {
         ${this._genCallOnAllChangesDoneBody()}
       }
 
-      ${this._typeName}.prototype.hydrate = function(context, locals, directives, pipes) {
-        ${this._names.getModeName()} =
-            "${ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy)}";
-        ${this._names.getFieldName(CONTEXT_INDEX)} = context;
-        ${this._names.getLocalsAccessorName()} = locals;
-        this.hydrateDirectives(directives);
-        ${this._names.getPipesAccessorName()} = pipes;
-        ${this._names.getAlreadyCheckedName()} = false;
-      }
-
       ${this._maybeGenHydrateDirectives()}
 
-      ${this._typeName}.prototype.dehydrate = function() {
-        this.dehydrateDirectives(true);
-        ${this._names.getLocalsAccessorName()} = null;
-        ${this._names.getPipesAccessorName()} = null;
-      }
-
       ${this._maybeGenDehydrateDirectives()}
 
-      ${this._typeName}.prototype.hydrated = function() {
-        return ${this._names.getFieldName(CONTEXT_INDEX)} !== null;
-      }
-
       return function(dispatcher) {
         return new ${this._typeName}(dispatcher, protos, directiveRecords);
       }
@@ -153,6 +124,7 @@ export class ChangeDetectorJITGenerator {
     var notifications = [];
     var dirs = this.directiveRecords;
 
+    // NOTE(kegluneq): Order is important!
     for (var i = dirs.length - 1; i >= 0; --i) {
       var dir = dirs[i];
       if (dir.callOnAllChangesDone) {

modules/angular2/src/change_detection/codegen_name_util.ts

@@ -124,11 +124,11 @@ export class CodegenNameUtil {
   genDehydrateFields(): string {
     var fields = this.getAllFieldNames();
     ListWrapper.removeAt(fields, CONTEXT_INDEX);
-    if (!ListWrapper.isEmpty(fields)) {
-      // At least one assignment.
-      fields.push(`${this.utilName}.uninitialized;`);
-    }
-    return `${this.getFieldName(CONTEXT_INDEX)} = null; ${ListWrapper.join(fields, ' = ')}`;
+    if (ListWrapper.isEmpty(fields)) return '';
+
+    // At least one assignment.
+    fields.push(`${this.utilName}.uninitialized;`);
+    return ListWrapper.join(fields, ' = ');
   }
 
   /**

modules/angular2/src/change_detection/dynamic_change_detector.ts

@@ -1,10 +1,8 @@
 import {isPresent, isBlank, BaseException, FunctionWrapper} from 'angular2/src/facade/lang';
 import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
-import {Locals} from 'angular2/src/change_detection/parser/locals';
 
 import {AbstractChangeDetector} from './abstract_change_detector';
 import {BindingRecord} from './binding_record';
-import {Pipes} from './pipes/pipes';
 import {ChangeDetectionUtil, SimpleChange} from './change_detection_util';
 
 
@@ -17,39 +15,34 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
   prevContexts: List<any>;
   directives: any = null;
 
-  constructor(id: string, private changeControlStrategy: string, dispatcher: any,
+  constructor(id: string, changeDetectionStrategy: string, dispatcher: any,
               protos: List<ProtoRecord>, directiveRecords: List<any>) {
-    super(id, dispatcher, protos, directiveRecords);
-    this.values = ListWrapper.createFixedSize(protos.length + 1);
-    this.localPipes = ListWrapper.createFixedSize(protos.length + 1);
-    this.prevContexts = ListWrapper.createFixedSize(protos.length + 1);
-    this.changes = ListWrapper.createFixedSize(protos.length + 1);
+    super(id, dispatcher, protos, directiveRecords,
+          ChangeDetectionUtil.changeDetectionMode(changeDetectionStrategy));
+    var len = protos.length + 1;
+    this.values = ListWrapper.createFixedSize(len);
+    this.localPipes = ListWrapper.createFixedSize(len);
+    this.prevContexts = ListWrapper.createFixedSize(len);
+    this.changes = ListWrapper.createFixedSize(len);
 
-    this.values[0] = null;
-    ListWrapper.fill(this.values, ChangeDetectionUtil.uninitialized, 1);
-    ListWrapper.fill(this.localPipes, null);
-    ListWrapper.fill(this.prevContexts, ChangeDetectionUtil.uninitialized);
-    ListWrapper.fill(this.changes, false);
+    this.dehydrateDirectives(false);
   }
 
-  hydrate(context: any, locals: Locals, directives: any, pipes: Pipes): void {
-    this.mode = ChangeDetectionUtil.changeDetectionMode(this.changeControlStrategy);
-    this.values[0] = context;
-    this.locals = locals;
+  hydrateDirectives(directives: any): void {
+    this.values[0] = this.context;
     this.directives = directives;
-    this.alreadyChecked = false;
-    this.pipes = pipes;
   }
 
-  dehydrate() {
-    this._destroyPipes();
+  dehydrateDirectives(destroyPipes: boolean) {
+    if (destroyPipes) {
+      this._destroyPipes();
+    }
     this.values[0] = null;
+    this.directives = null;
     ListWrapper.fill(this.values, ChangeDetectionUtil.uninitialized, 1);
     ListWrapper.fill(this.changes, false);
     ListWrapper.fill(this.localPipes, null);
     ListWrapper.fill(this.prevContexts, ChangeDetectionUtil.uninitialized);
-    this.locals = null;
-    this.pipes = null;
   }
 
   _destroyPipes() {
@@ -60,10 +53,9 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
     }
   }
 
-  hydrated(): boolean { return this.values[0] !== null; }
-
   checkNoChanges(): void { this.runDetectChanges(true); }
 
+  // TODO(vsavkin): #3366. Update to work with [AbstractChangeDetector#detectChangesInRecords]
   detectChangesInRecords(throwOnChange: boolean) {
     if (!this.hydrated()) {
       ChangeDetectionUtil.throwDehydrated();

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

@@ -104,22 +104,11 @@ class _CodegenState {
 
         $_changeDetectorTypeName(dispatcher, protos, directiveRecords)
           : super(${_encodeValue(_changeDetectorDefId)},
-              dispatcher, protos, directiveRecords) {
+              dispatcher, protos, directiveRecords, '$_changeDetectionMode') {
           dehydrateDirectives(false);
         }
 
-        void detectChangesInRecords(throwOnChange) {
-          if (!hydrated()) {
-            $_UTIL.throwDehydrated();
-          }
-          try {
-            __detectChangesInRecords(throwOnChange);
-          } catch (e, s) {
-            throwError(${_names.getCurrentProtoName()}, e, s);
-          }
-        }
-
-        void __detectChangesInRecords(throwOnChange) {
+        void detectChangesInRecordsInternal(throwOnChange) {
           ${_names.getCurrentProtoName()} = null;
 
           ${_names.genInitLocals()}
@@ -137,28 +126,10 @@ class _CodegenState {
           ${_getCallOnAllChangesDoneBody()}
         }
 
-        void hydrate(
-            $_contextTypeName context, locals, directives, pipes) {
-          ${_names.getModeName()} = '$_changeDetectionMode';
-          ${_names.getFieldName(CONTEXT_INDEX)} = context;
-          ${_names.getLocalsAccessorName()} = locals;
-          hydrateDirectives(directives);
-          ${_names.getAlreadyCheckedName()} = false;
-          ${_names.getPipesAccessorName()} = pipes;
-        }
-
         ${_maybeGenHydrateDirectives()}
 
-        void dehydrate() {
-          dehydrateDirectives(true);
-          ${_names.getLocalsAccessorName()} = null;
-          ${_names.getPipesAccessorName()} = null;
-        }
-
         ${_maybeGenDehydrateDirectives()}
 
-        hydrated() => ${_names.getFieldName(CONTEXT_INDEX)} != null;
-
         static $_GEN_PREFIX.ProtoChangeDetector
             $PROTO_CHANGE_DETECTOR_FACTORY_METHOD(
             $_GEN_PREFIX.ChangeDetectorDefinition def) {

modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart

@@ -27,23 +27,13 @@ class _MyComponent_ChangeDetector0
     extends _gen.AbstractChangeDetector<MyComponent> {
   var myNum0, interpolate1;
 
-  _MyComponent_ChangeDetector0(dispatcher, protos, directiveRecords)
-      : super("MyComponent_comp_0", dispatcher, protos, directiveRecords) {
+  _MyComponent_ChangeDetector0(dispatcher, protos, directiveRecords) : super(
+          "MyComponent_comp_0", dispatcher, protos, directiveRecords,
+          'ALWAYS_CHECK') {
     dehydrateDirectives(false);
   }
 
-  void detectChangesInRecords(throwOnChange) {
-    if (!hydrated()) {
-      _gen.ChangeDetectionUtil.throwDehydrated();
-    }
-    try {
-      __detectChangesInRecords(throwOnChange);
-    } catch (e, s) {
-      throwError(this.currentProto, e, s);
-    }
-  }
-
-  void __detectChangesInRecords(throwOnChange) {
+  void detectChangesInRecordsInternal(throwOnChange) {
     this.currentProto = null;
     var l_context = this.context,
         l_myNum0,
@@ -96,28 +86,10 @@ class _MyComponent_ChangeDetector0
     this.dispatcher.notifyOnAllChangesDone();
   }
 
-  void hydrate(MyComponent context, locals, directives, pipes) {
-    this.mode = 'ALWAYS_CHECK';
-    this.context = context;
-    this.locals = locals;
-    hydrateDirectives(directives);
-    this.alreadyChecked = false;
-    this.pipes = pipes;
-  }
-
-  void dehydrate() {
-    dehydrateDirectives(true);
-    this.locals = null;
-    this.pipes = null;
-  }
-
   void dehydrateDirectives(destroyPipes) {
-    this.context = null;
     this.myNum0 = this.interpolate1 = _gen.ChangeDetectionUtil.uninitialized;
   }
 
-  hydrated() => this.context != null;
-
   static _gen.ProtoChangeDetector newProtoChangeDetector(
       _gen.ChangeDetectorDefinition def) {
     return new _gen.PregenProtoChangeDetector(