Context menus should appear above the touch point if invoked by long press.

Context menus should appear above the touch point if invoked by long press.
This CL does this for content area context menus only. I will send out another CL for same functionality on views controls.
We acheive this as follows: We remember in render_widget_host, which kind of
event is currently in process. When we get a request for showing context menu,
in render_view_host, we check if currently we are processing a gesture event,
and if so, we display the context menu appropriately.

BUG=142015
TEST=manual

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

[varunjain, 2012-09-05 22:59:22]

creis for content/*
sky for rest.

[sky, 2012-09-05 23:57:00]

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/browser/ren...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/browser/ren...
content/browser/renderer_host/render_view_host_impl.cc:1245:
content::ContextMenuEventType type = content::NO_CONTEXT_MENU;
When can we hit this?

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/public/comm...
File content/public/common/context_menu_event_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/public/comm...
content/public/common/context_menu_event_type.h:11: NO_CONTEXT_MENU,
Should this be named ContextMenuSourceType?

Either way, it would be nice if these started with the name of the enum, eg
CONTEXT_MENU_SOURCE_NONE (or maybe this should be _OTHER, but I have a question
related to when NO_CONTEXT_MENU is seen in OnMsgContextMenu),
CONTEXT_MENU_SOURCE_KEYBOARD ...

[varunjain, 2012-09-06 03:49:55]

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/browser/ren...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/browser/ren...
content/browser/renderer_host/render_view_host_impl.cc:1245:
content::ContextMenuEventType type = content::NO_CONTEXT_MENU;
On 2012/09/05 23:57:00, sky wrote:
> When can we hit this?

I included this to represent the case where javascript invoked a context menu.
But I think its the same as right click. Removed.

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/public/comm...
File content/public/common/context_menu_event_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7001/content/public/comm...
content/public/common/context_menu_event_type.h:11: NO_CONTEXT_MENU,
On 2012/09/05 23:57:00, sky wrote:
> Should this be named ContextMenuSourceType?
> 
> Either way, it would be nice if these started with the name of the enum, eg
> CONTEXT_MENU_SOURCE_NONE (or maybe this should be _OTHER, but I have a
question
> related to when NO_CONTEXT_MENU is seen in OnMsgContextMenu),
> CONTEXT_MENU_SOURCE_KEYBOARD ...

Done.

[sky, 2012-09-06 16:35:03]

Only thing holding up approval is why you want BOTTOMCENTER here.

https://chromiumcodereview.appspot.com/10917102/diff/7003/chrome/browser/ui/v...
File chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
(right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/chrome/browser/ui/v...
chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc:51:
views::MenuItemView::BOTTOMCENTER : views::MenuItemView::TOPLEFT;
Why do we want BOTTOMCENTER for long press?

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
File content/public/common/context_menu_source_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
content/public/common/context_menu_source_type.h:13:
CONTEXT_MENU_SOURCE_GESTURE,
You sure you don't want _TOUCH? At a minimum document its from a touch device.

[varunjain, 2012-09-06 16:43:45]

https://chromiumcodereview.appspot.com/10917102/diff/7003/chrome/browser/ui/v...
File chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc
(right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/chrome/browser/ui/v...
chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc:51:
views::MenuItemView::BOTTOMCENTER : views::MenuItemView::TOPLEFT;
On 2012/09/06 16:35:03, sky wrote:
> Why do we want BOTTOMCENTER for long press?

request from UX was to display the context menu centered above the finger for
long press. For right click with mouse, the context menu should remain where it
is.

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
File content/public/common/context_menu_source_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
content/public/common/context_menu_source_type.h:13:
CONTEXT_MENU_SOURCE_GESTURE,
On 2012/09/06 16:35:03, sky wrote:
> You sure you don't want _TOUCH? At a minimum document its from a touch device.

there is no way to invoke context menu from a touch event. The long press event
(or two finger tap) is a gesture event that invokes the context menu. Added
comment as you suggested.

[sky, 2012-09-06 17:04:19]

LGTM

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
File content/public/common/context_menu_source_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
content/public/common/context_menu_source_type.h:13:
CONTEXT_MENU_SOURCE_GESTURE,
On 2012/09/06 16:43:45, varunjain wrote:
> On 2012/09/06 16:35:03, sky wrote:
> > You sure you don't want _TOUCH? At a minimum document its from a touch
device.
> 
> there is no way to invoke context menu from a touch event. The long press
event
> (or two finger tap) is a gesture event that invokes the context menu. Added
> comment as you suggested.

The reason I don't like GESTURE is there are other types of gestures and in
particular mouse gestures. But this is only used from touch.

[varunjain, 2012-09-06 17:11:42]

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
File content/public/common/context_menu_source_type.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/7003/content/public/comm...
content/public/common/context_menu_source_type.h:13:
CONTEXT_MENU_SOURCE_GESTURE,
On 2012/09/06 17:04:19, sky wrote:
> On 2012/09/06 16:43:45, varunjain wrote:
> > On 2012/09/06 16:35:03, sky wrote:
> > > You sure you don't want _TOUCH? At a minimum document its from a touch
> device.
> > 
> > there is no way to invoke context menu from a touch event. The long press
> event
> > (or two finger tap) is a gesture event that invokes the context menu. Added
> > comment as you suggested.
> 
> The reason I don't like GESTURE is there are other types of gestures and in
> particular mouse gestures. But this is only used from touch.

changed to TOUCH

[Avi (use Gerrit), 2012-09-06 19:25:14]

content LGTM

[commit-bot: I haz the power, 2012-09-06 19:50:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/10917102/110

[commit-bot: I haz the power, 2012-09-06 21:13:26]

Try job failure for 10917102-110 (retry) on linux_rel for steps
"interactive_ui_tests, content_unittests".
It's a second try, previously, steps "interactive_ui_tests, content_unittests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-09-06 21:37:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/10917102/6055

[commit-bot: I haz the power, 2012-09-06 22:54:59]

Try job failure for 10917102-6055 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-09-06 23:09:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/10917102/6055

[commit-bot: I haz the power, 2012-09-06 23:30:18]

Try job failure for 10917102-6055 (retry) on win for step "runhooks".
It's a second try, previously, step "runhooks" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-09-07 00:45:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/10917102/6055

[commit-bot: I haz the power, 2012-09-07 02:36:39]

Try job failure for 10917102-6055 (retry) on mac_rel for step
"sync_integration_tests".
It's a second try, previously, step "sync_integration_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jam, 2012-09-07 17:07:34]

https://chromiumcodereview.appspot.com/10917102/diff/6055/content/public/brow...
File content/public/browser/web_contents_view_delegate.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/6055/content/public/brow...
content/public/browser/web_contents_view_delegate.h:48: const
content::ContextMenuSourceType& type) = 0;
nit: we don't pass enums by const reference, just content::ContextMenuSourceType
type

[jam, 2012-09-07 17:07:55]

https://chromiumcodereview.appspot.com/10917102/diff/6055/content/port/browse...
File content/port/browser/render_view_host_delegate_view.h (right):

https://chromiumcodereview.appspot.com/10917102/diff/6055/content/port/browse...
content/port/browser/render_view_host_delegate_view.h:37: const
ContextMenuSourceType& type) {}
ditto