Support touch scroll gestures in menus.

Support touch scroll gestures in menus.

Depends on http://codereview.chromium.org/8364039/


BUG=None
TEST=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118834

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118841

[Ian Vollick, 2011-11-09 20:29:06]

+gajen

[Ian Vollick, 2011-11-10 16:38:16]

On 2011/11/09 20:29:06, vollick wrote:
> +gajen

Merge with http://codereview.chromium.org/8364039/. Thanks to Gajen's changes,
the menu gestures now work out of the box!

[Ian Vollick, 2011-11-15 18:59:26]

On 2011/11/10 16:38:16, vollick wrote:
> On 2011/11/09 20:29:06, vollick wrote:
> > +gajen
> 
> Merge with http://codereview.chromium.org/8364039/. Thanks to Gajen's changes,
> the menu gestures now work out of the box!

You can now flick menus, too.

[sadrul, 2012-01-19 04:44:16]

The GR is now checked in. I think this can get in now?

[sadrul, 2012-01-19 15:29:29]

I haven't reviewed views/animation/ since I am not really familiar with it.

http://codereview.chromium.org/8508024/diff/24003/ui/views/controls/menu/menu...
File ui/views/controls/menu/menu_host_root_view.cc (right):

http://codereview.chromium.org/8508024/diff/24003/ui/views/controls/menu/menu...
ui/views/controls/menu/menu_host_root_view.cc:66: #if defined(OS_LINUX)
Why is this ifdef necessary? My understanding is once we already have a
views::GestureEvent, the event processing from thereon should be fine on all
platforms?

[Ian Vollick, 2012-01-19 15:42:22]

On 2012/01/19 15:29:29, sadrul wrote:
> I haven't reviewed views/animation/ since I am not really familiar with it.
> 
>
http://codereview.chromium.org/8508024/diff/24003/ui/views/controls/menu/menu...
> File ui/views/controls/menu/menu_host_root_view.cc (right):
> 
>
http://codereview.chromium.org/8508024/diff/24003/ui/views/controls/menu/menu...
> ui/views/controls/menu/menu_host_root_view.cc:66: #if defined(OS_LINUX)
> Why is this ifdef necessary? My understanding is once we already have a
> views::GestureEvent, the event processing from thereon should be fine on all
> platforms?
You're right. This shouldn't be linux-only. I've moved the gesture processing
code out of the ifdefs.

[sadrul, 2012-01-19 15:46:46]

LGTM

It's likely that you will need to update the copyright headers s/2011/2012/ in
the files before landing.

http://codereview.chromium.org/8508024/diff/30004/ui/views/controls/menu/subm...
File ui/views/controls/menu/submenu_view.h (right):

http://codereview.chromium.org/8508024/diff/30004/ui/views/controls/menu/subm...
ui/views/controls/menu/submenu_view.h:14: #include
"ui/views/animation/scroll_animator.h"
sort

[sadrul, 2012-01-19 15:47:20]

On 2012/01/19 15:46:46, sadrul wrote:
> LGTM

LGTM (did not review views/animation/)

> 
> It's likely that you will need to update the copyright headers s/2011/2012/ in
> the files before landing.
> 
>
http://codereview.chromium.org/8508024/diff/30004/ui/views/controls/menu/subm...
> File ui/views/controls/menu/submenu_view.h (right):
> 
>
http://codereview.chromium.org/8508024/diff/30004/ui/views/controls/menu/subm...
> ui/views/controls/menu/submenu_view.h:14: #include
> "ui/views/animation/scroll_animator.h"
> sort

[rjkroege, 2012-01-19 16:15:07]

ancient comments already handled by sadrul. 

And the wiring of gestures is now semi-stabilized so you can ignore that.

LGTM

http://codereview.chromium.org/8508024/diff/12001/views/controls/menu/menu_co...
File views/controls/menu/menu_controller.h (right):

http://codereview.chromium.org/8508024/diff/12001/views/controls/menu/menu_co...
views/controls/menu/menu_controller.h:103: #if defined(OS_LINUX)
does this need the #ifdef?

http://codereview.chromium.org/8508024/diff/12001/views/controls/menu/menu_ho...
File views/controls/menu/menu_host_root_view.cc (right):

http://codereview.chromium.org/8508024/diff/12001/views/controls/menu/menu_ho...
views/controls/menu/menu_host_root_view.cc:73: // TODO(vollick): gestures in
Windows.
you need to hook up with cpu about this I think

[sadrul, 2012-01-24 02:41:42]

ping!

[rjkroege, 2012-01-24 13:59:51]

On 2012/01/24 02:41:42, sadrul wrote:
> ping!

still LGTM