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

ui/views/view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
@@ skipping 39 matching lines @@
 namespace {
 
 // Whether to use accelerated compositing when necessary (e.g. when a view has a
 // transformation).
 #if defined(USE_AURA)
 bool use_acceleration_when_possible = true;
 #else
 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:
   explicit ScopedCanvas(gfx::Canvas* canvas) : canvas_(canvas) {
     if (canvas_)
       canvas_->Save();
   }
   ~ScopedCanvas() {
     if (canvas_)
       canvas_->Restore();
@@ skipping 1969 matching lines @@
   ContextMenuController* context_menu_controller = event.IsRightMouseButton() ?
       context_menu_controller_ : 0;
 
   const bool enabled = enabled_;
   const bool result = OnMousePressed(event);
   // WARNING: we may have been deleted, don't use any View variables.
 
   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;
   }
   return !!context_menu_controller || result;
 }
 
 bool View::ProcessMouseDragged(const ui::MouseEvent& event) {
   // Copy the field, that way if we're deleted after drag and drop no harm is
   // done.
@@ skipping 10 matching lines @@
   } else {
     if (OnMouseDragged(event))
       return true;
     // Fall through to return value based on context menu controller.
   }
   // WARNING: we may have been deleted.
   return (context_menu_controller != NULL) || possible_drag;
 }
 
 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());
     OnMouseReleased(event);
     if (HitTestPoint(location)) {
       ConvertPointToScreen(this, &location);
       ShowContextMenu(location, true);
     }
   } else {
     OnMouseReleased(event);
@@ skipping 140 matching lines @@
   ConvertPointToWidget(this, &widget_location);
   GetWidget()->RunShellDrag(this, data, widget_location, drag_operations,
       source);
   return true;
 #else
   return false;
 #endif  // !defined(OS_MACOSX)
 }
 
 }  // namespace views