Changes tree/table to pass in an appropriate location when the context

menu is invoked from the keyboard.

BUG=4029
TEST=In the bookmark manager bring up the context menu of the
  tree/table via the menu key and make sure the menu appears at a
  reasonable location.

Review URL: http://codereview.chromium.org/10256

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5125 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 95 insertions, 47 deletions

diff --git a/chrome/views/native_control.cc b/chrome/views/native_control.cc
index 04a23fd..c2816be 100644
--- a/chrome/views/native_control.cc
+++ b/chrome/views/native_control.cc
@@ -258,8 +258,16 @@ void NativeControl::OnContextMenu(const CPoint& location) {
   if (!GetContextMenuController())
     return;
 
-  GetContextMenuController()->ShowContextMenu(
-      this, location.x, location.y, true);
+  int x = location.x;
+  int y = location.y;
+  bool is_mouse = true;
+  if (x == -1 && y == -1) {
+    gfx::Point point = GetKeyboardContextMenuLocation();
+    x = point.x();
+    y = point.y();
+    is_mouse = false;
+  }
+  ShowContextMenu(x, y, is_mouse);
 }
 
 void NativeControl::Focus() {
diff --git a/chrome/views/root_view.cc b/chrome/views/root_view.cc
index aa8f1b3..4b4027d 100644
--- a/chrome/views/root_view.cc
+++ b/chrome/views/root_view.cc
@@ -805,8 +805,8 @@ void RootView::ProcessKeyEvent(const KeyEvent& event) {
         (event.GetCharacter() == VK_F10 && event.IsShiftDown())) {
       v = GetFocusedView();
       if (v->IsEnabled()) {
-        v->ShowContextMenu(v->x() + (v->width() / 2),
-                           v->y() + (v->height() / 2), false);
+        gfx::Point screen_loc = v->GetKeyboardContextMenuLocation();
+        v->ShowContextMenu(screen_loc.x(), screen_loc.y(), false);
         return;
       }
     }
diff --git a/chrome/views/table_view.cc b/chrome/views/table_view.cc
index a94b9a3..7a00723 100644
--- a/chrome/views/table_view.cc
+++ b/chrome/views/table_view.cc
@@ -504,6 +504,24 @@ bool TableView::HasColumn(int id) {
   return all_columns_.count(id) > 0;
 }
 
+gfx::Point TableView::GetKeyboardContextMenuLocation() {
+  int first_selected = FirstSelectedRow();
+  int y = height() / 2;
+  if (first_selected != -1) {
+    RECT cell_bounds;
+    RECT client_rect;
+    if (ListView_GetItemRect(GetNativeControlHWND(), first_selected,
+                             &cell_bounds, LVIR_BOUNDS) &&
+        GetClientRect(GetNativeControlHWND(), &client_rect) &&
+        cell_bounds.bottom >= 0 && cell_bounds.bottom < client_rect.bottom) {
+      y = cell_bounds.bottom;
+    }
+  }
+  gfx::Point screen_loc(0, y);
+  ConvertPointToScreen(this, &screen_loc);
+  return screen_loc;
+}
+
 void TableView::SetCustomColorsEnabled(bool custom_colors_enabled) {
   custom_colors_enabled_ = custom_colors_enabled;
 }
diff --git a/chrome/views/table_view.h b/chrome/views/table_view.h
index 46e40cb6a..e0e4ad4 100644
--- a/chrome/views/table_view.h
+++ b/chrome/views/table_view.h
@@ -428,6 +428,9 @@ class TableView : public NativeControl,
   }
 
  protected:
+  // Overriden to return the position of the first selected row.
+  virtual gfx::Point GetKeyboardContextMenuLocation();
+
   // Subclasses that want to customize the colors of a particular row/column,
   // must invoke this passing in true. The default value is false, such that
   // GetCellColors is never invoked.
diff --git a/chrome/views/tree_view.cc b/chrome/views/tree_view.cc
index c62b6e7..2a11c85 100644
--- a/chrome/views/tree_view.cc
+++ b/chrome/views/tree_view.cc
@@ -284,6 +284,24 @@ void TreeView::TreeNodeChanged(TreeModel* model, TreeModelNode* node) {
   TreeView_SetItem(tree_view_, &tv_item);
 }
 
