Adding an inheriting data model to Node

tools/dom/templates/html/impl/impl_Node.darttemplate

 // 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.
 
 part of $LIBRARYNAME;
 
 /**
  * Lazy implementation of the child nodes of an element that does not request
  * the actual child nodes of an element until strictly necessary greatly
  * improving performance for the typical cases where it is not required.
@@ skipping 312 matching lines @@
         // Should use $dom_firstChild, Bug 8886.
         this.insertBefore(newNodes[0], refChild);
       }
     } else {
       for (var node in newNodes) {
         this.insertBefore(node, refChild);
       }
     }
   }
 
+  // Note that this may either be the locally set model or a cached value
+  // of the inherited model. This is cached to minimize model change
+  // notifications.
+  var _model;
+  bool _hasLocalModel;
+  StreamController _modelChangedStream;
+
+  /**
+   * The data model which is inherited through the tree.
+   *
+   * Setting this will propagate the value to all descendant nodes. If the
+   * model is not set on this node then it will be inherited from ancestor
+   * nodes.
+   *
+   * [clearModel] must be used to remove the model property from this node
+   * and have the model inherit from ancestor nodes.
+   */
+  @Experimental
+  get model {
+    // If we have a change handler then we've cached the model locally.
+    if (_modelChangedStream != null) {
+      return _model;
+    }
+    // Otherwise start looking up the tree.
+    for (var node = this; node != null; node = node.parentNode) {
+      if (node._hasLocalModel == true) {
+        return node._model;
+      }
+    }
+    return null;
+  }
+
+  @Experimental
+  void set model(value) {
+    _model = value;

[Jennifer Messerly, 2013/03/15 03:20:46]

bail if old model == value?

[blois, 2013/03/15 18:26:05]

Added check to eliminate invalid change notification & unnecessary tree
traversal.

+    _hasLocalModel = true;
+    _ModelTreeObserver.initialize();
+
+    if (_modelChangedStream != null) {
+      _modelChangedStream.add(value);

[Jennifer Messerly, 2013/03/15 03:20:46]

Does this deliver synchronously?

If so, it seems like we might want to wait until after we've propagated? Mainly
to ensure that user code doesn't see inconsistent tree/model state. (Also they
could muck with the tree, not sure if that could have adverse effects)

[blois, 2013/03/15 18:26:05]

Yeah, was going to mention that- still need to work on the timing a bit. Right
now there's a difference between the MutationObservers and MutationEvents code
which could cause problems, then there's when these events get fired.

I was going to do it as a separate CL, but can do it in this one- I think most
of the work is in the testing.

+    }
+    // Propagate new model to all descendants.
+    _ModelTreeObserver.propagateModel(this, value, false);
+  }
+
+  /**
+   * Clears the locally set model and makes this model be inherited from parent
+   * nodes.
+   */

[Jennifer Messerly, 2013/03/15 03:20:46]

mark this @Experimental too, along with onModelChanged?

[blois, 2013/03/15 18:26:05]

Done.

+  void clearModel() {

[Jennifer Messerly, 2013/03/15 03:20:46]

out of curiosity, why not use `null` for this? Does it make sense to distinguish
between "not set" and "null"?

[blois, 2013/03/15 18:26:05]

If a detail view is set display the selected item and there is no selection then
you don't want it inheriting the model from further up the tree- this could
cause bindings to pick up values which are incorrect. So null can be set and
will inherit down.

FWIW, MDV addresses this by differentiating between null and undefined, where
null is set and undefined is inherit.

+    if (_hasLocalModel == true) {
+      _hasLocalModel = false;
+
+      // Propagate new model to all descendants.
+      if (parentNode != null) {
+        _ModelTreeObserver.propagateModel(this, parentNode.model, false);
+      } else {
+        _ModelTreeObserver.propagateModel(this, null, false);
+      }
+    }
+  }
+
+  /**
+   * Get a stream of models, whenever the model changes.
+   */
+  Stream get onModelChanged {

[Jennifer Messerly, 2013/03/15 03:20:46]

One thing I was wondering about for this event. With templates, we'd potentially
be listening on a lot of fine-grained modelChanged events. Do we want to do
that? Would a global event handler be better? That would seem to imply some sort
of bubbling, which is bad. Maybe the callback API should look more like mutation
observers -- a batch of .model changes are delivered on the next microtask to
the template system. On the other hand, the current API is nice and simple.

WDYT?

[blois, 2013/03/15 18:26:05]

One thing that I was thinking about is that we should be able to transparently
redirect these change notifications to the node which the model is set on- so
when the model is changed we just loop through the set of change watchers. 

This would make it efficient to repeatedly modify the model on a specific
element, which I believe is the primary scenario and would remove the tree
traversal for model changes.

My concern is getting the timings right with the tree change events- it may
require better tracking of whether items are in the tree or not. Again, more
tests needed.

I also want to make sure that this works well with custom elements, where users
are going to care directly about the model changing and will be poking new
values in at random times.

I'd like to see how far we can get with this fine-grained level of notifications
and see where the performance issues are.

+    if (_modelChangedStream == null) {
+      // Ensure the model is cached locally to minimize change notifications.
+      _model = model;
+      _modelChangedStream = new StreamController.broadcast();
+    }
+    return _modelChangedStream.stream;
+  }
+
 $!MEMBERS
 }