Ensure getInstanceModel is only called when a new instance will be created

Ensure getInstanceModel is only called when a new instance will be created
Note that this also allows for the common case of inserting an instance via a DocumentFragment. Only if there is a instanceCache hit, do we insert an array of nodes.
port of https://github.com/Polymer/mdv/commit/694a4114779a9cd8ea3c915d1f5e9ec5ad6bd68c

R=justinfagnani@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25373

[justinfagnani, 2013-07-23 20:42:06]

lgtm

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
File pkg/mdv/lib/src/template_iterator.dart (right):

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
pkg/mdv/lib/src/template_iterator.dart:253: if (terminator == null) terminator =
previousTerminator;
Initialize terminator to null? I'm not sure if fragment.lastChild or
instanceNodes.last can be null, but talking to Steven, it sounded like there
were potential code size benefits to not allowing variables to be null. He
recommended making all possible instance vars final, not sure if that applies to
locals.

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
pkg/mdv/lib/src/template_iterator.dart:336: if (instanceNodes == null) {
check for empty too?

[Jennifer Messerly, 2013-07-23 20:49:39]

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
File pkg/mdv/lib/src/template_iterator.dart (right):

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
pkg/mdv/lib/src/template_iterator.dart:253: if (terminator == null) terminator =
previousTerminator;
On 2013/07/23 20:42:06, justinfagnani wrote:
> Initialize terminator to null? I'm not sure if fragment.lastChild or
> instanceNodes.last can be null, but talking to Steven, it sounded like there
> were potential code size benefits to not allowing variables to be null. He
> recommended making all possible instance vars final, not sure if that applies
to
> locals.

:confused: it is initialized to null ... yes, fragment.lastChild can be null,
likewise instanceNodes can be empty.

This particular case should be no problem for any competent compiler :). After
this line, "terminator" will have the same non-null properties as
previousTerminator.

https://chromiumcodereview.appspot.com/19689009/diff/1/pkg/mdv/lib/src/templa...
pkg/mdv/lib/src/template_iterator.dart:336: if (instanceNodes == null) {
On 2013/07/23 20:42:06, justinfagnani wrote:
> check for empty too?

if we expanded to an empty set of nodes before (hence cached it) we want to
reuse the empty nodes.

[Jennifer Messerly, 2013-07-23 22:06:09]

Committed patchset #2 manually as r25373 (presubmit successful).