Make Element.animate work in dart2js Chrome

Make Element.animate work in dart2js Chrome

BUG=
R=terry@google.com

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

[Alan Knight, 2015-03-17 23:29:39]

alanknight@google.com changed reviewers:
+ terry@google.com

[sra1, 2015-03-18 00:20:46]

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

[sra1, 2015-03-18 00:20:47]

DBC.

Is there a test?

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:793: *
1. Indent +4
2. Missing ]
3. Inconsistent space on second opacity.

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:811: // If frames is an
AnimationEffect, leave it alone?
Be more defensive. Fail if not a list or AnimationEffect.

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:815: (x) =>
convertDartToNative_Dictionary(x)).toList();
frames.map(convertDartToNative_Dictionary).toList()

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:821: return
_animate(convertedFrames, convertedTiming);
What if timing is null, does it work for both is null and is undefined? Probably
best to call _animate with one arg if timing == null.

[sra1, 2015-03-18 00:20:59]

DBC.

Is there a test?

[sra1, 2015-03-18 00:21:00]

DBC.

Is there a test?

[sra1, 2015-03-18 00:23:38]

DBC.

Is there a test?

[Alan Knight, 2015-03-18 20:09:20]

DBC?

Had forgotten to 'git add' the test file.

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:793: *
On 2015/03/18 00:20:47, sra1 wrote:
> 1. Indent +4
> 2. Missing ]
> 3. Inconsistent space on second opacity.

1. dartfmt says only +2
2. Done
3. Done

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:811: // If frames is an
AnimationEffect, leave it alone?
On 2015/03/18 00:20:47, sra1 wrote:
> Be more defensive. Fail if not a list or AnimationEffect.

Oops. It must de facto be a List, that comment was old. It turns out
AnimationEffect is not constructable. Checked for a List and that the entries
are Maps.

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:815: (x) =>
convertDartToNative_Dictionary(x)).toList();
On 2015/03/18 00:20:47, sra1 wrote:
> frames.map(convertDartToNative_Dictionary).toList()

Oh yeah. Nice. Done.

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:821: return
_animate(convertedFrames, convertedTiming);
On 2015/03/18 00:20:47, sra1 wrote:
> What if timing is null, does it work for both is null and is undefined?
Probably
> best to call _animate with one arg if timing == null.

It seems to make no difference between 0, null, and undefined. Changed the way
it's called. Added test.

[kevmoo, 2015-03-18 22:03:29]

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

[kevmoo, 2015-03-18 22:03:29]

DBC

https://codereview.chromium.org/1014843004/diff/40001/tests/html/html.status
File tests/html/html.status (right):

https://codereview.chromium.org/1014843004/diff/40001/tests/html/html.status#...
tests/html/html.status:18: element_animate_test: Fail # Not supported on Dartium
Should we have an issue to make this work on Dartium?

[sra1, 2015-03-19 02:20:46]

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:793: *
On 2015/03/18 20:09:20, Alan Knight wrote:
> On 2015/03/18 00:20:47, sra1 wrote:
> > 1. Indent +4
> > 2. Missing ]
> > 3. Inconsistent space on second opacity.
> 
> 1. dartfmt says only +2

Markdown in this doc comment starts recognizing a block as sample code at +4 but
the 'var' line is +6 from 'Examples'. Make it +4


> 2. Done
> 3. Done

[terry, 2015-03-19 17:03:16]

lgtm just minor nit.

https://codereview.chromium.org/1014843004/diff/60001/tests/html/element_anim...
File tests/html/element_animate_test.dart (right):

https://codereview.chromium.org/1014843004/diff/60001/tests/html/element_anim...
tests/html/element_animate_test.dart:51: }
Should we have a test where the dictionary fails e.g., timing parameter is a
{'xyzzy': 'foobar'}

Both dartium and chrome should fail if a dictionary name isn't valid.

[Alan Knight, 2015-03-23 18:26:16]

Committed patchset #4 (id:60001) manually as 44642 (presubmit successful).

[Alan Knight, 2015-03-23 18:57:52]

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/1014843004/diff/20001/tools/dom/templates/htm...
tools/dom/templates/html/impl/impl_Element.darttemplate:793: *
On 2015/03/19 02:20:46, sra1 wrote:
> On 2015/03/18 20:09:20, Alan Knight wrote:
> > On 2015/03/18 00:20:47, sra1 wrote:
> > > 1. Indent +4
> > > 2. Missing ]
> > > 3. Inconsistent space on second opacity.
> > 
> > 1. dartfmt says only +2
> 
> Markdown in this doc comment starts recognizing a block as sample code at +4
but
> the 'var' line is +6 from 'Examples'. Make it +4
> 
> 
> > 2. Done
> > 3. Done
> 

Ah, I misunderstood. Done.

https://codereview.chromium.org/1014843004/diff/60001/tests/html/element_anim...
File tests/html/element_animate_test.dart (right):

https://codereview.chromium.org/1014843004/diff/60001/tests/html/element_anim...
tests/html/element_animate_test.dart:51: }
On 2015/03/19 17:03:15, terry wrote:
> Should we have a test where the dictionary fails e.g., timing parameter is a
> {'xyzzy': 'foobar'}
> 
> Both dartium and chrome should fail if a dictionary name isn't valid.

It would be nice if they did, but Chrome happily accepts it and returns an
animation player. Dartium throws an unsupported operation for any animate call.
I'm making a separate bug for that.