Make system menus show up at the right place for all frames, whether they're triggered by clicking the window icon or hitting alt-space. Make clicking the icon for windows without a non-client view (e.g. the Bookmark Manager on Aero Glass) bring up the system menu.

Clean up more dead code, verbose code, or unnecessary statics/members.

TEST=Right-click titlebars, hit alt-space, and left-click window icons (where present), for:
* Main window
* Dialogs, e.g. the bookmark manager
* Popups
* App windows
When right-clicking, a system menu should appear at the cursor.  For the other cases, a system menu should appear at the system native location (roughly just below the titlebar).
Review URL: http://codereview.chromium.org/20225

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

7 files changed, 42 insertions, 28 deletions

diff --git a/chrome/views/custom_frame_window.cc b/chrome/views/custom_frame_window.cc
index e46f624..e4a7ef6 100644
--- a/chrome/views/custom_frame_window.cc
+++ b/chrome/views/custom_frame_window.cc
@@ -230,11 +230,11 @@ class DefaultNonClientView : public NonClientView,
   explicit DefaultNonClientView(CustomFrameWindow* container);
   virtual ~DefaultNonClientView();
 
-  // Overridden from CustomFrameWindow::NonClientView:
+  // Overridden from views::NonClientView:
   virtual gfx::Rect CalculateClientAreaBounds(int width, int height) const;
   virtual gfx::Size CalculateWindowSizeForClientSize(int width,
                                                      int height) const;
-  virtual CPoint GetSystemMenuPoint() const;
+  virtual gfx::Point GetSystemMenuPoint() const;
   virtual int NonClientHitTest(const gfx::Point& point);
   virtual void GetWindowMask(const gfx::Size& size, gfx::Path* window_mask);
   virtual void EnableClose(bool enable);
@@ -440,12 +440,10 @@ gfx::Size DefaultNonClientView::CalculateWindowSizeForClientSize(
                    height + NonClientTopBorderHeight() + border_thickness);
 }
 
