First cut at behaviors. This just implements the lifecycle methodsportion. We may get the rest for …

First cut at behaviors. This just implements the lifecycle methodsportion. We may get the rest for free (properties, observers, listeners, hostAttributes), but haven't tested those yet

BUG=

[Siggi Cherem (dart-lang), 2015-08-12 22:43:15]

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dart
File lib/src/common/behavior.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:1: library polymer.src.common.behavior;
+copyright

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:8: typedef void _VoidFn();
delete? (seems unused)

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:11: abstract class BehaviorInterface {
how about rename to BehaviorAnnotation ?

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:51: : super(declarationsCapability, typeCapability,
const StaticInvokeCapability(_lifecycleMethodsPattern));
nit: 80/formatter

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:57: final List<String> _jsPath;
Given how it is going to be a path with dots in JS, it might be worth
considering adding support for that syntax here too:

  @BehaviorProxy('Foo.Bar') class DartBar {}


Maybe add a TODO?

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
File lib/src/common/declarations.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:6: typedef bool _DeclarationWhereFn(String
name, DeclarationMirror declaration);
naming nit: maybe use Function or Callback instead of Fn (here and below).

Or, since this is just used once, you could inline this in the argument type.
For example:

  ..., {bool where(ClassMirror mirror)}) {
  ...

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:14: while(superClass != null &&
superClass.reflectedType != PolymerElement) {
run formatter

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:29: while (superClass != null &&
superClass.reflectedType != PolymerElement) {
just to double check - do we ever need to fetch something from the
PolymerElement class? or is it safe that we are skipping it? (In the past we
used to stop at HtmlElement instead)

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_desc...
File lib/src/common/polymer_descriptor.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_desc...
lib/src/common/polymer_descriptor.dart:198: var interfaces = mixinsFor(type,
jsProxyReflectable,
Seems like we are visiting each behavior annotation twice. I wonder if it's
worth adding a "forEach" besides the current query APIs so you can do some extra
work while walking up the chain:

mixinForEach(type, jsProxyReflectable, (mixin) {
  BehaviorAnnotation meta = mixin.metadata.firstWhere(isBehavior,
      orElse: () => null);
  if (meta != null) behaviors.add(meta.getBehavior(mixin.reflectedType));
});

WDYT?

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_mixi...
File lib/src/common/polymer_mixin.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_mixi...
lib/src/common/polymer_mixin.dart:14: abstract class PolymerMixin implements
JsProxy, CustomElementProxyMixin {
not sure I follow - why is this change needed for behaviors?

[jakemac, 2015-08-13 17:50:44]

jakemac@google.com changed reviewers:
+ jakemac@google.com

[jakemac, 2015-08-13 17:50:45]

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dart
File lib/src/common/behavior.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:1: library polymer.src.common.behavior;
On 2015/08/12 22:43:14, Siggi Cherem (dart-lang) wrote:
> +copyright

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:8: typedef void _VoidFn();
On 2015/08/12 22:43:14, Siggi Cherem (dart-lang) wrote:
> delete? (seems unused)

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:11: abstract class BehaviorInterface {
On 2015/08/12 22:43:14, Siggi Cherem (dart-lang) wrote:
> how about rename to BehaviorAnnotation ?

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:51: : super(declarationsCapability, typeCapability,
const StaticInvokeCapability(_lifecycleMethodsPattern));
On 2015/08/12 22:43:14, Siggi Cherem (dart-lang) wrote:
> nit: 80/formatter

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/behavior.dar...
lib/src/common/behavior.dart:57: final List<String> _jsPath;
On 2015/08/12 22:43:14, Siggi Cherem (dart-lang) wrote:
> Given how it is going to be a path with dots in JS, it might be worth
> considering adding support for that syntax here too:
> 
>   @BehaviorProxy('Foo.Bar') class DartBar {}
> 
> 
> Maybe add a TODO?

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
File lib/src/common/declarations.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:6: typedef bool _DeclarationWhereFn(String
name, DeclarationMirror declaration);
On 2015/08/12 22:43:15, Siggi Cherem (dart-lang) wrote:
> naming nit: maybe use Function or Callback instead of Fn (here and below).
> 
> Or, since this is just used once, you could inline this in the argument type.
> For example:
> 
>   ..., {bool where(ClassMirror mirror)}) {
>   ...

Opted for the inline type annotation, I forgot you could do that :P

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:14: while(superClass != null &&
superClass.reflectedType != PolymerElement) {
On 2015/08/12 22:43:15, Siggi Cherem (dart-lang) wrote:
> run formatter 

Done.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/declarations...
lib/src/common/declarations.dart:29: while (superClass != null &&
superClass.reflectedType != PolymerElement) {
On 2015/08/12 22:43:15, Siggi Cherem (dart-lang) wrote:
> just to double check - do we ever need to fetch something from the
> PolymerElement class? or is it safe that we are skipping it? (In the past we
> used to stop at HtmlElement instead)

Its fine to stop at PolymerElement, although I actually changed this to stop at 
the PolymerMixin mixin instead, but that is effectively the same thing.
Basically we just need all values that could possibly be used in a template.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_desc...
File lib/src/common/polymer_descriptor.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_desc...
lib/src/common/polymer_descriptor.dart:198: var interfaces = mixinsFor(type,
jsProxyReflectable,
On 2015/08/12 22:43:15, Siggi Cherem (dart-lang) wrote:
> Seems like we are visiting each behavior annotation twice. I wonder if it's
> worth adding a "forEach" besides the current query APIs so you can do some
extra
> work while walking up the chain:
> 
> mixinForEach(type, jsProxyReflectable, (mixin) {
>   BehaviorAnnotation meta = mixin.metadata.firstWhere(isBehavior,
>       orElse: () => null);
>   if (meta != null) behaviors.add(meta.getBehavior(mixin.reflectedType));
> });
> 
> WDYT?

I didn't add mixinForEach, but I did modify it to only check the annotations
once using similar logic.

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_mixi...
File lib/src/common/polymer_mixin.dart (right):

https://codereview.chromium.org/1290643006/diff/1/lib/src/common/polymer_mixi...
lib/src/common/polymer_mixin.dart:14: abstract class PolymerMixin implements
JsProxy, CustomElementProxyMixin {
On 2015/08/12 22:43:15, Siggi Cherem (dart-lang) wrote:
> not sure I follow - why is this change needed for behaviors?

The js behaviors require an implementation of `CustomElementProxyMixin`, so I
needed to specify this here in order to be able to mix them in (its already
implementing the same api).

[Siggi Cherem (dart-lang), 2015-08-13 18:52:41]

lgtm

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/behavior...
File lib/src/common/behavior.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/behavior...
lib/src/common/behavior.dart:1: // Copyright (c) 2014, the Dart project authors.
 Please see the AUTHORS file
2015 :)

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/js_proxy...
File lib/src/common/js_proxy.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/js_proxy...
lib/src/common/js_proxy.dart:67: return !declaration.owner.metadata.any((m) => m
is BehaviorProxy);
should we have a type to declare things that don't require the js constructor?
(BehaviorProxy is currently the only one, but wouldn't that be necessary for any
other mixin that relies on code from JS?) I guess maybe there is nothing else
like it in the polymer.js world?

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_base...
File lib/src/js/polymer_base_dart.html (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_base...
lib/src/js/polymer_base_dart.html:2: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
2015

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_bind...
File lib/src/js/polymer_bind_dart.html (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_bind...
lib/src/js/polymer_bind_dart.html:2: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
+1 :)

https://codereview.chromium.org/1290643006/diff/20001/test/common.dart
File test/common.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/test/common.dart#newcode1
test/common.dart:1: //Copyright (c) 2014, the Dart project authors.  Please see
the AUTHORS file
++

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/js_prox...
File test/src/common/js_proxy_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/js_prox...
test/src/common/js_proxy_test.html:3: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
++

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
File test/src/common/polymer_descriptor_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
test/src/common/polymer_descriptor_test.html:3: Copyright (c) 2014, the Dart
project authors.  Please see the AUTHORS file
++

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
File test/src/common/polymer_mixin_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
test/src/common/polymer_mixin_test.html:3: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
++ (Seems this might be the case on all of them)

[jakemac, 2015-08-13 19:14:30]

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/behavior...
File lib/src/common/behavior.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/behavior...
lib/src/common/behavior.dart:1: // Copyright (c) 2014, the Dart project authors.
 Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> 2015 :)

Acknowledged.

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/js_proxy...
File lib/src/common/js_proxy.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/common/js_proxy...
lib/src/common/js_proxy.dart:67: return !declaration.owner.metadata.any((m) => m
is BehaviorProxy);
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> should we have a type to declare things that don't require the js constructor?
> (BehaviorProxy is currently the only one, but wouldn't that be necessary for
any
> other mixin that relies on code from JS?) I guess maybe there is nothing else
> like it in the polymer.js world?

Nothing else that I know of today, I think we can add that if needed later on.

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_base...
File lib/src/js/polymer_base_dart.html (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_base...
lib/src/js/polymer_base_dart.html:2: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> 2015

Done.

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_bind...
File lib/src/js/polymer_bind_dart.html (right):

https://codereview.chromium.org/1290643006/diff/20001/lib/src/js/polymer_bind...
lib/src/js/polymer_bind_dart.html:2: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> +1 :)

Done.

https://codereview.chromium.org/1290643006/diff/20001/test/common.dart
File test/common.dart (right):

https://codereview.chromium.org/1290643006/diff/20001/test/common.dart#newcode1
test/common.dart:1: //Copyright (c) 2014, the Dart project authors.  Please see
the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> ++

Done.

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/js_prox...
File test/src/common/js_proxy_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/js_prox...
test/src/common/js_proxy_test.html:3: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> ++

Done.

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
File test/src/common/polymer_descriptor_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
test/src/common/polymer_descriptor_test.html:3: Copyright (c) 2014, the Dart
project authors.  Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> ++

Done.

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
File test/src/common/polymer_mixin_test.html (right):

https://codereview.chromium.org/1290643006/diff/20001/test/src/common/polymer...
test/src/common/polymer_mixin_test.html:3: Copyright (c) 2014, the Dart project
authors.  Please see the AUTHORS file
On 2015/08/13 18:52:41, Siggi Cherem (dart-lang) wrote:
> ++ (Seems this might be the case on all of them)

Done.