Reorganize mdv and observe packages

Reorganize mdv and observe functionality
* keep correct dart:html APIs 
* move implementation into package:mdv and package:observe
* remove dart:mdv_observe_impl
* remove getValueWorkaround/setValueWorkaround from package:observe

R=blois@google.com, floitsch@google.com, justinfagnani@google.com

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

[Jennifer Messerly, 2013-06-25 02:54:26]

Hi guys,

This change reorganizes package:mdv and package:observe in a way that should
give us more freedom to version the MDV implementation and Observable impl/APIs.

The only Model-Driven Views code that is kept in dart:html:
* the <template> element content polyfill
* Node.bind, unbind, unbindAll APIs
* Element.model, createInstance APIs
* CustomBindingSyntax APIs (this can move soon though)

MDV is initialized by importing the package and calling `mdv.initialize();`

All of the code just moved, except for one functional change: removal of
getValueWorkaround/setValueWorkaround from Observable. See path_observer.dart.

Feel free to review any part of this but I was hoping for:

Florian -- have a look at libraries.dart and create_sdk.py
Justin -- have a look at package:mdv and package:observe
Pete -- have a look at dart:html

Note: this change does *not* introduce ObservableStream. As soon as I get this
one in I'll work on a followup to bring us up to date with MDV, which will
remove the global deliverChangeRecords, and removes TemplateElement.syntax as
well as a few other miscellaneous changes.

[justinfagnani, 2013-06-25 23:03:31]