+gfx::Point TreeView::GetKeyboardContextMenuLocation() {
+  int y = height() / 2;
+  if (GetSelectedNode()) {
+    RECT bounds;
+    RECT client_rect;
+    if (TreeView_GetItemRect(tree_view_,
+                             GetNodeDetails(GetSelectedNode())->tree_item,
+                             &bounds, TRUE) &&
+        GetClientRect(tree_view_, &client_rect) &&
+        bounds.bottom >= 0 && bounds.bottom < client_rect.bottom) {
+      y = bounds.bottom;
+    }
+  }
+  gfx::Point screen_loc(0, y);
+  ConvertPointToScreen(this, &screen_loc);
+  return screen_loc;
+}
+
 HWND TreeView::CreateNativeControl(HWND parent_container) {
   int style = WS_CHILD | TVS_HASBUTTONS | TVS_HASLINES | TVS_SHOWSELALWAYS;
   if (!drag_enabled_)
@@ -433,51 +451,34 @@ bool TreeView::OnKeyDown(int virtual_key_code) {
 }
 
 void TreeView::OnContextMenu(const CPoint& location) {
-  if (GetContextMenuController()) {
-    if (location.x == -1 && location.y == -1) {
-      // Indicates the user pressed the context menu key.
-      bool valid_loc = false;
-      int x, y;
-      if (GetSelectedNode()) {
-        RECT bounds;
-        if (TreeView_GetItemRect(tree_view_,
-                                 GetNodeDetails(GetSelectedNode())->tree_item,
-                                 &bounds, TRUE)) {
-          x = bounds.left;
-          y = bounds.top + (bounds.bottom - bounds.top) / 2;
-          valid_loc = true;
-        }
-      } else if (show_context_menu_only_when_node_selected_) {
-        return;
-      }
-      if (!valid_loc) {
-        x = width() / 2;
-        y = height() / 2;
-      }
-      gfx::Point screen_loc(x, y);
-      ConvertPointToScreen(this, &screen_loc);
-      GetContextMenuController()->ShowContextMenu(this, screen_loc.x(),
-                                                  screen_loc.y(), false);
-    } else if (!show_context_menu_only_when_node_selected_) {
-      GetContextMenuController()->ShowContextMenu(this, location.x, location.y,
-                                                  true);
-    } else if (GetSelectedNode()) {
-      // Make sure the mouse is over the selected node.
-      TVHITTESTINFO hit_info;
-      gfx::Point local_loc(location);
-      ConvertPointToView(NULL, this, &local_loc);
-      hit_info.pt.x = local_loc.x();
-      hit_info.pt.y = local_loc.y();
-      HTREEITEM hit_item = TreeView_HitTest(tree_view_, &hit_info);
-      if (hit_item &&
-          GetNodeDetails(GetSelectedNode())->tree_item == hit_item &&
-          (hit_info.flags & (TVHT_ONITEM | TVHT_ONITEMRIGHT |
-                            TVHT_ONITEMINDENT)) != 0) {
-        GetContextMenuController()->ShowContextMenu(this, location.x,
-                                                    location.y, true);
-      }
+  if (!GetContextMenuController())
+    return;
+
+  if (location.x == -1 && location.y == -1) {
+    // Let NativeControl's implementation handle keyboard gesture.
+    NativeControl::OnContextMenu(location);
+    return;
+  }
+  
+  if (show_context_menu_only_when_node_selected_) {
+    if (!GetSelectedNode())
+      return;
+
+    // Make sure the mouse is over the selected node.
+    TVHITTESTINFO hit_info;
+    gfx::Point local_loc(location);
+    ConvertPointToView(NULL, this, &local_loc);
+    hit_info.pt.x = local_loc.x();
+    hit_info.pt.y = local_loc.y();
+    HTREEITEM hit_item = TreeView_HitTest(tree_view_, &hit_info);
+    if (!hit_item ||
+        GetNodeDetails(GetSelectedNode())->tree_item != hit_item ||
+        (hit_info.flags & (TVHT_ONITEM | TVHT_ONITEMRIGHT |
+                          TVHT_ONITEMINDENT)) == 0) {
+      return;
     }
   }
+  ShowContextMenu(location.x, location.y, true);
 }
 
 TreeModelNode* TreeView::GetNodeForTreeItem(HTREEITEM tree_item) {
diff --git a/chrome/views/tree_view.h b/chrome/views/tree_view.h
index a385c60..c2fe4d3 100644
--- a/chrome/views/tree_view.h
+++ b/chrome/views/tree_view.h
@@ -208,6 +208,8 @@ class TreeView : public NativeControl, TreeModelObserver {
   bool GetSelectOnRightMouseDown() { return select_on_right_mouse_down_; }
 
  protected:
+  // Overriden to return a location based on the selected node.
+  virtual gfx::Point GetKeyboardContextMenuLocation();
 
   // Creates and configures the tree_view.
   virtual HWND CreateNativeControl(HWND parent_container);
diff --git a/chrome/views/view.cc b/chrome/views/view.cc
index 1dbe9f8..9925ff4 100644
--- a/chrome/views/view.cc
+++ b/chrome/views/view.cc
@@ -680,6 +680,14 @@ bool View::IsProcessingPaint() const {
 }
 #endif
 
+gfx::Point View::GetKeyboardContextMenuLocation() {
+  gfx::Rect vis_bounds = GetVisibleBounds();
+  gfx::Point screen_point(vis_bounds.x() + vis_bounds.width() / 2,
+                          vis_bounds.y() + vis_bounds.height() / 2);
+  ConvertPointToScreen(this, &screen_point);
+  return screen_point;
+}
+
 bool View::HasHitTestMask() const {
   return false;
 }
diff --git a/chrome/views/view.h b/chrome/views/view.h
index d8bf716..b98951f 100644
--- a/chrome/views/view.h
+++ b/chrome/views/view.h
@@ -989,6 +989,14 @@ class View : public AcceleratorTarget {
   virtual bool IsProcessingPaint() const;
 #endif
 
+  // Returns the location, in screen coordinates, to show the context menu at
+  // when the context menu is shown from the keyboard. This implementation
+  // returns the middle of the visible region of this view.
+  //
+  // This method is invoked when the context menu is shown by way of the
+  // keyboard.
+  virtual gfx::Point GetKeyboardContextMenuLocation();
+
   // Called by HitTest to see if this View has a custom hit test mask. If the
   // return value is true, GetHitTestMask will be called to obtain the mask.
   // Default value is false, in which case the View will hit-test against its