Context menu on views must show on mouse dpwn for non-WIN.

ui/views/view.cc

Index: ui/views/view.cc
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 98e3b44b210a9652d9abf9a81f277fa4edd7421d..d75012dceedb65b112e3254b9b303fff76c1e431 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -57,6 +57,12 @@ bool use_acceleration_when_possible = true;
 bool use_acceleration_when_possible = false;
 #endif
 
+#if defined(OS_WIN)
+bool context_menu_on_mouse_press = false;
+#else
+bool context_menu_on_mouse_press = true;
+#endif
+
 // Saves the drawing state, and restores the state when going out of scope.
 class ScopedCanvas {
  public:
@@ -2046,6 +2052,16 @@ bool View::ProcessMousePressed(const ui::MouseEvent& event) {
   if (!enabled)
     return result;
 
+  if (event.IsOnlyRightMouseButton() && context_menu_controller_ &&
+      context_menu_on_mouse_press) {

[sadrul, 2013/03/04 23:22:04]

Note the WARNING on 2050.

I think you should first check here that the view didn't already process the
mouse-press event (i.e. |result| == false). So this condition would be:

  if (!result && context_menu_on_mouse_press && context_menu_controller) { ... }

That should take care of the warning (I think it is a reasonable requirement
that if a View destroys itself from an event-handler, then it should claim that
it has processed the event).

[varunjain, 2013/03/05 00:04:28]

Ha! I missed that warning. Although, the GetDragInfo below does not seem to
care.
The problem with checking for result is that some views return true even if they
are not deleted. What you are suggesting changes the existing behavior which is
that we show context menu even if a view returns true for OnMousePressed. I
looked around a bit and saw that ProcessMouseReleased has the same problem (view
may be deleted in OnMouseReleased). There, existence of a
context_menu_controller is considered evidence that the view is not deleted (see
comment on line 2097). Although I dont understand the validity of that
assertion, I think we can do the same thing here?

[sadrul, 2013/03/05 16:30:11]

Good catch. Perhaps GetDragInfo() should be cached before the call to
OnMousePressed too.
Ew, but I suppose it should hold true in both places, yeah.

[varunjain, 2013/03/05 21:14:21]

Done.

+    gfx::Point location(event.location());
+    if (HitTestPoint(location)) {
+      ConvertPointToScreen(this, &location);
+      ShowContextMenu(location, true);
+      return true;
+    }
+  }
+
   if (drag_operations != ui::DragDropTypes::DRAG_NONE) {
     GetDragInfo()->PossibleDrag(event.location());
     return true;
@@ -2076,7 +2092,8 @@ bool View::ProcessMouseDragged(const ui::MouseEvent& event) {
 }
 
 void View::ProcessMouseReleased(const ui::MouseEvent& event) {
-  if (context_menu_controller_ && event.IsOnlyRightMouseButton()) {
+  if (!context_menu_on_mouse_press && context_menu_controller_ &&
+      event.IsOnlyRightMouseButton()) {
     // Assume that if there is a context menu controller we won't be deleted
     // from mouse released.
     gfx::Point location(event.location());