-CPoint DefaultNonClientView::GetSystemMenuPoint() const {
-  // TODO(pkasting): This is wrong; Windows native runs the menu at the bottom
-  // of the titlebar, not the bottom of the window icon.
-  CPoint system_menu_point(system_menu_button_->x(),
-      system_menu_button_->y() + system_menu_button_->height());
-  MapWindowPoints(container_->GetHWND(), HWND_DESKTOP, &system_menu_point, 1);
+gfx::Point DefaultNonClientView::GetSystemMenuPoint() const {
+  gfx::Point system_menu_point(FrameBorderThickness(),
+      NonClientTopBorderHeight() - BottomEdgeThicknessWithinNonClientHeight());
+  ConvertPointToScreen(this, &system_menu_point);
   return system_menu_point;
 }
 
diff --git a/chrome/views/non_client_view.cc b/chrome/views/non_client_view.cc
index 1d53d06..205f0cb 100644
--- a/chrome/views/non_client_view.cc
+++ b/chrome/views/non_client_view.cc
@@ -6,6 +6,7 @@
 
 namespace views {
 
+const int NonClientView::kFrameShadowThickness = 1;
 const int NonClientView::kClientEdgeThickness = 1;
 
 int NonClientView::GetHTComponentForFrame(const gfx::Point& point,
diff --git a/chrome/views/non_client_view.h b/chrome/views/non_client_view.h
index 29cbda3..2900f98 100644
--- a/chrome/views/non_client_view.h
+++ b/chrome/views/non_client_view.h
@@ -27,6 +27,9 @@ class ClientView;
 //
 class NonClientView : public View {
  public:
+  // Various edges of the frame border have a 1 px shadow along their edges; in
+  // a few cases we shift elements based on this amount for visual appeal.
+  static const int kFrameShadowThickness;
   // In restored mode, we draw a 1 px edge around the content area inside the
   // frame border.
   static const int kClientEdgeThickness;
@@ -43,7 +46,7 @@ class NonClientView : public View {
 
   // Returns the point, in screen coordinates, where the system menu should
   // be shown so it shows up anchored to the system menu icon.
-  virtual CPoint GetSystemMenuPoint() const = 0;
+  virtual gfx::Point GetSystemMenuPoint() const = 0;
 
   // Determines the windows HT* code when the mouse cursor is at the
   // specified point, in window coordinates.
diff --git a/chrome/views/view.cc b/chrome/views/view.cc
index 59e5b73..8d1926f 100644
--- a/chrome/views/view.cc
+++ b/chrome/views/view.cc
@@ -1268,13 +1268,12 @@ void View::ConvertPointToView(const View* src,
 }
 
 // static
-void View::ConvertPointToWidget(View* src, gfx::Point* p) {
+void View::ConvertPointToWidget(const View* src, gfx::Point* p) {
   DCHECK(src);
   DCHECK(p);
 
-  View *v;
   gfx::Point offset;
-  for (v = src; v; v = v->GetParent()) {
+  for (const View* v = src; v; v = v->GetParent()) {
     offset.set_x(offset.x() + v->GetX(APPLY_MIRRORING_TRANSFORMATION));
     offset.set_y(offset.y() + v->y());
   }
@@ -1282,14 +1281,14 @@ void View::ConvertPointToWidget(View* src, gfx::Point* p) {
 }
 
 // static
-void View::ConvertPointFromWidget(View *source, gfx::Point* p) {
+void View::ConvertPointFromWidget(const View* dest, gfx::Point* p) {
   gfx::Point t;
-  ConvertPointToWidget(source, &t);
+  ConvertPointToWidget(dest, &t);
   p->SetPoint(p->x() - t.x(), p->y() - t.y());
 }
 
 // static
-void View::ConvertPointToScreen(View* src, gfx::Point* p) {
+void View::ConvertPointToScreen(const View* src, gfx::Point* p) {
   DCHECK(src);
   DCHECK(p);
 
diff --git a/chrome/views/view.h b/chrome/views/view.h
index 8651183..aee08a3 100644
--- a/chrome/views/view.h
+++ b/chrome/views/view.h
@@ -684,14 +684,14 @@ class View : public AcceleratorTarget {
   // Widget. This is useful for example when sizing HWND children of the
   // Widget that don't know about the View hierarchy and need to be placed
   // relative to the Widget that is their parent.
-  static void ConvertPointToWidget(View* src, gfx::Point* point);
+  static void ConvertPointToWidget(const View* src, gfx::Point* point);
 
   // Convert a point from a view Widget to a View dest
-  static void ConvertPointFromWidget(View *dest, gfx::Point* p);
+  static void ConvertPointFromWidget(const View* dest, gfx::Point* p);
 
   // Convert a point from the coordinate system of a View to that of the
   // screen. This is useful for example when placing popup windows.
-  static void ConvertPointToScreen(View* src, gfx::Point* point);
+  static void ConvertPointToScreen(const View* src, gfx::Point* point);
 
   // Event Handlers
 
diff --git a/chrome/views/window.cc b/chrome/views/window.cc
index e690a04..84662a9 100644
--- a/chrome/views/window.cc
+++ b/chrome/views/window.cc
@@ -326,7 +326,7 @@ void Window::SizeWindowToDefault() {
                                 true);
 }
 
-void Window::RunSystemMenu(const CPoint& point) {
+void Window::RunSystemMenu(const gfx::Point& point) {
   // We need to reset and clean up any currently created system menu objects.
   // We need to call this otherwise there's a small chance that we aren't going
   // to get a system menu. We also can't take the return value of this
@@ -335,7 +335,7 @@ void Window::RunSystemMenu(const CPoint& point) {
   HMENU system_menu = ::GetSystemMenu(GetHWND(), FALSE);
   int id = ::TrackPopupMenu(system_menu,
                             TPM_LEFTBUTTON | TPM_RIGHTBUTTON | TPM_RETURNCMD,
-                            point.x, point.y, 0, GetHWND(), NULL);
+                            point.x(), point.y(), 0, GetHWND(), NULL);
   ExecuteSystemMenuCommand(id);
 }
 
@@ -406,17 +406,26 @@ LRESULT Window::OnNCHitTest(const CPoint& point) {
 }
 
 void Window::OnNCLButtonDown(UINT ht_component, const CPoint& point) {
-  if (non_client_view_ && ht_component == HTSYSMENU)
-    RunSystemMenu(non_client_view_->GetSystemMenuPoint());
-  WidgetWin::OnNCLButtonDown(ht_component, point);
+  if (ht_component == HTSYSMENU) {
+    gfx::Point system_menu_point;
+    if (non_client_view_) {
+      system_menu_point = non_client_view_->GetSystemMenuPoint();
+    } else {
+      CPoint temp(0, -NonClientView::kFrameShadowThickness);
+      MapWindowPoints(GetHWND(), HWND_DESKTOP, &temp, 1);
+      system_menu_point = gfx::Point(temp);
+    }
+    RunSystemMenu(system_menu_point);
+  } else {
+    WidgetWin::OnNCLButtonDown(ht_component, point);
+  }
 }
 
 void Window::OnNCRButtonDown(UINT ht_component, const CPoint& point) {
-  if (ht_component == HTCAPTION || ht_component == HTSYSMENU) {
-    RunSystemMenu(point);
-  } else {
+  if (ht_component == HTCAPTION || ht_component == HTSYSMENU)
+    RunSystemMenu(gfx::Point(point));
+  else
     WidgetWin::OnNCRButtonDown(ht_component, point);
-  }
 }
 
 
@@ -464,6 +473,10 @@ void Window::OnSysCommand(UINT notification_code, CPoint click) {
 
     // Now change the actual window's behavior.
     AlwaysOnTopChanged();
+  } else if ((notification_code == SC_KEYMENU) && (click.x == VK_SPACE) &&
+             non_client_view_) {
+    // Run the system menu at the NonClientView's desired location.
+    RunSystemMenu(non_client_view_->GetSystemMenuPoint());
   } else {
     // Use the default implementation for any other command.
     DefWindowProc(GetHWND(), WM_SYSCOMMAND, notification_code,
diff --git a/chrome/views/window.h b/chrome/views/window.h
index 69316e4..38fe2bd 100644
--- a/chrome/views/window.h
+++ b/chrome/views/window.h
@@ -155,7 +155,7 @@ class Window : public WidgetWin,
   void set_client_view(ClientView* client_view) { client_view_ = client_view; }
 
   // Shows the system menu at the specified screen point.
-  void RunSystemMenu(const CPoint& point);
+  void RunSystemMenu(const gfx::Point& point);
 
   // Overridden from WidgetWin:
   virtual void OnActivate(UINT action, BOOL minimized, HWND window);