lgtm

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/bindings.dart
File pkg/mdv/lib/src/bindings.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/bindings.d...
pkg/mdv/lib/src/bindings.dart:121: class _Bindings {
should this class become a library?

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/node.dart
File pkg/mdv/lib/src/node.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/node.dart#...
pkg/mdv/lib/src/node.dart:18: // TODO(jmesserly): should we throw instead?
I don't think so

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/template.dart
File pkg/mdv/lib/src/template.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/template.d...
pkg/mdv/lib/src/template.dart:69: * Setting this will destructive propagate the
value to all descendant nodes,
can we do an identity check to avoid churn in some cases?

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/observe.dart
File pkg/observe/lib/observe.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/observe.da...
pkg/observe/lib/observe.dart:253: * literals into the corresponding observable
collection type.
It may be obvious, but do you want to mention that the collection is only
observable when modified via the same observable wrapper instance returned from
this function?

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/src/observ...
File pkg/observe/lib/src/observable_map.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/src/observ...
pkg/observe/lib/src/observable_map.dart:71: other.forEach((key, value) {
result[key] = value; });
Map has addAll() now:
http://api.dartlang.org/docs/releases/latest/dart_core/Map.html#addAll

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/test/utils.dart
File pkg/observe/test/utils.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/test/utils.dar...
pkg/observe/test/utils.dart:10: // compare to toStrings.
there are list matchers:
http://api.dartlang.org/docs/releases/latest/matcher.html#orderedEquals

[floitsch, 2013-06-26 18:06:39]

LGTM.
Some MDV/Observable things are over my head, so please wait for the LGTM from
the other reviewers.

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/mdv.dart
File pkg/mdv/lib/mdv.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/mdv.dart#newco...
pkg/mdv/lib/mdv.dart:81: // TODO(jmesserly): is using nodeType and localName
faster than "is"?
`is` should be fast. Afaik dart2js compiles it to a simple method invocation,
and in the VM it isn't slow either.

[blois, 2013-06-26 18:39:37]

DOM changes lgtm

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:16283: Function mdvPackage = (node) {
I'm a bit concerned about adding unqualified top-level members given the
post-1.0 compatibility issues of adding new ones. This one itself won't cause
issues, but we won't be able to follow the same pattern later.

Would prefer to use a pattern we can follow in the future.

[Jennifer Messerly, 2013-06-26 18:47:24]

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:16283: Function mdvPackage = (node) {
On 2013/06/26 18:39:37, blois wrote:
> I'm a bit concerned about adding unqualified top-level members given the
> post-1.0 compatibility issues of adding new ones. This one itself won't cause
> issues, but we won't be able to follow the same pattern later.
> 
> Would prefer to use a pattern we can follow in the future.

That's fair. I feel the same way. I desperately wish we had another solution.

I'm caught between:
* the Dart language, that doesn't give libraries a way to extend HTML APIs
(unlike JavaScript)
* the dart2js implementation, that causes too much size overhead if
Node.noSuchMethod is added
  - and Dart tooling, that wouldn't autocomplete correctly if NSM used
* the web standards (MDV) effort to add native data binding capabilities
* EcmaScript embracing Object.observe
* Dart not embracing dart:observe
* Polymer choosing to be built on MDV
* Wanting to get Polymer UI to our users
* Wanting to not have less friendly APIs than JS.

... HALP!?!!

[Jennifer Messerly, 2013-06-26 22:27:09]

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/17552019/diff/5001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:16283: Function mdvPackage = (node) {
On 2013/06/26 18:47:24, John Messerly wrote:
> On 2013/06/26 18:39:37, blois wrote:
> > I'm a bit concerned about adding unqualified top-level members given the
> > post-1.0 compatibility issues of adding new ones. This one itself won't
cause
> > issues, but we won't be able to follow the same pattern later.
> > 
> > Would prefer to use a pattern we can follow in the future.
> 
> That's fair. I feel the same way. I desperately wish we had another solution.
> 
> I'm caught between:
> * the Dart language, that doesn't give libraries a way to extend HTML APIs
> (unlike JavaScript)
> * the dart2js implementation, that causes too much size overhead if
> Node.noSuchMethod is added
>   - and Dart tooling, that wouldn't autocomplete correctly if NSM used
> * the web standards (MDV) effort to add native data binding capabilities
> * EcmaScript embracing Object.observe
> * Dart not embracing dart:observe
> * Polymer choosing to be built on MDV
> * Wanting to get Polymer UI to our users
> * Wanting to not have less friendly APIs than JS.
> 
> ... HALP!?!!

Chatting with Pete it sounds like the main concern here is something about
adding top level members being a breaking change. I'll just make this static on
TemplateElement...

[Jennifer Messerly, 2013-06-26 23:09:12]

Thanks guys :)

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/mdv.dart
File pkg/mdv/lib/mdv.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/mdv.dart#newco...
pkg/mdv/lib/mdv.dart:81: // TODO(jmesserly): is using nodeType and localName
faster than "is"?
On 2013/06/26 18:06:39, floitsch wrote:
> `is` should be fast. Afaik dart2js compiles it to a simple method invocation,
> and in the VM it isn't slow either.

thanks for the info! removed TODO.

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/bindings.dart
File pkg/mdv/lib/src/bindings.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/bindings.d...
pkg/mdv/lib/src/bindings.dart:121: class _Bindings {
On 2013/06/25 23:03:31, justinfagnani wrote:
> should this class become a library?

not currently, there are various internals that are shared within package:mdv.

I could get rid of the class though and make everything top level. Adding the
class was a request by Pete when it was all part of dart:html, because that is
one giant flat namespace.

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/node.dart
File pkg/mdv/lib/src/node.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/node.dart#...
pkg/mdv/lib/src/node.dart:18: // TODO(jmesserly): should we throw instead?
On 2013/06/25 23:03:31, justinfagnani wrote:
> I don't think so

thanks, removed TODO

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/template.dart
File pkg/mdv/lib/src/template.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/mdv/lib/src/template.d...
pkg/mdv/lib/src/template.dart:69: * Setting this will destructive propagate the
value to all descendant nodes,
On 2013/06/25 23:03:31, justinfagnani wrote:
> can we do an identity check to avoid churn in some cases?

fixed comment. it wasn't accurate (in light of future MDV changes)

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/observe.dart
File pkg/observe/lib/observe.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/observe.da...
pkg/observe/lib/observe.dart:253: * literals into the corresponding observable
collection type.
On 2013/06/25 23:03:31, justinfagnani wrote:
> It may be obvious, but do you want to mention that the collection is only
> observable when modified via the same observable wrapper instance returned
from
> this function?

attempted to clarify comment

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/src/observ...
File pkg/observe/lib/src/observable_map.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/lib/src/observ...
pkg/observe/lib/src/observable_map.dart:71: other.forEach((key, value) {
result[key] = value; });
On 2013/06/25 23:03:31, justinfagnani wrote:
> Map has addAll() now:
> http://api.dartlang.org/docs/releases/latest/dart_core/Map.html#addAll

filed a bug: https://code.google.com/p/dart/issues/detail?id=11539

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/test/utils.dart
File pkg/observe/test/utils.dart (right):

https://codereview.chromium.org/17552019/diff/5001/pkg/observe/test/utils.dar...
pkg/observe/test/utils.dart:10: // compare to toStrings.
On 2013/06/25 23:03:31, justinfagnani wrote:
> there are list matchers:
> http://api.dartlang.org/docs/releases/latest/matcher.html#orderedEquals

the issue is we need to compare the ChangeRecord types and they don't implement
equality. i'll fix comment

[Jennifer Messerly, 2013-06-26 23:26:12]

Committed patchset #5 manually as r24498 (presubmit successful).