Port PopupMenu to the new animation system

Port PopupMenu to the new animation system

This CL is the first step in the port. The second step will be to decompose the
animation in to a number of variables, one for the menu itself and one for each
of the menu items.

R=mpcomplete@chromium.org, mpcomplete@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/abb73abef272ae26d21fd19217b44df4e94f9beb

[Matt Perry, 2015-07-10 00:32:17]

mpcomplete@chromium.org changed reviewers:
+ mpcomplete@chromium.org

[Matt Perry, 2015-07-10 00:32:18]

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
File sky/sdk/lib/widgets/popup_menu.dart (right):

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:42: MenuState get state => _state;
This might be better if it were inferred from the timeline state.
 timeline animating forwards => opening
 else timeline animating backwards => closing
 else timeline == 0 => closed
 else timeline == 1 => opened
WDYT? We'd need to add some stuff to the Timeline for that, but I think it's
useful in general. Drawer has a similar requirement.

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:72: return new Future.value(null);
Do you mean to return _closeCompleter.future here? (I'm guessing that would
allow multiple callers to await on the close Future?)

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:79: ..reverse(delay: _kMenuCloseDelay);
a timeout API would work here as long as we cancelled it in open().

[abarth-chromium, 2015-07-10 02:42:11]

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
File sky/sdk/lib/widgets/popup_menu.dart (right):

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:42: MenuState get state => _state;
On 2015/07/10 at 00:32:17, Matt Perry wrote:
> This might be better if it were inferred from the timeline state.
>  timeline animating forwards => opening
>  else timeline animating backwards => closing
>  else timeline == 0 => closed
>  else timeline == 1 => opened
> WDYT? We'd need to add some stuff to the Timeline for that, but I think it's
useful in general. Drawer has a similar requirement.

Yeah, that's a good idea.  Would it be ok to do that in a follow up CL?

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:72: return new Future.value(null);
On 2015/07/10 at 00:32:17, Matt Perry wrote:
> Do you mean to return _closeCompleter.future here? (I'm guessing that would
allow multiple callers to await on the close Future?)

That's different from what the old code did, but that makes a lot of sense. 
Will do.

https://codereview.chromium.org/1228233004/diff/1/sky/sdk/lib/widgets/popup_m...
sky/sdk/lib/widgets/popup_menu.dart:79: ..reverse(delay: _kMenuCloseDelay);
On 2015/07/10 at 00:32:17, Matt Perry wrote:
> a timeout API would work here as long as we cancelled it in open().

If you'd prefer, I'm happy to switch this to a timeout API and add a comment
explaining the situation.

[abarth-chromium, 2015-07-10 03:21:19]

Comments addressed, PTAL

[Matt Perry, 2015-07-10 17:10:35]

LGTM, thanks

https://codereview.chromium.org/1228233004/diff/20001/sky/sdk/lib/widgets/pop...
File sky/sdk/lib/widgets/popup_menu.dart (right):

https://codereview.chromium.org/1228233004/diff/20001/sky/sdk/lib/widgets/pop...
sky/sdk/lib/widgets/popup_menu.dart:76: return _closeCompleter.future;
Does this still work after _closeCompleter.complete() is called?

[abarth-chromium, 2015-07-10 17:23:00]

https://codereview.chromium.org/1228233004/diff/20001/sky/sdk/lib/widgets/pop...
File sky/sdk/lib/widgets/popup_menu.dart (right):

https://codereview.chromium.org/1228233004/diff/20001/sky/sdk/lib/widgets/pop...
sky/sdk/lib/widgets/popup_menu.dart:76: return _closeCompleter.future;
On 2015/07/10 at 17:10:35, Matt Perry wrote:
> Does this still work after _closeCompleter.complete() is called?

Yes.  It gives you a promise that is already resolve, which means when you wait
on it, you get back the value right away.  We use this in the image cache for
example.

[abarth-chromium, 2015-07-10 17:23:56]

Committed patchset #2 (id:20001) manually as
abb73abef272ae26d21fd19217b44df4e94f9beb (presubmit successful).