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

12 files changed, 141 insertions, 155 deletions

diff --git a/chrome/browser/views/constrained_window_impl.cc b/chrome/browser/views/constrained_window_impl.cc
index 3c1d78f..207ee31 100644
--- a/chrome/browser/views/constrained_window_impl.cc
+++ b/chrome/browser/views/constrained_window_impl.cc
@@ -201,7 +201,7 @@ class ConstrainedWindowNonClientView
   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);
@@ -353,10 +353,13 @@ gfx::Size ConstrainedWindowNonClientView::CalculateWindowSizeForClientSize(
                    height + NonClientTopBorderHeight() + border_thickness);
 }
 
-CPoint ConstrainedWindowNonClientView::GetSystemMenuPoint() const {
-  // Doesn't matter what we return, since this is only used when the user clicks
-  // a window icon, and we never have an icon.
-  return CPoint();
+gfx::Point ConstrainedWindowNonClientView::GetSystemMenuPoint() const {
+  // Doesn't really matter, since we never show system menus on constrained
+  // windows...
+  gfx::Point system_menu_point(FrameBorderThickness(),
+                               NonClientTopBorderHeight());
+  ConvertPointToScreen(this, &system_menu_point);
+  return system_menu_point;
 }
 
 int ConstrainedWindowNonClientView::NonClientHitTest(const gfx::Point& point) {
diff --git a/chrome/browser/views/frame/aero_glass_non_client_view.cc b/chrome/browser/views/frame/aero_glass_non_client_view.cc
index 077fb31..ed37f7a 100644
--- a/chrome/browser/views/frame/aero_glass_non_client_view.cc
+++ b/chrome/browser/views/frame/aero_glass_non_client_view.cc
@@ -13,8 +13,6 @@
 
 // An enumeration of bitmap resources used by this window.
 enum {
-  FRAME_PART_BITMAP_FIRST = 0,  // must be first.
-
   // Client Edge Border.
   FRAME_CLIENT_EDGE_TOP_LEFT,
   FRAME_CLIENT_EDGE_TOP,
@@ -30,60 +28,44 @@ enum {
 
 class AeroGlassWindowResources {
  public:
-  AeroGlassWindowResources() { InitClass(); }
+  AeroGlassWindowResources() {
+    InitClass();
+  }
   virtual ~AeroGlassWindowResources() { }
 
   virtual SkBitmap* GetPartBitmap(views::FramePartBitmap part) const {
     return standard_frame_bitmaps_[part];
   }
 
-  SkBitmap app_top_left() const { return app_top_left_; }
-  SkBitmap app_top_center() const { return app_top_center_; }
-  SkBitmap app_top_right() const { return app_top_right_; }
-
  private:
   static void InitClass() {
     static bool initialized = false;
     if (!initialized) {
       static const int kFramePartBitmapIds[] = {
-        0,
         IDR_CONTENT_TOP_LEFT_CORNER, IDR_CONTENT_TOP_CENTER,
             IDR_CONTENT_TOP_RIGHT_CORNER, IDR_CONTENT_RIGHT_SIDE,
             IDR_CONTENT_BOTTOM_RIGHT_CORNER, IDR_CONTENT_BOTTOM_CENTER,
             IDR_CONTENT_BOTTOM_LEFT_CORNER, IDR_CONTENT_LEFT_SIDE,
-        0
       };
 
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i) {
-        int id = kFramePartBitmapIds[i];
-        if (id != 0)
-          standard_frame_bitmaps_[i] = rb.GetBitmapNamed(id);
-      }
-      app_top_left_ = *rb.GetBitmapNamed(IDR_APP_TOP_LEFT);
-      app_top_center_ = *rb.GetBitmapNamed(IDR_APP_TOP_CENTER);
-      app_top_right_ = *rb.GetBitmapNamed(IDR_APP_TOP_RIGHT);      
+      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i)
+        standard_frame_bitmaps_[i] = rb.GetBitmapNamed(kFramePartBitmapIds[i]);
 
       initialized = true;
     }
   }
 
   static SkBitmap* standard_frame_bitmaps_[FRAME_PART_BITMAP_COUNT];
-  static SkBitmap app_top_left_;
-  static SkBitmap app_top_center_;
-  static SkBitmap app_top_right_;
 
   DISALLOW_EVIL_CONSTRUCTORS(AeroGlassWindowResources);
 };
 
 // static
 SkBitmap* AeroGlassWindowResources::standard_frame_bitmaps_[];
-SkBitmap AeroGlassWindowResources::app_top_left_;
-SkBitmap AeroGlassWindowResources::app_top_center_;
-SkBitmap AeroGlassWindowResources::app_top_right_;
 
 AeroGlassWindowResources* AeroGlassNonClientView::resources_ = NULL;
-SkBitmap AeroGlassNonClientView::distributor_logo_;
+SkBitmap* AeroGlassNonClientView::distributor_logo_ = NULL;
 
 namespace {
 // There are 3 px of client edge drawn inside the outer frame borders.
@@ -166,10 +148,17 @@ gfx::Size AeroGlassNonClientView::CalculateWindowSizeForClientSize(
                    height + NonClientTopBorderHeight() + border_thickness);
 }
 
-CPoint AeroGlassNonClientView::GetSystemMenuPoint() const {
-  CPoint offset;
-  MapWindowPoints(GetWidget()->GetHWND(), HWND_DESKTOP, &offset, 1);
-  return offset;
+gfx::Point AeroGlassNonClientView::GetSystemMenuPoint() const {
+  gfx::Point system_menu_point;
+  if (browser_view_->IsTabStripVisible()) {
+    system_menu_point.SetPoint(
+        NonClientBorderThickness() - kClientEdgeThickness,
+        NonClientTopBorderHeight() + browser_view_->GetTabStripHeight());
+  } else {
+    system_menu_point.SetPoint(0, -kFrameShadowThickness);
+  }
+  ConvertPointToScreen(this, &system_menu_point);
+  return system_menu_point;
 }
 
 int AeroGlassNonClientView::NonClientHitTest(const gfx::Point& point) {
@@ -239,11 +228,11 @@ int AeroGlassNonClientView::NonClientTopBorderHeight() const {
 void AeroGlassNonClientView::PaintDistributorLogo(ChromeCanvas* canvas) {
   // The distributor logo is only painted when the frame is not maximized and
   // when we actually have a logo.
-  if (!frame_->IsMaximized() && !distributor_logo_.empty()) {
+  if (!frame_->IsMaximized() && distributor_logo_) {
     // NOTE: We don't mirror the logo placement here because the outer frame
     // itself isn't mirrored in RTL.  This is a bug; if it is fixed, this should
     // be mirrored as in opaque_non_client_view.cc.
-    canvas->DrawBitmapInt(distributor_logo_, logo_bounds_.x(),
+    canvas->DrawBitmapInt(*distributor_logo_, logo_bounds_.x(),
                           logo_bounds_.y());
   }
 }
@@ -324,10 +313,14 @@ void AeroGlassNonClientView::PaintClientEdge(ChromeCanvas* canvas) {
 }
 
 void AeroGlassNonClientView::LayoutDistributorLogo() {
-  int logo_x = frame_->GetMinimizeButtonOffset() - (distributor_logo_.empty() ?
-      0 : (distributor_logo_.width() + kLogoCaptionSpacing));
-  logo_bounds_.SetRect(logo_x, kLogoTopSpacing, distributor_logo_.width(),
-                       distributor_logo_.height());
+  if (distributor_logo_) {
+    logo_bounds_.SetRect(frame_->GetMinimizeButtonOffset() -
+        distributor_logo_->width() - kLogoCaptionSpacing, kLogoTopSpacing,
+        distributor_logo_->width(), distributor_logo_->height());
+  } else {
+    logo_bounds_.SetRect(frame_->GetMinimizeButtonOffset(), kLogoTopSpacing, 0,
+                         0);
+  }
 }
 
 void AeroGlassNonClientView::LayoutOTRAvatar() {
@@ -353,9 +346,9 @@ void AeroGlassNonClientView::InitClass() {
   if (!initialized) {
     resources_ = new AeroGlassWindowResources;
 
-    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 #if defined(GOOGLE_CHROME_BUILD)
-    distributor_logo_ = *rb.GetBitmapNamed(IDR_DISTRIBUTOR_LOGO);
+    distributor_logo_ = ResourceBundle::GetSharedInstance().
+        GetBitmapNamed(IDR_DISTRIBUTOR_LOGO);
 #endif
 
     initialized = true;
diff --git a/chrome/browser/views/frame/aero_glass_non_client_view.h b/chrome/browser/views/frame/aero_glass_non_client_view.h
index 68ef83c..5023ea0 100644
--- a/chrome/browser/views/frame/aero_glass_non_client_view.h
+++ b/chrome/browser/views/frame/aero_glass_non_client_view.h
@@ -27,7 +27,7 @@ class AeroGlassNonClientView : public 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) { }
@@ -67,17 +67,17 @@ class AeroGlassNonClientView : public views::NonClientView {
   // The layout rect of the distributor logo, if visible.
   gfx::Rect logo_bounds_;
 
-  // The layout rect of the OTR avatar.
+  // The layout rect of the OTR avatar icon, if visible.
   gfx::Rect otr_avatar_bounds_;
 
   // The frame that hosts this view.
   AeroGlassFrame* frame_;
 
-  // The BrowserView that we contain.
+  // The BrowserView hosted within this View.
   BrowserView* browser_view_;
 
   static void InitClass();
-  static SkBitmap distributor_logo_;
+  static SkBitmap* distributor_logo_;
   static AeroGlassWindowResources* resources_;
 
   DISALLOW_EVIL_CONSTRUCTORS(AeroGlassNonClientView);
diff --git a/chrome/browser/views/frame/opaque_non_client_view.cc b/chrome/browser/views/frame/opaque_non_client_view.cc
index e007a83..af3a77d 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.cc
+++ b/chrome/browser/views/frame/opaque_non_client_view.cc
@@ -19,8 +19,6 @@
 
 // An enumeration of bitmap resources used by this window.
 enum {
-  FRAME_PART_BITMAP_FIRST = 0,  // Must be first.
-
   // Window Controls.
   FRAME_CLOSE_BUTTON_ICON,
   FRAME_CLOSE_BUTTON_ICON_H,
@@ -58,6 +56,11 @@ enum {
   FRAME_CLIENT_EDGE_BOTTOM_LEFT,
   FRAME_CLIENT_EDGE_LEFT,
 
+  // No-toolbar client edge.
+  FRAME_NO_TOOLBAR_TOP_LEFT,
+  FRAME_NO_TOOLBAR_TOP_CENTER,
+  FRAME_NO_TOOLBAR_TOP_RIGHT,
+
   // Popup-mode toolbar edges.
   FRAME_TOOLBAR_POPUP_EDGE_LEFT,
   FRAME_TOOLBAR_POPUP_EDGE_RIGHT,
@@ -82,7 +85,6 @@ class ActiveWindowResources : public views::WindowResources {
     static bool initialized = false;
     if (!initialized) {
       static const int kFramePartBitmapIds[] = {
-        0,
         IDR_CLOSE, IDR_CLOSE_H, IDR_CLOSE_P,
         IDR_CLOSE_SA, IDR_CLOSE_SA_H, IDR_CLOSE_SA_P,
         IDR_RESTORE, IDR_RESTORE_H, IDR_RESTORE_P,
@@ -96,22 +98,18 @@ class ActiveWindowResources : public views::WindowResources {
             IDR_CONTENT_TOP_RIGHT_CORNER, IDR_CONTENT_RIGHT_SIDE,
             IDR_CONTENT_BOTTOM_RIGHT_CORNER, IDR_CONTENT_BOTTOM_CENTER,
             IDR_CONTENT_BOTTOM_LEFT_CORNER, IDR_CONTENT_LEFT_SIDE,
+        IDR_APP_TOP_LEFT, IDR_APP_TOP_CENTER, IDR_APP_TOP_RIGHT,
         IDR_LOCATIONBG_POPUPMODE_LEFT, IDR_LOCATIONBG_POPUPMODE_RIGHT,
-        0
       };
 
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i) {
-        int id = kFramePartBitmapIds[i];
-        if (id != 0)
-          standard_frame_bitmaps_[i] = rb.GetBitmapNamed(id);
-      }
+      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i)
+        standard_frame_bitmaps_[i] = rb.GetBitmapNamed(kFramePartBitmapIds[i]);
       initialized = true;
     }
   }
 
   static SkBitmap* standard_frame_bitmaps_[FRAME_PART_BITMAP_COUNT];
-  static ChromeFont title_font_;
 
   DISALLOW_EVIL_CONSTRUCTORS(ActiveWindowResources);
 };
@@ -133,7 +131,6 @@ class InactiveWindowResources : public views::WindowResources {
     static bool initialized = false;
     if (!initialized) {
       static const int kFramePartBitmapIds[] = {
-        0,
         IDR_CLOSE, IDR_CLOSE_H, IDR_CLOSE_P,
         IDR_CLOSE_SA, IDR_CLOSE_SA_H, IDR_CLOSE_SA_P,
         IDR_RESTORE, IDR_RESTORE_H, IDR_RESTORE_P,
@@ -147,22 +144,18 @@ class InactiveWindowResources : public views::WindowResources {
             IDR_CONTENT_TOP_RIGHT_CORNER, IDR_CONTENT_RIGHT_SIDE,
             IDR_CONTENT_BOTTOM_RIGHT_CORNER, IDR_CONTENT_BOTTOM_CENTER,
             IDR_CONTENT_BOTTOM_LEFT_CORNER, IDR_CONTENT_LEFT_SIDE,
+        IDR_APP_TOP_LEFT, IDR_APP_TOP_CENTER, IDR_APP_TOP_RIGHT,
         IDR_LOCATIONBG_POPUPMODE_LEFT, IDR_LOCATIONBG_POPUPMODE_RIGHT,
-        0
       };
 
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i) {
-        int id = kFramePartBitmapIds[i];
-        if (id != 0)
-          standard_frame_bitmaps_[i] = rb.GetBitmapNamed(id);
-      }
+      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i)
+        standard_frame_bitmaps_[i] = rb.GetBitmapNamed(kFramePartBitmapIds[i]);
       initialized = true;
     }
   }
 
   static SkBitmap* standard_frame_bitmaps_[FRAME_PART_BITMAP_COUNT];
-  static ChromeFont title_font_;
 
   DISALLOW_EVIL_CONSTRUCTORS(InactiveWindowResources);
 };
@@ -184,7 +177,6 @@ class OTRActiveWindowResources : public views::WindowResources {
     static bool initialized = false;
     if (!initialized) {
       static const int kFramePartBitmapIds[] = {
-        0,
         IDR_CLOSE, IDR_CLOSE_H, IDR_CLOSE_P,
         IDR_CLOSE_SA, IDR_CLOSE_SA_H, IDR_CLOSE_SA_P,
         IDR_RESTORE, IDR_RESTORE_H, IDR_RESTORE_P,
@@ -198,16 +190,13 @@ class OTRActiveWindowResources : public views::WindowResources {
             IDR_CONTENT_TOP_RIGHT_CORNER, IDR_CONTENT_RIGHT_SIDE,
             IDR_CONTENT_BOTTOM_RIGHT_CORNER, IDR_CONTENT_BOTTOM_CENTER,
             IDR_CONTENT_BOTTOM_LEFT_CORNER, IDR_CONTENT_LEFT_SIDE,
+        IDR_APP_TOP_LEFT, IDR_APP_TOP_CENTER, IDR_APP_TOP_RIGHT,
         IDR_LOCATIONBG_POPUPMODE_LEFT, IDR_LOCATIONBG_POPUPMODE_RIGHT,
-        0
       };
 
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i) {
-        int id = kFramePartBitmapIds[i];
-        if (id != 0)
-          standard_frame_bitmaps_[i] = rb.GetBitmapNamed(id);
-      }
+      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i)
+        standard_frame_bitmaps_[i] = rb.GetBitmapNamed(kFramePartBitmapIds[i]);
       initialized = true;
     }
   }
@@ -234,7 +223,6 @@ class OTRInactiveWindowResources : public views::WindowResources {
     static bool initialized = false;
     if (!initialized) {
       static const int kFramePartBitmapIds[] = {
-        0,
         IDR_CLOSE, IDR_CLOSE_H, IDR_CLOSE_P,
         IDR_CLOSE_SA, IDR_CLOSE_SA_H, IDR_CLOSE_SA_P,
         IDR_RESTORE, IDR_RESTORE_H, IDR_RESTORE_P,
@@ -249,22 +237,18 @@ class OTRInactiveWindowResources : public views::WindowResources {
             IDR_CONTENT_TOP_RIGHT_CORNER, IDR_CONTENT_RIGHT_SIDE,
             IDR_CONTENT_BOTTOM_RIGHT_CORNER, IDR_CONTENT_BOTTOM_CENTER,
             IDR_CONTENT_BOTTOM_LEFT_CORNER, IDR_CONTENT_LEFT_SIDE,
+        IDR_APP_TOP_LEFT, IDR_APP_TOP_CENTER, IDR_APP_TOP_RIGHT,
         IDR_LOCATIONBG_POPUPMODE_LEFT, IDR_LOCATIONBG_POPUPMODE_RIGHT,
-        0
       };
 
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i) {
-        int id = kFramePartBitmapIds[i];
-        if (id != 0)
-          standard_frame_bitmaps_[i] = rb.GetBitmapNamed(id);
-      }
+      for (int i = 0; i < FRAME_PART_BITMAP_COUNT; ++i)
+        standard_frame_bitmaps_[i] = rb.GetBitmapNamed(kFramePartBitmapIds[i]);
       initialized = true;
     }
   }
 
   static SkBitmap* standard_frame_bitmaps_[FRAME_PART_BITMAP_COUNT];
-  static ChromeFont title_font_;
 
   DISALLOW_EVIL_CONSTRUCTORS(OTRInactiveWindowResources);
 };
@@ -279,19 +263,13 @@ views::WindowResources* OpaqueNonClientView::active_resources_ = NULL;
 views::WindowResources* OpaqueNonClientView::inactive_resources_ = NULL;
 views::WindowResources* OpaqueNonClientView::active_otr_resources_ = NULL;
 views::WindowResources* OpaqueNonClientView::inactive_otr_resources_ = NULL;
-SkBitmap OpaqueNonClientView::distributor_logo_;
-SkBitmap OpaqueNonClientView::app_top_left_;
-SkBitmap OpaqueNonClientView::app_top_center_;
-SkBitmap OpaqueNonClientView::app_top_right_;
+SkBitmap* OpaqueNonClientView::distributor_logo_ = NULL;
 ChromeFont OpaqueNonClientView::title_font_;
 
 namespace {
 // The frame border is only visible in restored mode and is hardcoded to 4 px on
 // each side regardless of the system window border size.
 const int kFrameBorderThickness = 4;
-// 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.
-const int kFrameShadowThickness = 1;
 // Besides the frame border, there's another 11 px of empty space atop the
 // window in restored mode, to use to drag the window around.
 const int kNonClientRestoredExtraThickness = 11;
@@ -499,11 +477,13 @@ gfx::Size OpaqueNonClientView::CalculateWindowSizeForClientSize(
                    height + NonClientTopBorderHeight() + border_thickness);
 }
 
-CPoint OpaqueNonClientView::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(icon_bounds_.x(), icon_bounds_.bottom());
-  MapWindowPoints(frame_->GetHWND(), HWND_DESKTOP, &system_menu_point, 1);
+gfx::Point OpaqueNonClientView::GetSystemMenuPoint() const {
+  int tabstrip_height = browser_view_->IsTabStripVisible() ?
+      browser_view_->GetTabStripHeight() : 0;
+  gfx::Point system_menu_point(FrameBorderThickness(),
+      NonClientTopBorderHeight() + tabstrip_height -
+      BottomEdgeThicknessWithinNonClientHeight());
+  ConvertPointToScreen(this, &system_menu_point);
   return system_menu_point;
 }
 
@@ -795,10 +775,12 @@ void OpaqueNonClientView::PaintMaximizedFrameBorder(ChromeCanvas* canvas) {
 
   if (!browser_view_->IsToolbarVisible()) {
     // There's no toolbar to edge the frame border, so we need to draw a bottom
-    // edge.  The App Window graphic we use for this has a built in client edge,
-    // so we clip it off the bottom.
-    int edge_height = app_top_center_.height() - kClientEdgeThickness;
-    canvas->TileImageInt(app_top_center_, 0,
+    // edge.  The graphic we use for this has a built in client edge, so we clip
+    // it off the bottom.
+    SkBitmap* top_center =
+        resources()->GetPartBitmap(FRAME_NO_TOOLBAR_TOP_CENTER);
+    int edge_height = top_center->height() - kClientEdgeThickness;
+    canvas->TileImageInt(*top_center, 0,
         frame_->client_view()->y() - edge_height, width(), edge_height);
   }
 }
@@ -806,8 +788,8 @@ void OpaqueNonClientView::PaintMaximizedFrameBorder(ChromeCanvas* canvas) {
 void OpaqueNonClientView::PaintDistributorLogo(ChromeCanvas* canvas) {
   // The distributor logo is only painted when the frame is not maximized and
   // when we actually have a logo.
-  if (!frame_->IsMaximized() && !distributor_logo_.empty()) {
-    canvas->DrawBitmapInt(distributor_logo_,
+  if (!frame_->IsMaximized() && distributor_logo_) {
+    canvas->DrawBitmapInt(*distributor_logo_,
         MirroredLeftPointForRect(logo_bounds_), logo_bounds_.y());
   }
 }
@@ -893,15 +875,18 @@ void OpaqueNonClientView::PaintRestoredClientEdge(ChromeCanvas* canvas) {
     // shorter than the top left and right bitmaps.  We need their top edges to
     // line up, and we need the left and right edges to start below the corners'
     // bottoms.
-    int top_edge_y = client_area_top - app_top_center_.height();
-    client_area_top = top_edge_y + app_top_left_.height();
-    canvas->DrawBitmapInt(app_top_left_,
-                          client_area_bounds.x() - app_top_left_.width(),
-                          top_edge_y);
-    canvas->TileImageInt(app_top_center_, client_area_bounds.x(), top_edge_y,
-                         client_area_bounds.width(), app_top_center_.height());
-    canvas->DrawBitmapInt(app_top_right_, client_area_bounds.right(),
+    SkBitmap* top_center =
+        resources()->GetPartBitmap(FRAME_NO_TOOLBAR_TOP_CENTER);
+    SkBitmap* top_left = resources()->GetPartBitmap(FRAME_NO_TOOLBAR_TOP_LEFT);
+    int top_edge_y = client_area_top - top_center->height();
+    client_area_top = top_edge_y + top_left->height();
+    canvas->DrawBitmapInt(*top_left, client_area_bounds.x() - top_left->width(),
                           top_edge_y);
+    canvas->TileImageInt(*top_center, client_area_bounds.x(), top_edge_y,
+                         client_area_bounds.width(), top_center->height());
+    canvas->DrawBitmapInt(
+        *resources()->GetPartBitmap(FRAME_NO_TOOLBAR_TOP_RIGHT),
+        client_area_bounds.right(), top_edge_y);
   }
 
   int client_area_bottom =
@@ -982,10 +967,13 @@ void OpaqueNonClientView::LayoutWindowControls() {
 void OpaqueNonClientView::LayoutDistributorLogo() {
   // Always lay out the logo, even when it's not present, so we can lay out the
   // window title based on its position.
-  int logo_x = minimize_button_->x() - (distributor_logo_.empty() ?
-      0 : (distributor_logo_.width() + kLogoCaptionSpacing));
-  logo_bounds_.SetRect(logo_x, TopResizeHeight(), distributor_logo_.width(),
-                       distributor_logo_.height());
+  if (distributor_logo_) {
+    logo_bounds_.SetRect(minimize_button_->x() - distributor_logo_->width() -
+        kLogoCaptionSpacing, TopResizeHeight(), distributor_logo_->width(),
+        distributor_logo_->height());
+  } else {
+    logo_bounds_.SetRect(minimize_button_->x(), TopResizeHeight(), 0, 0);
+  }
 }
 
 void OpaqueNonClientView::LayoutTitleBar() {
@@ -1020,15 +1008,13 @@ void OpaqueNonClientView::LayoutTitleBar() {
   views::WindowDelegate* d = frame_->window_delegate();
   if (!d->ShouldShowWindowIcon())
     icon_size = 0;
-  icon_bounds_.SetRect(icon_x, icon_y, icon_size, icon_size);
   if (window_icon_)
-    window_icon_->SetBounds(icon_bounds_);
+    window_icon_->SetBounds(icon_x, icon_y, icon_size, icon_size);
 
   // Size the title, if visible.
   if (d->ShouldShowWindowTitle()) {
-    int icon_right = icon_bounds_.right();
-    int title_x =
-        icon_right + (d->ShouldShowWindowIcon() ? kIconTitleSpacing : 0);
+    int title_x = icon_x + icon_size +
+        (d->ShouldShowWindowIcon() ? kIconTitleSpacing : 0);
     title_bounds_.SetRect(title_x,
         title_top_spacing + ((title_thickness - title_font_.height()) / 2),
         std::max(0, logo_bounds_.x() - kTitleLogoSpacing - title_x),
@@ -1060,15 +1046,11 @@ void OpaqueNonClientView::InitClass() {
     active_resources_ = new ActiveWindowResources;
     inactive_resources_ = new InactiveWindowResources;
 
-    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 #if defined(GOOGLE_CHROME_BUILD)
-    distributor_logo_ = *rb.GetBitmapNamed(IDR_DISTRIBUTOR_LOGO_LIGHT);
+    distributor_logo_ = ResourceBundle::GetSharedInstance().
+        GetBitmapNamed(IDR_DISTRIBUTOR_LOGO_LIGHT);
 #endif
 
-    app_top_left_ = *rb.GetBitmapNamed(IDR_APP_TOP_LEFT);
-    app_top_center_ = *rb.GetBitmapNamed(IDR_APP_TOP_CENTER);
-    app_top_right_ = *rb.GetBitmapNamed(IDR_APP_TOP_RIGHT);      
-
     initialized = true;
   }
 }
diff --git a/chrome/browser/views/frame/opaque_non_client_view.h b/chrome/browser/views/frame/opaque_non_client_view.h
index 6769fff..58c5fa1 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.h
+++ b/chrome/browser/views/frame/opaque_non_client_view.h
@@ -43,7 +43,7 @@ class OpaqueNonClientView : public 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);
@@ -123,9 +123,6 @@ class OpaqueNonClientView : public views::NonClientView,
   // The layout rect of the title, if visible.
   gfx::Rect title_bounds_;
 
-  // The layout rect of the window icon.
-  gfx::Rect icon_bounds_;
-
   // The layout rect of the distributor logo, if visible.
   gfx::Rect logo_bounds_;
 
@@ -156,10 +153,7 @@ class OpaqueNonClientView : public views::NonClientView,
 
   static void InitClass();
   static void InitAppWindowResources();
-  static SkBitmap distributor_logo_;
-  static SkBitmap app_top_left_;
-  static SkBitmap app_top_center_;
-  static SkBitmap app_top_right_;
+  static SkBitmap* distributor_logo_;
   static views::WindowResources* active_resources_;
   static views::WindowResources* inactive_resources_;
   static views::WindowResources* active_otr_resources_;
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);